Revision: 6604 Author: [email protected] Date: Tue Nov 3 08:52:16 2009 Log: Add a test for circular references via a CustomFieldSerializer. Update deRPC code to pass this test.
Patch by: bobv Review by: jgw http://code.google.com/p/google-web-toolkit/source/detail?r=6604 Modified: /trunk/user/src/com/google/gwt/rpc/client/impl/CommandClientSerializationStreamWriter.java /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamReader.java /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamWriter.java /trunk/user/src/com/google/gwt/rpc/server/WebModePayloadSink.java /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTest.java /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestService.java /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestServiceAsync.java /trunk/user/test/com/google/gwt/user/client/rpc/TestSetFactory.java /trunk/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java /trunk/user/test/com/google/gwt/user/server/rpc/ObjectGraphTestServiceImpl.java ======================================= --- /trunk/user/src/com/google/gwt/rpc/client/impl/CommandClientSerializationStreamWriter.java Wed Oct 28 09:10:53 2009 +++ /trunk/user/src/com/google/gwt/rpc/client/impl/CommandClientSerializationStreamWriter.java Tue Nov 3 08:52:16 2009 @@ -51,13 +51,19 @@ anObject.hashCode(); } - private final Map<Object, IdentityValueCommand> identityMap = new IdentityHashMap<Object, IdentityValueCommand>(); + private final Map<Object, IdentityValueCommand> identityMap; private final TypeOverrides serializer; public CommandClientSerializationStreamWriter(TypeOverrides serializer, CommandSink sink) { + this(serializer, sink, new IdentityHashMap<Object, IdentityValueCommand>()); + } + + private CommandClientSerializationStreamWriter(TypeOverrides serializer, + CommandSink sink, Map<Object, IdentityValueCommand> identityMap) { super(sink); this.serializer = serializer; + this.identityMap = identityMap; } /** @@ -141,8 +147,13 @@ InvokeCustomFieldSerializerCommand command = new InvokeCustomFieldSerializerCommand( type, null, null); identityMap.put(value, command); + + /* + * Pass the current identityMap into the new writer to allow circular + * references through the graph emitted by the CFS. + */ CommandClientSerializationStreamWriter subWriter = new CommandClientSerializationStreamWriter( - serializer, new HasValuesCommandSink(command)); + serializer, new HasValuesCommandSink(command), identityMap); serializeFunction.serialize(subWriter, value); if (serializer.hasExtraFields(type.getName())) { ======================================= --- /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamReader.java Wed Oct 28 09:10:53 2009 +++ /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamReader.java Tue Nov 3 08:52:16 2009 @@ -177,7 +177,8 @@ public boolean visit(InvokeCustomFieldSerializerCommand x, Context ctx) { if (maybePushBackRef(x)) { - CommandServerSerializationStreamReader subReader = new CommandServerSerializationStreamReader(); + CommandServerSerializationStreamReader subReader = new CommandServerSerializationStreamReader( + backRefs); subReader.prepareToRead(x.getValues()); Class<?> serializerClass = x.getSerializerClass(); @@ -267,9 +268,18 @@ } } - Map<IdentityValueCommand, Object> backRefs = new HashMap<IdentityValueCommand, Object>(); + final Map<IdentityValueCommand, Object> backRefs; Iterator<ValueCommand> values; + public CommandServerSerializationStreamReader() { + this(new HashMap<IdentityValueCommand, Object>()); + } + + private CommandServerSerializationStreamReader( + Map<IdentityValueCommand, Object> backRefs) { + this.backRefs = backRefs; + } + public void prepareToRead(List<ValueCommand> commands) { values = commands.iterator(); assert values.hasNext() : "No commands"; ======================================= --- /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamWriter.java Wed Oct 28 09:10:53 2009 +++ /trunk/user/src/com/google/gwt/rpc/server/CommandServerSerializationStreamWriter.java Tue Nov 3 08:52:16 2009 @@ -48,7 +48,7 @@ CommandSerializationStreamWriterBase { private final ClientOracle clientOracle; - private final Map<Object, IdentityValueCommand> identityMap = new IdentityHashMap<Object, IdentityValueCommand>(); + private final Map<Object, IdentityValueCommand> identityMap; public CommandServerSerializationStreamWriter(CommandSink sink) { this(new HostedModeClientOracle(), sink); @@ -56,8 +56,14 @@ public CommandServerSerializationStreamWriter(ClientOracle oracle, CommandSink sink) { + this(oracle, sink, new IdentityHashMap<Object, IdentityValueCommand>()); + } + + private CommandServerSerializationStreamWriter(ClientOracle oracle, + CommandSink sink, Map<Object, IdentityValueCommand> identityMap) { super(sink); this.clientOracle = oracle; + this.identityMap = identityMap; } /** @@ -70,14 +76,17 @@ return NullValueCommand.INSTANCE; } + /* + * Check accessor map before the identity map because we don't want to + * recurse on wrapped primitive values. + */ Accessor accessor; - - if (identityMap.containsKey(value)) { + if ((accessor = CommandSerializationUtil.getAccessor(type)).canMakeValueCommand()) { + return accessor.makeValueCommand(value); + + } else if (identityMap.containsKey(value)) { return identityMap.get(value); - } else if ((accessor = CommandSerializationUtil.getAccessor(type)).canMakeValueCommand()) { - return accessor.makeValueCommand(value); - } else if (type.isArray()) { return makeArray(type, value); @@ -92,6 +101,7 @@ private ArrayValueCommand makeArray(Class<?> type, Object value) throws SerializationException { ArrayValueCommand toReturn = new ArrayValueCommand(type.getComponentType()); + identityMap.put(value, toReturn); for (int i = 0, j = Array.getLength(value); i < j; i++) { Object arrayValue = Array.get(value, i); if (arrayValue == null) { @@ -102,7 +112,6 @@ toReturn.add(makeValue(valueType, arrayValue)); } } - identityMap.put(value, toReturn); return toReturn; } @@ -202,8 +211,12 @@ instanceClass, customSerializer, manuallySerializedType); identityMap.put(instance, toReturn); + /* + * Pass the current identityMap into the new writer to allow circular + * references through the graph emitted by the CFS. + */ CommandServerSerializationStreamWriter subWriter = new CommandServerSerializationStreamWriter( - clientOracle, new HasValuesCommandSink(toReturn)); + clientOracle, new HasValuesCommandSink(toReturn), identityMap); method.invoke(null, subWriter, instance); return toReturn; ======================================= --- /trunk/user/src/com/google/gwt/rpc/server/WebModePayloadSink.java Wed Oct 28 09:10:53 2009 +++ /trunk/user/src/com/google/gwt/rpc/server/WebModePayloadSink.java Tue Nov 3 08:52:16 2009 @@ -295,101 +295,101 @@ @Override public boolean visit(InvokeCustomFieldSerializerCommand x, Context ctx) { - byte[] currentBackRef = null; - // TODO Extract the commands as an inline function - if (!isStarted(x)) { - InstantiateCommand makeReader = new InstantiateCommand( - CommandClientSerializationStreamReader.class); - /* - * Ensure that the reader will stick around for both instantiate and - * deserialize calls. - */ - makeBackRef(makeReader); - - ArrayValueCommand payload = new ArrayValueCommand(Object.class); - for (ValueCommand value : x.getValues()) { - payload.add(value); - } - makeReader.set(CommandClientSerializationStreamReader.class, "payload", - payload); - - currentBackRef = begin(x); - - String instantiateIdent = clientOracle.getMethodId( - x.getSerializerClass(), "instantiate", - SerializationStreamReader.class); - - // x = $Foo(new Foo); - // x = instantiate(reader); - push(currentBackRef); - eq(); - if (instantiateIdent == null) { - // No instantiate method, we'll have to invoke the constructor - - instantiateIdent = clientOracle.getSeedName(x.getTargetClass()); - assert instantiateIdent != null : "instantiateIdent"; - - // $Foo() - String constructorMethodName; - if (x.getTargetClass().getEnclosingClass() == null) { - constructorMethodName = "$" + x.getTargetClass().getSimpleName(); - } else { - String name = x.getTargetClass().getName(); - constructorMethodName = "$" - + name.substring(name.lastIndexOf('.') + 1); - } - - String constructorIdent = clientOracle.getMethodId( - x.getTargetClass(), constructorMethodName, x.getTargetClass()); - assert constructorIdent != null : "constructorIdent " - + constructorMethodName; - - // constructor(new Seed); - push(constructorIdent); - lparen(); - _new(); - push(instantiateIdent); - rparen(); - semi(); + if (isStarted(x)) { + push(makeBackRef(x)); + return false; + } + + // ( backref = instantiate(), deserialize(), setter, ..., backref ) + byte[] currentBackRef = begin(x); + + lparen(); + + InstantiateCommand makeReader = new InstantiateCommand( + CommandClientSerializationStreamReader.class); + /* + * Ensure that the reader will stick around for both instantiate and + * deserialize calls. + */ + makeBackRef(makeReader); + + ArrayValueCommand payload = new ArrayValueCommand(Object.class); + for (ValueCommand value : x.getValues()) { + payload.add(value); + } + makeReader.set(CommandClientSerializationStreamReader.class, "payload", + payload); + + String instantiateIdent = clientOracle.getMethodId( + x.getSerializerClass(), "instantiate", + SerializationStreamReader.class); + + // x = $Foo(new Foo), + // x = instantiate(reader), + push(currentBackRef); + eq(); + if (instantiateIdent == null) { + // No instantiate method, we'll have to invoke the constructor + + instantiateIdent = clientOracle.getSeedName(x.getTargetClass()); + assert instantiateIdent != null : "instantiateIdent"; + + // $Foo() + String constructorMethodName; + if (x.getTargetClass().getEnclosingClass() == null) { + constructorMethodName = "$" + x.getTargetClass().getSimpleName(); } else { - // instantiate(reader); - push(instantiateIdent); - lparen(); - accept(makeReader); - rparen(); - semi(); + String name = x.getTargetClass().getName(); + constructorMethodName = "$" + + name.substring(name.lastIndexOf('.') + 1); } - // Call the deserialize method if it exists - String deserializeIdent = clientOracle.getMethodId( - x.getSerializerClass(), "deserialize", - SerializationStreamReader.class, x.getManuallySerializedType()); - if (deserializeIdent != null) { - // deserialize(reader, obj); - push(deserializeIdent); - lparen(); - accept(makeReader); - comma(); - push(currentBackRef); - rparen(); - semi(); - } - - // If there are extra fields, set them - for (SetCommand setter : x.getSetters()) { - accept(setter); - semi(); - } - - commit(x); - forget(makeReader); + String constructorIdent = clientOracle.getMethodId(x.getTargetClass(), + constructorMethodName, x.getTargetClass()); + assert constructorIdent != null : "constructorIdent " + + constructorMethodName; + + // constructor(new Seed), + push(constructorIdent); + lparen(); + _new(); + push(instantiateIdent); + rparen(); + comma(); + } else { + // instantiate(reader), + push(instantiateIdent); + lparen(); + accept(makeReader); + rparen(); + comma(); } - if (currentBackRef == null) { - push(makeBackRef(x)); - } else { + // Call the deserialize method if it exists + String deserializeIdent = clientOracle.getMethodId( + x.getSerializerClass(), "deserialize", + SerializationStreamReader.class, x.getManuallySerializedType()); + if (deserializeIdent != null) { + // deserialize(reader, obj), + push(deserializeIdent); + lparen(); + accept(makeReader); + comma(); push(currentBackRef); - } + rparen(); + comma(); + } + + // If there are extra fields, set them + for (SetCommand setter : x.getSetters()) { + accept(setter); + comma(); + } + + push(currentBackRef); + rparen(); + commit(x, false); + forget(makeReader); return false; } @@ -696,7 +696,7 @@ } while (leafType.getComponentType() != null); assert dims > 0; // leafType cannot be null here - + String s = getJavahSignatureName(leafType); for (int i = 0; i < dims; ++i) { s = "_3" + s; ======================================= --- /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTest.java Wed Feb 18 17:08:17 2009 +++ /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTest.java Tue Nov 3 08:52:16 2009 @@ -18,6 +18,7 @@ import com.google.gwt.core.client.GWT; import com.google.gwt.junit.client.GWTTestCase; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode; +import com.google.gwt.user.client.rpc.TestSetFactory.SerializableGraphWithCFS; import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays; import com.google.gwt.user.client.rpc.impl.AbstractSerializationStream; @@ -67,6 +68,25 @@ } }); } + + public void testComplexCyclicGraphWithCFS() { + delayTestFinish(TEST_DELAY); + + ObjectGraphTestServiceAsync service = getServiceAsync(); + service.echo_ComplexCyclicGraphWithCFS( + TestSetFactory.createComplexCyclicGraphWithCFS(), + new AsyncCallback<SerializableGraphWithCFS>() { + public void onFailure(Throwable caught) { + TestSetValidator.rethrowException(caught); + } + + public void onSuccess(SerializableGraphWithCFS result) { + assertNotNull(result); + assertTrue(TestSetValidator.isValidComplexCyclicGraphWithCFS(result)); + finishTest(); + } + }); + } public void testComplexCyclicGraph2() { delayTestFinish(TEST_DELAY); ======================================= --- /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestService.java Thu Mar 27 10:03:00 2008 +++ /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestService.java Tue Nov 3 08:52:16 2009 @@ -16,6 +16,7 @@ package com.google.gwt.user.client.rpc; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode; +import com.google.gwt.user.client.rpc.TestSetFactory.SerializableGraphWithCFS; import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays; @@ -33,9 +34,13 @@ SerializableDoublyLinkedNode echo_ComplexCyclicGraph( SerializableDoublyLinkedNode node1, SerializableDoublyLinkedNode node2); + SerializableGraphWithCFS echo_ComplexCyclicGraphWithCFS( + SerializableGraphWithCFS createComplexCyclicGraphWithArrays); + SerializablePrivateNoArg echo_PrivateNoArg(SerializablePrivateNoArg node); - SerializableWithTwoArrays echo_SerializableWithTwoArrays(SerializableWithTwoArrays node); + SerializableWithTwoArrays echo_SerializableWithTwoArrays( + SerializableWithTwoArrays node); SerializableDoublyLinkedNode echo_TrivialCyclicGraph( SerializableDoublyLinkedNode node); ======================================= --- /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestServiceAsync.java Thu Mar 27 10:03:00 2008 +++ /trunk/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestServiceAsync.java Tue Nov 3 08:52:16 2009 @@ -16,6 +16,7 @@ package com.google.gwt.user.client.rpc; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode; +import com.google.gwt.user.client.rpc.TestSetFactory.SerializableGraphWithCFS; import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays; @@ -34,8 +35,13 @@ void echo_PrivateNoArg(SerializablePrivateNoArg node, AsyncCallback callback); - void echo_SerializableWithTwoArrays(SerializableWithTwoArrays node, AsyncCallback callback); + void echo_SerializableWithTwoArrays(SerializableWithTwoArrays node, + AsyncCallback callback); void echo_TrivialCyclicGraph(SerializableDoublyLinkedNode node, AsyncCallback callback); -} + + void echo_ComplexCyclicGraphWithCFS( + SerializableGraphWithCFS createComplexCyclicGraphWithArrays, + AsyncCallback<SerializableGraphWithCFS> asyncCallback); +} ======================================= --- /trunk/user/test/com/google/gwt/user/client/rpc/TestSetFactory.java Mon Jul 6 16:17:17 2009 +++ /trunk/user/test/com/google/gwt/user/client/rpc/TestSetFactory.java Tue Nov 3 08:52:16 2009 @@ -255,6 +255,32 @@ this.rightChild = rightChild; } } + + /** + * Test reference cycles where the reference graph passes through a + * CustomFieldSerializer. + */ + public static final class SerializableGraphWithCFS implements Serializable { + private ArrayList<SerializableGraphWithCFS> array; + private SerializableGraphWithCFS parent; + + public SerializableGraphWithCFS() { + array = new ArrayList<SerializableGraphWithCFS>(); + array.add(new SerializableGraphWithCFS(this)); + } + + public SerializableGraphWithCFS(SerializableGraphWithCFS parent) { + this.parent = parent; + } + + public ArrayList<SerializableGraphWithCFS> getArray() { + return array; + } + + public SerializableGraphWithCFS getParent() { + return parent; + } + } /** * Tests that classes with a private no-arg constructor can be serialized. @@ -351,6 +377,10 @@ new Character(Character.MAX_VALUE), new Character(Character.MIN_VALUE), new Character(Character.MAX_VALUE), new Character(Character.MIN_VALUE)}; } + + public static SerializableGraphWithCFS createComplexCyclicGraphWithCFS() { + return new SerializableGraphWithCFS(); + } @SuppressWarnings("deprecation") public static Date[] createDateArray() { ======================================= --- /trunk/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java Thu Jul 16 16:13:32 2009 +++ /trunk/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java Tue Nov 3 08:52:16 2009 @@ -15,17 +15,18 @@ */ package com.google.gwt.user.client.rpc; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertSame; + import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap; import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeSet; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode; +import com.google.gwt.user.client.rpc.TestSetFactory.SerializableGraphWithCFS; import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertNotNull; -import static junit.framework.Assert.assertSame; - import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -41,8 +42,8 @@ import java.util.Map.Entry; /** - * Misnamed set of static validation methods used by various - * collection class tests. + * Misnamed set of static validation methods used by various collection class + * tests. * <p> * TODO: could add generics to require args to be of the same type */ @@ -247,7 +248,7 @@ return reference.equals(list); } - public static boolean isValid(HashMap<?,?> expected, HashMap<?,?> map) { + public static boolean isValid(HashMap<?, ?> expected, HashMap<?, ?> map) { if (map == null) { return false; } @@ -259,7 +260,7 @@ Set<?> entries = expected.entrySet(); Iterator<?> entryIter = entries.iterator(); while (entryIter.hasNext()) { - Entry<?,?> entry = (Entry<?,?>) entryIter.next(); + Entry<?, ?> entry = (Entry<?, ?>) entryIter.next(); Object value = map.get(entry.getKey()); @@ -298,8 +299,9 @@ return true; } - public static boolean isValid(LinkedHashMap<?,?> expected, LinkedHashMap<?,?> map) { - if (isValid((HashMap<?,?>) expected, (HashMap<?,?>) map)) { + public static boolean isValid(LinkedHashMap<?, ?> expected, + LinkedHashMap<?, ?> map) { + if (isValid((HashMap<?, ?>) expected, (HashMap<?, ?>) map)) { Iterator<?> expectedEntries = expected.entrySet().iterator(); Iterator<?> actualEntries = map.entrySet().iterator(); return equals(expectedEntries, actualEntries); @@ -493,6 +495,19 @@ return true; } + + public static boolean isValidComplexCyclicGraphWithCFS( + SerializableGraphWithCFS result) { + assertNotNull(result); + List<SerializableGraphWithCFS> array = result.getArray(); + assertNotNull(array); + assertEquals(1, array.size()); + + SerializableGraphWithCFS child = array.get(0); + assertFalse(result == child); + assertSame(result, child.getParent()); + return true; + } public static boolean isValidTrivialCyclicGraph( SerializableDoublyLinkedNode actual) { @@ -538,5 +553,4 @@ private static boolean equalsWithNullCheck(Object a, Object b) { return a == b || (a != null && a.equals(b)); } - -} +} ======================================= --- /trunk/user/test/com/google/gwt/user/server/rpc/ObjectGraphTestServiceImpl.java Mon Jul 6 16:17:17 2009 +++ /trunk/user/test/com/google/gwt/user/server/rpc/ObjectGraphTestServiceImpl.java Tue Nov 3 08:52:16 2009 @@ -18,6 +18,7 @@ import com.google.gwt.user.client.rpc.ObjectGraphTestService; import com.google.gwt.user.client.rpc.TestSetValidator; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode; +import com.google.gwt.user.client.rpc.TestSetFactory.SerializableGraphWithCFS; import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg; import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays; @@ -41,6 +42,15 @@ if (!TestSetValidator.isValidComplexCyclicGraph(root)) { throw new RuntimeException(); } + + return root; + } + + public SerializableGraphWithCFS echo_ComplexCyclicGraphWithCFS( + SerializableGraphWithCFS root) { + if (!TestSetValidator.isValidComplexCyclicGraphWithCFS(root)) { + throw new RuntimeException(); + } return root; } --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
