Implemented a smart diff algorithm on the server side to detect actual UPDATES.
Each entity is serialized to a jsonObject, and their before and after states
are compared. Added tests that exercise this smart diff-ing.

Patch by: amitmanjhi
Review by: rjrjr (desk review)


git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8570 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java b/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
index 91891b3..2ea103a 100644
--- a/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
+++ b/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
@@ -53,13 +53,11 @@
  */
 public class JsonRequestProcessor implements RequestProcessor<String> {
 
-  public static final String RELATED = "related";
-
   // TODO should we consume String, InputStream, or JSONObject?
   private class DvsData {
     private final JSONObject jsonObject;
     private final WriteOperation writeOperation;
-    
+
     DvsData(JSONObject jsonObject, WriteOperation writeOperation) {
       this.jsonObject = jsonObject;
       this.writeOperation = writeOperation;
@@ -70,13 +68,10 @@
     private final Object entityInstance;
     // TODO: violations should have more structure than JSONObject
     private final JSONObject violations;
-    private final WriteOperation writeOperation;
 
-    EntityData(Object entityInstance, JSONObject violations,
-        WriteOperation writeOperation) {
+    EntityData(Object entityInstance, JSONObject violations) {
       this.entityInstance = entityInstance;
       this.violations = violations;
-      this.writeOperation = writeOperation;
     }
   }
 
@@ -110,6 +105,20 @@
     }
   }
 
+  private class SerializedEntity {
+    // the field value of the entityInstance might change from under us.
+    private final Object entityInstance;
+
+    private final JSONObject serializedEntity;
+
+    SerializedEntity(Object entityInstance, JSONObject serializedEntity) {
+      this.entityInstance = entityInstance;
+      this.serializedEntity = serializedEntity;
+    }
+  }
+
+  public static final String RELATED = "related";
+
   public static final Set<String> BLACK_LIST = initBlackList();
 
   private static final Logger log = Logger.getLogger(JsonRequestProcessor.class.getName());
@@ -140,9 +149,9 @@
    */
   private Set<EntityKey> involvedKeys = new HashSet<EntityKey>();
   private Map<EntityKey, DvsData> dvsDataMap = new HashMap<EntityKey, DvsData>();
-  private Map<EntityKey, EntityData> beforeDataMap = new HashMap<EntityKey, EntityData>();
+  private Map<EntityKey, SerializedEntity> beforeDataMap = new HashMap<EntityKey, SerializedEntity>();
   private Map<EntityKey, EntityData> afterDvsDataMap = new HashMap<EntityKey, EntityData>();
-  
+
   public Collection<Property<?>> allProperties(Class<? extends Record> clazz) {
     Set<Property<?>> rtn = new HashSet<Property<?>>();
     for (Field f : clazz.getFields()) {
@@ -417,11 +426,11 @@
         violations = validator.validate(entityInstance);
       }
       return new EntityData(entityInstance, (violations.isEmpty() ? null
-          : getViolationsAsJson(violations)), writeOperation);
+          : getViolationsAsJson(violations)));
     } catch (Exception ex) {
       log.severe(String.format("Caught exception [%s] %s",
           ex.getClass().getName(), ex.getLocalizedMessage()));
-      return getEntityDataForException(entityKey, writeOperation, ex);
+      return getEntityDataForException(entityKey, ex);
     }
   }
 
@@ -641,20 +650,22 @@
     }
 
     if (topLevelJsonObject.has(RequestData.CONTENT_TOKEN)) {
-       // updates involvedKeys and dvsDataMap.
-       decodeDVS(topLevelJsonObject.getString(RequestData.CONTENT_TOKEN));
+      // updates involvedKeys and dvsDataMap.
+      decodeDVS(topLevelJsonObject.getString(RequestData.CONTENT_TOKEN));
     }
-   // get the domain object (for instance methods) and args.
+    // get the domain object (for instance methods) and args.
     Object args[][] = getObjectsFromParameterMap(operation.isInstance(),
         getParameterMap(topLevelJsonObject), domainMethod.getParameterTypes());
-    
-    // Construct beforeDataMap 
+
+    // the involvedKeys set is complete at this point.
+
+    // Construct beforeDataMap
     constructBeforeDataMap();
     // Construct afterDvsDataMap.
     constructAfterDvsDataMap();
-    
+
     // resolve parameters that are so far just EntityKeys.
-    // TODO: resolve paramters other than  the domainInstance 
+    // TODO: resolve paramters other than the domainInstance
     EntityKey domainEntityKey = null;
     if (args[0][0] != null) {
       domainEntityKey = (EntityKey) args[0][0];
@@ -710,6 +721,47 @@
     }
   }
 
+  /**
+   * Returns true iff the after and before JSONObjects are different.
+   * 
+   * @throws JSONException
+   */
+  boolean hasChanged(JSONObject before, JSONObject after) {
+    if (before == null) {
+      return after != null;
+    }
+    // before != null
+    if (after == null) {
+      return true;
+    }
+    try {
+      // before != null && after != null
+      Iterator<?> keyIterator = before.keys();
+      while (keyIterator.hasNext()) {
+        String key = keyIterator.next().toString();
+        Object beforeValue = before.get(key);
+        Object afterValue = after.get(key);
+        if (beforeValue == null) {
+          if (afterValue == null) {
+            continue;
+          }
+          return true;
+        }
+        if (afterValue == null) {
+          return true;
+        }
+        if (!beforeValue.equals(afterValue)) {
+          return true;
+        }
+      }
+    } catch (JSONException ex) {
+      log.warning("Encountered a JSONException " + ex.getMessage()
+          + " in getChanged");
+      return false;
+    }
+    return false;
+  }
+
   private void addRelatedObject(String keyRef, Object returnValue,
       Class<? extends Record> propertyType, RequestProperty propertyContext)
       throws JSONException, IllegalAccessException, NoSuchMethodException,
@@ -738,10 +790,10 @@
         afterDvsDataMap.put(entityKey, entityData);
       } else {
         assert !entityKey.isFuture;
-        EntityData entityData = beforeDataMap.get(entityKey);
-        assert entityData != null;
-        // TODO(cromwellian): copy here
-        afterDvsDataMap.put(entityKey, entityData);
+        SerializedEntity serializedEntity = beforeDataMap.get(entityKey);
+        assert serializedEntity.entityInstance != null;
+        afterDvsDataMap.put(entityKey, new EntityData(
+            serializedEntity.entityInstance, null));
       }
     }
   }
@@ -753,36 +805,23 @@
    * Algorithm: go through the involvedKeys, and find the entityData
    * corresponding to each.
    * 
-   * @throws NoSuchMethodException
-   * @throws InvocationTargetException
-   * @throws IllegalAccessException
-   * @throws SecurityException
-   * @throws IllegalArgumentException
    */
   private void constructBeforeDataMap() throws IllegalArgumentException,
       SecurityException, IllegalAccessException, InvocationTargetException,
-      NoSuchMethodException {
-    beforeDataMap = new HashMap<EntityKey, EntityData>();
+      NoSuchMethodException, JSONException {
     for (EntityKey entityKey : involvedKeys) {
       if (entityKey.isFuture) {
         // the "before" is empty.
         continue;
       }
-      // 
-      Class<?> entityClass = getEntityFromRecordAnnotation(entityKey.record);
-      // TODO: merge this lookup code with other uses.
-      Object entityInstance = entityClass.getMethod(
-          "find" + entityClass.getSimpleName(), Long.class).invoke(null,
-          new Long(entityKey.id));
-      beforeDataMap.put(entityKey, new EntityData(entityInstance, null, null));
+      beforeDataMap.put(entityKey, getSerializedEntity(entityKey));
     }
   }
 
   /**
    * Decode deltaValueStore to populate involvedKeys and dvsDataMap.
    */
-  private void decodeDVS(String content)
-      throws SecurityException {
+  private void decodeDVS(String content) throws SecurityException {
     try {
       JSONObject jsonObject = new JSONObject(content);
       for (WriteOperation writeOperation : WriteOperation.values()) {
@@ -820,7 +859,7 @@
   private WriteOperation detectDeleteOrUpdate(EntityKey entityKey,
       EntityData entityData) throws IllegalArgumentException,
       SecurityException, IllegalAccessException, InvocationTargetException,
-      NoSuchMethodException {
+      NoSuchMethodException, JSONException {
     if (entityData.entityInstance == null) {
       return null;
     }
@@ -833,9 +872,8 @@
     if (entityInstance == null) {
       return WriteOperation.DELETE;
     }
-    // TODO (cromwellian): is it an update? how to detect?
-    // This is a HACK!!!
-    if (entityData.writeOperation == WriteOperation.UPDATE) {
+    if (hasChanged(beforeDataMap.get(entityKey).serializedEntity,
+        serializeEntity(entityInstance, entityKey.record))) {
       return WriteOperation.UPDATE;
     }
     return null;
@@ -877,8 +915,7 @@
     return returnObject;
   }
 
-  private EntityData getEntityDataForException(EntityKey entityKey,
-      WriteOperation writeOperation, Exception ex) {
+  private EntityData getEntityDataForException(EntityKey entityKey, Exception ex) {
 
     JSONObject violations = null;
     try {
@@ -893,7 +930,7 @@
       // ignore.
       e.printStackTrace();
     }
-    return new EntityData(null, violations, writeOperation);
+    return new EntityData(null, violations);
   }
 
   /**
@@ -913,6 +950,20 @@
     return record.getName() + "-" + newId;
   }
 
+  private SerializedEntity getSerializedEntity(EntityKey entityKey)
+      throws IllegalArgumentException, SecurityException,
+      IllegalAccessException, InvocationTargetException, NoSuchMethodException,
+      JSONException {
+    Class<?> entityClass = getEntityFromRecordAnnotation(entityKey.record);
+    // TODO: merge this lookup code with other uses.
+    Object entityInstance = entityClass.getMethod(
+        "find" + entityClass.getSimpleName(), Long.class).invoke(null,
+        new Long(entityKey.id));
+    JSONObject serializedEntity = serializeEntity(entityInstance,
+        entityKey.record);
+    return new SerializedEntity(entityInstance, serializedEntity);
+  }
+
   /**
    * Returns a JSONObject with at most three keys: CREATE, UPDATE, DELETE. Each
    * value is a JSONArray of JSONObjects.
@@ -975,6 +1026,42 @@
     }
   }
 
+  /**
+   * Return the properties of an entityInstance, visible on the client, as a
+   * JSONObject.
+   *<p>
+   * TODO: clean up the copy-paste from getJSONObject.
+   */
+  private JSONObject serializeEntity(Object entityInstance,
+      Class<? extends Record> recordClass) throws SecurityException,
+      NoSuchMethodException, IllegalArgumentException, IllegalAccessException,
+      InvocationTargetException, JSONException {
+    if (entityInstance == null) {
+      return null;
+    }
+    JSONObject jsonObject = new JSONObject();
+    for (Property<?> p : allProperties(recordClass)) {
+      String propertyName = p.getName();
+      String methodName = getMethodNameFromPropertyName(propertyName, "get");
+      Method method = entityInstance.getClass().getMethod(methodName);
+      Object returnValue = method.invoke(entityInstance);
+
+      Object propertyValue;
+      if (returnValue != null && Record.class.isAssignableFrom(p.getType())) {
+        Method idMethod = returnValue.getClass().getMethod("getId");
+        Long id = (Long) idMethod.invoke(returnValue);
+
+        propertyValue = operationRegistry.getSecurityProvider().encodeClassType(
+            p.getType())
+            + "-" + id;
+      } else {
+        propertyValue = encodePropertyValue(returnValue);
+      }
+      jsonObject.put(propertyName, propertyValue);
+    }
+    return jsonObject;
+  }
+
   @SuppressWarnings("unchecked")
   private Object toJsonArray(RequestDefinition operation, Object result)
       throws IllegalAccessException, JSONException, NoSuchMethodException,
diff --git a/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java b/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
index 5818711..f87a40b 100644
--- a/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
+++ b/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
@@ -24,8 +24,10 @@
 import junit.framework.TestCase;
 
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 
+import java.lang.reflect.InvocationTargetException;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.Date;
@@ -94,21 +96,7 @@
     com.google.gwt.valuestore.server.SimpleFoo.reset();
     try {
       // fetch object
-      JSONObject results = (JSONObject) requestProcessor.processJsonRequest("{ \""
-          + RequestData.OPERATION_TOKEN + "\": \""
-          + com.google.gwt.valuestore.shared.SimpleFooRequest.class.getName()
-          + ReflectionBasedOperationRegistry.SCOPE_SEPARATOR
-          + "findSimpleFooById\", " + "\"" + RequestData.PARAM_TOKEN
-          + "0\": \"999\" }");
-      JSONObject foo = results.getJSONObject("result");   
-      assertEquals(foo.getInt("id"), 999);
-      assertEquals(foo.getInt("intId"), 42);
-      assertEquals(foo.getString("userName"), "GWT");
-      assertEquals(foo.getLong("longField"), 8L);
-      assertEquals(foo.getInt("enumField"), 0);
-      assertEquals(foo.getInt("version"), 1);
-      assertEquals(foo.getBoolean("boolField"), true);
-      assertTrue(foo.has("created"));
+     JSONObject foo = fetchVerifyAndGetInitialObject();
 
       // modify fields and sync
       foo.put("intId", 45);
@@ -118,25 +106,16 @@
       foo.put("boolField", false);
       Date now = new Date();
       foo.put("created", "" + now.getTime());
-      JSONObject recordWithSchema = new JSONObject();
-      recordWithSchema.put(SimpleFooRecord.class.getName(), foo);
-      JSONArray arr = new JSONArray();
-      arr.put(recordWithSchema);
-      JSONObject operation = new JSONObject();
-      
-      operation.put(WriteOperation.UPDATE.toString(), arr);
-      JSONObject sync = new JSONObject();
-      sync.put(RequestData.OPERATION_TOKEN, "com.google.gwt.valuestore.shared.SimpleFooRequest::persist");
-      sync.put(RequestData.CONTENT_TOKEN, operation.toString());
-      sync.put(RequestData.PARAM_TOKEN + "0", foo.getInt("id") + "-NO" + "-"
-          + SimpleFooRecord.class.getName());
-      JSONObject result = (JSONObject) requestProcessor.processJsonRequest(sync.toString());
 
-      // TODO: commented till snapshotting and automatic-diff is ready.
+     JSONObject result = getResultFromServer(foo);
+
       // check modified fields and no violations
       SimpleFoo fooResult = SimpleFoo.getSingleton();
-      assertFalse(result.getJSONObject("sideEffects").getJSONArray("UPDATE").getJSONObject(
-          0).has("violations"));
+      JSONArray updateArray = result.getJSONObject("sideEffects").getJSONArray("UPDATE");
+      assertEquals(1, updateArray.length());
+      assertEquals(1, updateArray.getJSONObject(0).length());
+      assertTrue(updateArray.getJSONObject(0).has("id"));
+      assertFalse(updateArray.getJSONObject(0).has("violations"));
       assertEquals((int) 45, (int) fooResult.getIntId());
       assertEquals("JSC", fooResult.getUserName());
       assertEquals(now, fooResult.getCreated());
@@ -151,6 +130,62 @@
     }
   }
 
+  public void testEndToEndSmartDiff_NoChange() {
+    com.google.gwt.valuestore.server.SimpleFoo.reset();
+    try {
+      // fetch object
+      JSONObject foo = fetchVerifyAndGetInitialObject();
+
+      // change the value on the server behind the back
+      SimpleFoo fooResult = SimpleFoo.getSingleton();
+      fooResult.setUserName("JSC");
+      fooResult.setIntId(45);
+
+      // modify fields and sync -- there should be no change on the server.
+      foo.put("intId", 45);
+      foo.put("userName", "JSC");
+      JSONObject result = getResultFromServer(foo);
+
+      // check modified fields and no violations
+      assertFalse(result.getJSONObject("sideEffects").has("UPDATE"));
+      fooResult = SimpleFoo.getSingleton();
+      assertEquals((int) 45, (int) fooResult.getIntId());
+      assertEquals("JSC", fooResult.getUserName());
+    } catch (Exception e) {
+      e.printStackTrace();
+      fail(e.toString());
+    }
+  }
+
+  public void testEndToEndSmartDiff_SomeChange() {
+    com.google.gwt.valuestore.server.SimpleFoo.reset();
+    try {
+      // fetch object
+      JSONObject foo = fetchVerifyAndGetInitialObject();
+
+      // change some fields but don't change other fields.
+      SimpleFoo fooResult = SimpleFoo.getSingleton();
+      fooResult.setUserName("JSC");
+      fooResult.setIntId(45);
+      foo.put("userName", "JSC");
+      foo.put("intId", 45);
+      Date now = new Date();
+      foo.put("created", "" + now.getTime());
+
+      JSONObject result = getResultFromServer(foo);
+
+      // check modified fields and no violations
+      assertTrue(result.getJSONObject("sideEffects").has("UPDATE"));
+      fooResult = SimpleFoo.getSingleton();
+      assertEquals((int) 45, (int) fooResult.getIntId());
+      assertEquals("JSC", fooResult.getUserName());
+      assertEquals(now, fooResult.getCreated());
+    } catch (Exception e) {
+      e.printStackTrace();
+      fail(e.toString());
+    }
+  }
+
   private void assertEncodedType(Class<?> expected, Object value) {
     assertEquals(expected,
         requestProcessor.encodePropertyValue(value).getClass());
@@ -162,4 +197,46 @@
     assertEquals(expectedType, val.getClass());
     assertEquals(expectedValue, val);
   }
+
+  private JSONObject fetchVerifyAndGetInitialObject() throws JSONException,
+      NoSuchMethodException, IllegalAccessException, InvocationTargetException,
+      ClassNotFoundException {
+    JSONObject results = (JSONObject) requestProcessor.processJsonRequest("{ \""
+        + RequestData.OPERATION_TOKEN
+        + "\": \""
+        + com.google.gwt.valuestore.shared.SimpleFooRequest.class.getName()
+        + ReflectionBasedOperationRegistry.SCOPE_SEPARATOR
+        + "findSimpleFooById\", "
+        + "\""
+        + RequestData.PARAM_TOKEN
+        + "0\": \"999\" }");
+    JSONObject foo = results.getJSONObject("result");
+    assertEquals(foo.getInt("id"), 999);
+    assertEquals(foo.getInt("intId"), 42);
+    assertEquals(foo.getString("userName"), "GWT");
+    assertEquals(foo.getLong("longField"), 8L);
+    assertEquals(foo.getInt("enumField"), 0);
+    assertEquals(foo.getInt("version"), 1);
+    assertEquals(foo.getBoolean("boolField"), true);
+    assertTrue(foo.has("created"));
+    return foo;
+  }
+
+  private JSONObject getResultFromServer(JSONObject foo) throws JSONException,
+      NoSuchMethodException, IllegalAccessException, InvocationTargetException,
+      ClassNotFoundException {
+    JSONObject recordWithSchema = new JSONObject();
+    recordWithSchema.put(SimpleFooRecord.class.getName(), foo);
+    JSONArray arr = new JSONArray();
+    arr.put(recordWithSchema);
+    JSONObject operation = new JSONObject();
+    operation.put(WriteOperation.UPDATE.toString(), arr);
+    JSONObject sync = new JSONObject();
+    sync.put(RequestData.OPERATION_TOKEN,
+        "com.google.gwt.valuestore.shared.SimpleFooRequest::persist");
+    sync.put(RequestData.CONTENT_TOKEN, operation.toString());
+    sync.put(RequestData.PARAM_TOKEN + "0", foo.getInt("id") + "-NO" + "-"
+        + SimpleFooRecord.class.getName());
+    return (JSONObject) requestProcessor.processJsonRequest(sync.toString());
+  }
 }
diff --git a/user/test/com/google/gwt/valuestore/server/SimpleFoo.java b/user/test/com/google/gwt/valuestore/server/SimpleFoo.java
index a372901..bd0a625 100644
--- a/user/test/com/google/gwt/valuestore/server/SimpleFoo.java
+++ b/user/test/com/google/gwt/valuestore/server/SimpleFoo.java
@@ -73,6 +73,8 @@
 
   private Long longField;
 
+  private String password;
+
   private String userName;
 
   private SimpleBar barField;
@@ -116,6 +118,10 @@
     return longField;
   }
 
+  public String getPassword() {
+    return password;
+  }
+
   public String getUserName() {
     return userName;
   }
@@ -156,6 +162,10 @@
     this.longField = longField;
   }
 
+  public void setPassword(String password) {
+    this.password = password;
+  }
+
   public void setUserName(String userName) {
     this.userName = userName;
   }