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;
}