Revision: 8945
Author: amitman...@google.com
Date: Tue Oct  5 14:47:46 2010
Log: Send an update notification to a stale client by comparing the incoming version
number with the version number on the server.

Hacked the test harness so that the version number is only incremented on a
real change. Will fix the test harness in a follow-up patch.

Patch by: amitmanjhi
Review by: rjrjr, robertvawter

Review at http://gwt-code-reviews.appspot.com/959801

http://code.google.com/p/google-web-toolkit/source/detail?r=8945

Modified:
/trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java /trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
 /trunk/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java

=======================================
--- /trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java Tue Oct 5 11:03:13 2010 +++ /trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java Tue Oct 5 14:47:46 2010
@@ -62,13 +62,22 @@
 public class JsonRequestProcessor implements RequestProcessor<String> {

   // TODO should we consume String, InputStream, or JSONObject?
-  private static class DvsData {
+  private class DvsData {
     private final JSONObject jsonObject;
     private final WriteOperation writeOperation;
-
-    DvsData(JSONObject jsonObject, WriteOperation writeOperation) {
+    // save version because it is deleted later.
+    private final Integer version;
+
+    DvsData(JSONObject jsonObject, WriteOperation writeOperation)
+        throws JSONException, SecurityException, IllegalAccessException,
+        InvocationTargetException, NoSuchMethodException,
+        InstantiationException {
       this.jsonObject = jsonObject;
       this.writeOperation = writeOperation;
+ this.version = (Integer) (jsonObject.has(Constants.ENCODED_VERSION_PROPERTY) + ? decodeParameterValue(Constants.ENTITY_VERSION_PROPERTY.getType(), + jsonObject.get(Constants.ENCODED_VERSION_PROPERTY).toString())
+          : null);
     }
   }

@@ -1144,7 +1153,8 @@
    * Decode deltaValueStore to populate involvedKeys and dvsDataMap.
    */
   private void decodeDVS(String content) throws SecurityException,
-      JSONException {
+      JSONException, IllegalAccessException, InvocationTargetException,
+      NoSuchMethodException, InstantiationException {
     JSONObject jsonObject = new JSONObject(content);
     for (WriteOperation writeOperation : WriteOperation.values()) {
       if (!jsonObject.has(writeOperation.name())) {
@@ -1175,26 +1185,6 @@
       }
     }
   }
-
-  private WriteOperation detectDeleteOrUpdate(EntityKey entityKey,
-      EntityData entityData) throws IllegalArgumentException,
-      SecurityException, IllegalAccessException, InvocationTargetException,
-      NoSuchMethodException, JSONException, InstantiationException {
-    if (entityData == null || entityData.entityInstance == null) {
-      return null;
-    }
-
-    Object entityInstance = getEntityInstance(entityKey);
-    if (entityInstance == null) {
-      return WriteOperation.DELETE;
-    }
-    SerializedEntity beforeEntity = beforeDataMap.get(entityKey);
-    if (beforeEntity != null && hasChanged(beforeEntity.serializedEntity,
-        serializeEntity(entityInstance, entityKey))) {
-      return WriteOperation.UPDATE;
-    }
-    return null;
-  }

   private String encodeId(Object id) {
     if (id instanceof String) {
@@ -1375,6 +1365,7 @@
       if (entityData == null) {
         continue;
       }
+      // handle CREATE
       if (entityKey.isFuture) {
JSONObject createRecord = getCreateReturnRecord(entityKey, entityData);
         if (createRecord != null) {
@@ -1382,20 +1373,35 @@
         }
         continue;
       }
-      WriteOperation writeOperation = detectDeleteOrUpdate(entityKey,
-          entityData);
-      if (writeOperation == WriteOperation.DELETE) {
+      // handle DELETE
+      Object entityInstanceAfterOperation = getEntityInstance(entityKey);
+      if (null == entityInstanceAfterOperation) {
         JSONObject deleteRecord = new JSONObject();
-        deleteRecord.put(Constants.ENCODED_ID_PROPERTY, getSchemaAndId(
-            entityKey.proxyType, entityKey.encodedId));
+        deleteRecord.put(Constants.ENCODED_ID_PROPERTY,
+            getSchemaAndId(entityKey.proxyType, entityKey.encodedId));
         deleteArray.put(deleteRecord);
-      }
-      if (writeOperation == WriteOperation.UPDATE) {
-        Object entityInstance = entityData.entityInstance;
-        assert entityInstance != null;
+        continue;
+      }
+      /*
+ * Send an UPDATE if the client is at a version different than that of the + * server or if the server version has changed after invoking the domain
+       * method.
+       */
+      boolean clientNeedsUpdating = false;
+      DvsData dvsData = dvsDataMap.get(entityKey);
+      if (dvsData != null && dvsData.version != null) {
+        Integer serverVersion = (Integer) getRawPropertyValueFromDatastore(
+            entityInstanceAfterOperation,
+            Constants.ENTITY_VERSION_PROPERTY.getName());
+        if (!dvsData.version.equals(serverVersion)) {
+          clientNeedsUpdating = true;
+        }
+      }
+      if (clientNeedsUpdating
+ || hasServerVersionChanged(entityKey, entityInstanceAfterOperation)) {
         updateArray.put(getJsonObjectWithIdAndVersion(
             getSchemaAndId(entityKey.proxyType, entityKey.encodedId),
-            entityInstance, propertyRefs));
+            entityInstanceAfterOperation, propertyRefs));
       }
     }
     if (createArray.length() > 0) {
@@ -1436,6 +1442,18 @@
     }
     return violations;
   }
+
+  private boolean hasServerVersionChanged(EntityKey entityKey,
+      Object entityInstanceAfterOperation) throws IllegalArgumentException,
+      SecurityException, IllegalAccessException, InvocationTargetException,
+      NoSuchMethodException, JSONException, InstantiationException {
+    SerializedEntity beforeEntity = beforeDataMap.get(entityKey);
+    if (beforeEntity != null && hasChanged(beforeEntity.serializedEntity,
+        serializeEntity(entityInstanceAfterOperation, entityKey))) {
+      return true;
+    }
+    return false;
+  }

private String isEntityReference(Object entity, Class<?> proxyPropertyType)
       throws SecurityException, NoSuchMethodException,
=======================================
--- /trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java Mon Oct 4 15:35:00 2010 +++ /trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java Tue Oct 5 14:47:46 2010
@@ -111,6 +111,7 @@
// nulls becomes JSON Null. Needed because JSONObject stringify treats 'null'
     // as a reason to not emit the key in the stringified object
assertEquals(JSONObject.NULL, requestProcessor.encodePropertyValue(null)); + assertEquals(new Double(1.0), requestProcessor.encodePropertyValue(new Integer(1)));
   }

   public void testEndToEnd() throws Exception {
@@ -147,26 +148,102 @@
     assertEquals(false, (boolean) fooResult.getBoolField());
   }

-  public void testEndToEndSmartDiff_NoChange() throws Exception {
-    com.google.gwt.requestfactory.server.SimpleFoo.reset();
-    // fetch object
-    JSONObject foo = fetchVerifyAndGetInitialObject();
-
+ private String createNoChangeRequestAndGetServerVersion(JSONObject foo) throws Exception {
     // change the value on the server behind the back
     SimpleFoo fooResult = SimpleFoo.findSimpleFooById(999L);
     fooResult.setUserName("JSC");
     fooResult.setIntId(45);
+ String savedVersion = requestProcessor.encodePropertyValue(fooResult.getVersion() + 1).toString();
+    fooResult.setVersion(fooResult.getVersion() + 1);
+    fooResult.isChanged = false;

     // modify fields and sync -- there should be no change on the server.
     foo.put("intId", 45);
     foo.put("userName", "JSC");
+    return savedVersion;
+  }
+
+  public void testEndToEndSmartDiff_NoChange() throws Exception {
+    com.google.gwt.requestfactory.server.SimpleFoo.reset();
+    // fetch object
+    JSONObject foo = fetchVerifyAndGetInitialObject();
+
+    String savedVersion = createNoChangeRequestAndGetServerVersion(foo);
     JSONObject result = getResultFromServer(foo);

+    // check modified fields and no violations
+    assertTrue(result.getJSONObject("sideEffects").has("UPDATE"));
+ JSONArray updateArray = result.getJSONObject("sideEffects").getJSONArray(
+        "UPDATE");
+    // verify that the version number is unchanged.
+    assertEquals(1, updateArray.length());
+    assertEquals(2, updateArray.getJSONObject(0).length());
+ assertTrue(updateArray.getJSONObject(0).has(Constants.ENCODED_ID_PROPERTY));
+    assertTrue(updateArray.getJSONObject(0).has(
+        Constants.ENCODED_VERSION_PROPERTY));
+    assertEquals(savedVersion,
+        updateArray.getJSONObject(0).getString(
+            Constants.ENCODED_VERSION_PROPERTY));
+    // verify that the server values are unchanged.
+    SimpleFoo fooResult = SimpleFoo.findSimpleFooById(999L);
+    assertEquals(45, (int) fooResult.getIntId());
+    assertEquals("JSC", fooResult.getUserName());
+    assertEquals(savedVersion,
+ requestProcessor.encodePropertyValue(fooResult.getVersion()).toString());
+  }
+
+  /*
+ * This test differs from testEndToEndSmartDiff_NoChange in that the version
+   * numbers are not sent. As a result, the server does not send an UPDATE
+   * sideEffect in its response.
+   */
+  public void testEndToEndSmartDiff_NoChange_NoVersion() throws Exception {
+    com.google.gwt.requestfactory.server.SimpleFoo.reset();
+    // fetch object
+    JSONObject foo = fetchVerifyAndGetInitialObject();
+
+    String savedVersion = createNoChangeRequestAndGetServerVersion(foo);
+    // remove version number from the request.
+    foo.remove(Constants.ENCODED_VERSION_PROPERTY);
+    JSONObject result = getResultFromServer(foo);
+
     // check modified fields and no violations
     assertFalse(result.getJSONObject("sideEffects").has("UPDATE"));
-    fooResult = SimpleFoo.findSimpleFooById(999L);
+
+    // verify that the server values are unchanged.
+    SimpleFoo fooResult = SimpleFoo.findSimpleFooById(999L);
     assertEquals(45, (int) fooResult.getIntId());
     assertEquals("JSC", fooResult.getUserName());
+    assertEquals(savedVersion,
+ requestProcessor.encodePropertyValue(fooResult.getVersion()).toString());
+  }
+
+  /*
+ * This test differs from testEndToEndSmartDiff_NoChange in that the property + * that is changed here does not cause a version increment. As a result, no
+   * UPDATE is sent back.
+   */
+  public void testEndToEndSmartDiff_NoVersionChange() throws Exception {
+    com.google.gwt.requestfactory.server.SimpleFoo.reset();
+    // fetch object
+    JSONObject foo = fetchVerifyAndGetInitialObject();
+    // change the value on the server behind the back
+    SimpleFoo fooResult = SimpleFoo.findSimpleFooById(999L);
+    fooResult.setLongField(45L);
+ String savedVersion = requestProcessor.encodePropertyValue(fooResult.getVersion()).toString();
+
+    // modify fields and sync -- there should be no change on the server.
+    foo.put("longField", 45L);
+    JSONObject result = getResultFromServer(foo);
+
+    // check modified fields and no violations
+    assertFalse(result.getJSONObject("sideEffects").has("UPDATE"));
+
+    // verify that the server values are unchanged.
+    fooResult = SimpleFoo.findSimpleFooById(999L);
+    assertEquals(45L, (long) fooResult.getLongField());
+    assertEquals(savedVersion,
+ requestProcessor.encodePropertyValue(fooResult.getVersion()).toString());
   }

   public void testEndToEndSmartDiff_SomeChangeWithNull() throws Exception {
=======================================
--- /trunk/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java Tue Oct 5 10:23:33 2010 +++ /trunk/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java Tue Oct 5 14:47:46 2010
@@ -320,6 +320,13 @@

   private List<Integer> numberListField;

+  /*
+   * isChanged is just a quick-and-dirty way to get version-ing for now.
+   * Currently, only set by setUserName and setIntId. TODO for later: Use a
+   * cleaner solution to figure out when to increment version numbers.
+   */
+  boolean isChanged;
+
   public SimpleFoo() {
     intId = 42;
     version = 1;
@@ -342,6 +349,7 @@
     nullField = null;
     barNullField = null;
     pleaseCrash = 0;
+    isChanged = false;
   }

   public Long countSimpleFooWithUserNameSideEffect() {
@@ -495,7 +503,10 @@
       isNew = false;
       get().put(getId(), this);
     }
-    version++;
+    if (isChanged) {
+      version++;
+      isChanged = false;
+    }
   }

   public SimpleFoo persistAndReturnSelf() {
@@ -594,7 +605,10 @@
   }

   public void setIntId(Integer id) {
-    this.intId = id;
+    if (!this.intId.equals(id)) {
+      this.intId = id;
+      isChanged = true;
+    }
   }

   public void setLongField(Long longField) {
@@ -651,7 +665,10 @@
   }

   public void setUserName(String userName) {
-    this.userName = userName;
+    if (!this.userName.equals(userName)) {
+      this.userName = userName;
+      isChanged = true;
+    }
   }

   public void setVersion(Integer version) {

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to