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