Revision: 8920
Author: [email protected]
Date: Sat Oct 2 12:01:57 2010
Log: Fixing a ConcurrentModificationException in JsonRequestProcessor that
occurs when you use a setter that sets an Entity from null to a value. Also
fixes a bug in DeltaValueStoreJsonImpl#retainValue() where we only retain
the first element of a List instead of all elements in the list because the
return statement is in the wrong place. This revealed other NPEs on the
server that are also fixed.
Review at http://gwt-code-reviews.appspot.com/945801
Review by: [email protected]
http://code.google.com/p/google-web-toolkit/source/detail?r=8920
Modified:
/trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
/trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
/trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
=======================================
---
/trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
Fri Oct 1 18:15:55 2010
+++
/trunk/user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
Sat Oct 2 12:01:57 2010
@@ -342,8 +342,6 @@
entityKey, dvsData.jsonObject, dvsData.writeOperation);
return entityData.entityInstance;
} else {
- // TODO(rjrjr): This results in a
ConcurrentModificationException.
- // involvedKeys loops in
constructAfterDvsDataMapAfterCallingSetters.
involvedKeys.add(entityKey);
return getEntityInstance(entityKey);
}
@@ -356,8 +354,6 @@
throw new IllegalArgumentException("Unknown service, unable to
decode "
+ parameterValue);
}
- // TODO(rjrjr): This results in a ConcurrentModificationException.
- // involvedKeys loops in constructAfterDvsDataMapAfterCallingSetters.
involvedKeys.add(entityKey);
return getEntityInstance(entityKey);
}
@@ -1078,31 +1074,47 @@
private void constructAfterDvsDataMapAfterCallingSetters()
throws SecurityException, JSONException, IllegalAccessException,
InvocationTargetException, NoSuchMethodException,
InstantiationException {
+ /*
+ * EntityKeys can be added to involvedKeys if a setter sets an
EntityType
+ * that was null before calling the setter, so we need to loop in a
way that
+ * will protected against ConcurrentModificationExceptions.
+ */
afterDvsDataMap = new HashMap<EntityKey, EntityData>();
- for (EntityKey entityKey : involvedKeys) {
- // use the beforeDataMap and dvsDataMap
- DvsData dvsData = dvsDataMap.get(entityKey);
- if (dvsData != null) {
- EntityData entityData = getEntityDataForRecordWithSettersApplied(
- entityKey, dvsData.jsonObject, dvsData.writeOperation);
- if (entityKey.isFuture) {
- // TODO: assert that the id is null for entityData.entityInstance
- }
- afterDvsDataMap.put(entityKey, entityData);
- } else if (entityKey.isFuture) {
- // The client-side DVS failed to include a CREATE operation.
- throw new RuntimeException("Future object with no dvsData");
- } else {
- /*
- * Involved, but not in the deltaValueStore -- param ref to an
unedited,
- * existing object.
- */
- SerializedEntity serializedEntity = beforeDataMap.get(entityKey);
- if (serializedEntity.entityInstance != null) {
- afterDvsDataMap.put(entityKey, new EntityData(
- serializedEntity.entityInstance, null));
+ Set<EntityKey> done = new HashSet<EntityKey>();
+ Set<EntityKey> queue = new HashSet<EntityKey>(involvedKeys);
+ while (!queue.isEmpty()) {
+ for (EntityKey entityKey : queue) {
+ // use the beforeDataMap and dvsDataMap
+ DvsData dvsData = dvsDataMap.get(entityKey);
+ if (dvsData != null) {
+ EntityData entityData = getEntityDataForRecordWithSettersApplied(
+ entityKey, dvsData.jsonObject, dvsData.writeOperation);
+ if (entityKey.isFuture) {
+ // TODO: assert that the id is null for
entityData.entityInstance
+ }
+ afterDvsDataMap.put(entityKey, entityData);
+ } else if (entityKey.isFuture) {
+ // The client-side DVS failed to include a CREATE operation.
+ throw new RuntimeException("Future object with no dvsData");
+ } else {
+ /*
+ * Involved, but not in the deltaValueStore -- param ref to an
+ * unedited, existing object.
+ */
+ SerializedEntity serializedEntity = beforeDataMap.get(entityKey);
+ Object entityInstance = (serializedEntity == null) ?
+ getEntityInstance(entityKey) :
serializedEntity.entityInstance;
+ if (entityInstance != null) {
+ afterDvsDataMap.put(entityKey, new EntityData(entityInstance,
+ null));
+ }
}
}
+
+ // Reset the queue.
+ done.addAll(queue);
+ queue.addAll(involvedKeys);
+ queue.removeAll(done);
}
}
@@ -1178,7 +1190,8 @@
if (entityInstance == null) {
return WriteOperation.DELETE;
}
- if (hasChanged(beforeDataMap.get(entityKey).serializedEntity,
+ SerializedEntity beforeEntity = beforeDataMap.get(entityKey);
+ if (beforeEntity != null && hasChanged(beforeEntity.serializedEntity,
serializeEntity(entityInstance, entityKey))) {
return WriteOperation.UPDATE;
}
=======================================
---
/trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
Fri Oct 1 18:15:55 2010
+++
/trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
Sat Oct 2 12:01:57 2010
@@ -190,10 +190,9 @@
/**
* Test that we can commit child objects.
- *
- * TODO: Causes a ConcurrentModificationException
+
*/
- public void disabledTestCascadingCommit() {
+ public void testCascadingCommit() {
delayTestFinish(5000);
SimpleFooRequest context = req.simpleFooRequest();
final SimpleFooProxy foo = context.create(SimpleFooProxy.class);
=======================================
---
/trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
Fri Oct 1 18:15:55 2010
+++
/trunk/user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
Sat Oct 2 12:01:57 2010
@@ -183,8 +183,9 @@
foo.put("userName", JSONObject.NULL);
foo.put("intId", 45);
foo.put("nullField", "test");
- foo.put("barNullField", SimpleBar.getSingleton().getId()
- + "@IS@" + SimpleBarProxy.class.getName());
+ foo.put("barNullField",
+ JsonRequestProcessor.base64Encode(SimpleBar.getSingleton().getId())
+ + "@NO@" + SimpleBarProxy.class.getName());
foo.put("barField", JSONObject.NULL);
JSONObject result = getResultFromServer(foo);
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
