Fix issue with two RequestFactories with two different Proxies on the same domain class Review at https://gwt-code-reviews.appspot.com/1712803/ Contributed by alexandre.ardhuin.dw Fixes issue 7381 Review by: cromwellian@google.com git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11258 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/requestfactory/build.xml b/requestfactory/build.xml index 9769aa0..9215bfe 100755 --- a/requestfactory/build.xml +++ b/requestfactory/build.xml
@@ -122,6 +122,8 @@ <arg value="com.google.web.bindery.requestfactory.shared.BoxesAndPrimitivesTest.Factory" /> <arg value="com.google.web.bindery.requestfactory.shared.ComplexKeysTest.Factory" /> <arg value="com.google.web.bindery.requestfactory.shared.LocatorTest.Factory" /> + <arg value="com.google.web.bindery.requestfactory.shared.MultipleFactoriesTest.Factory1" /> + <arg value="com.google.web.bindery.requestfactory.shared.MultipleFactoriesTest.Factory2" /> <arg value="com.google.web.bindery.requestfactory.shared.ServiceInheritanceTest$Factory" /> <arg value="com.google.web.bindery.requestfactory.shared.SimpleRequestFactory" /> </java>
diff --git a/user/src/com/google/web/bindery/requestfactory/vm/impl/ClassComparator.java b/user/src/com/google/web/bindery/requestfactory/vm/impl/ClassComparator.java new file mode 100644 index 0000000..a3f142b --- /dev/null +++ b/user/src/com/google/web/bindery/requestfactory/vm/impl/ClassComparator.java
@@ -0,0 +1,55 @@ +/* + * Copyright 2012 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.web.bindery.requestfactory.vm.impl; + +import java.util.Comparator; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Orders classes by assignability, with most-derived types ordered first, and then by name. + */ +class ClassComparator implements Comparator<String> { + private static final Logger log = Logger.getLogger(ClassComparator.class.getName()); + + private final ClassLoader resolveClassesWith; + + public ClassComparator(ClassLoader resolveClassesWith) { + this.resolveClassesWith = resolveClassesWith; + } + + @Override + public int compare(String className1, String className2) { + Class<?> class1 = forName(className1); + Class<?> class2 = forName(className2); + if (class1.equals(class2)) { + return 0; + } else if (class1.isAssignableFrom(class2)) { + return 1; + } else if (class2.isAssignableFrom(class1)) { + return -1; + } + return className1.compareTo(className2); + } + + private Class<?> forName(String name) { + try { + return Class.forName(name, false, resolveClassesWith); + } catch (ClassNotFoundException e) { + String msg = "Could not locate class " + name; + log.log(Level.SEVERE, msg, e); + throw new RuntimeException(msg, e); + } + } +};
diff --git a/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java b/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java index 5c16de1..c70b63e 100644 --- a/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java +++ b/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
@@ -1,16 +1,14 @@ /* * Copyright 2011 Google Inc. * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under * the License. */ package com.google.web.bindery.requestfactory.vm.impl; @@ -22,11 +20,11 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeSet; /** - * Provides access to payload deobfuscation services for server and JVM-based - * clients. The deobfuscation data is baked into GWT-based clients by the - * generator. + * Provides access to payload deobfuscation services for server and JVM-based clients. The + * deobfuscation data is baked into GWT-based clients by the generator. */ public class Deobfuscator { /** @@ -34,9 +32,8 @@ */ public static class Builder { /** - * Load a pre-computed Builder from the classpath. The builder - * implementation is expected to have been generated by the annotation - * processor as part of the build process. + * Load a pre-computed Builder from the classpath. The builder implementation is expected to + * have been generated by the annotation processor as part of the build process. * * @see com.google.web.bindery.requestfactory.apt.DeobfuscatorBuilder * @see com.google.web.bindery.requestfactory.server.ResolverServiceLayer @@ -54,6 +51,7 @@ } Class<? extends Builder> builderClass = found.asSubclass(Builder.class); Builder builder = builderClass.newInstance(); + builder.resolveClassesWith = resolveClassesWith; return builder; } catch (ClassNotFoundException e) { throw new RuntimeException("The RequestFactory ValidationTool must be run for the " @@ -66,6 +64,8 @@ throw new RuntimeException(ex); } + private ClassLoader resolveClassesWith; + private Deobfuscator d = new Deobfuscator(); { @@ -86,7 +86,7 @@ } public Builder merge(Deobfuscator existing) { - d.domainToClientType.putAll(existing.domainToClientType); + d.domainToClientType.putAll(merge(d.domainToClientType, existing.domainToClientType)); d.operationData.putAll(existing.operationData); // referencedTypes recomputed in build() d.typeTokens.putAll(existing.typeTokens); @@ -118,14 +118,51 @@ d.typeTokens.put(token, binaryName); return this; } + + /** + * Merges two domainToClientType into one. Merged map's values are still ordering by + * assignability, with most-derived types ordered first. + * + * @see ClassComparator + */ + private Map<String, List<String>> merge(Map<String, List<String>> domainToClientType1, + Map<String, List<String>> domainToClientType2) { + Map<String, List<String>> result = new HashMap<String, List<String>>(); + Set<String> domains = new HashSet<String>(); + domains.addAll(domainToClientType1.keySet()); + domains.addAll(domainToClientType2.keySet()); + for (String domain : domains) { + List<String> clientTypes1 = domainToClientType1.get(domain); + List<String> clientTypes2 = domainToClientType2.get(domain); + List<String> clientTypes = mergeClientTypes(clientTypes1, clientTypes2); + result.put(domain, clientTypes); + } + return result; + } + + /** + * Merges two clientType lists into one. Merged values are still ordering by assignability, with + * most-derived types ordered first. + * + * @see ClassComparator + */ + private List<String> mergeClientTypes(List<String> clientTypes1, List<String> clientTypes2) { + Set<String> clientTypes = new TreeSet<String>(new ClassComparator(resolveClassesWith)); + if (clientTypes1 != null) { + clientTypes.addAll(clientTypes1); + } + if (clientTypes2 != null) { + clientTypes.addAll(clientTypes2); + } + return Collections.unmodifiableList(new ArrayList<String>(clientTypes)); + } } private static final String GENERATED_SUFFIX = "DeobfuscatorBuilder"; private static final String GENERATED_SUFFIX_LITE = GENERATED_SUFFIX + "Lite"; /** - * Maps domain types (e.g Foo) to client proxy types (e.g. FooAProxy, - * FooBProxy). + * Maps domain types (e.g Foo) to client proxy types (e.g. FooAProxy, FooBProxy). */ private Map<String, List<String>> domainToClientType; private Map<OperationKey, OperationData> operationData; @@ -139,9 +176,8 @@ } /** - * Returns the client proxy types whose {@code @ProxyFor} is exactly - * {@code binaryTypeName}. Ordered such that the most-derived types will be - * iterated over first. + * Returns the client proxy types whose {@code @ProxyFor} is exactly {@code binaryTypeName}. + * Ordered such that the most-derived types will be iterated over first. */ public List<String> getClientProxies(String binaryTypeName) { return domainToClientType.get(binaryTypeName);
diff --git a/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java b/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java index fb0f93b..0d4db18 100644 --- a/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java +++ b/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java
@@ -16,8 +16,8 @@ package com.google.web.bindery.requestfactory.gwt; import com.google.gwt.junit.tools.GWTTestSuite; -import com.google.web.bindery.requestfactory.gwt.client.RequestBatcherTest; import com.google.web.bindery.requestfactory.gwt.client.FindServiceTest; +import com.google.web.bindery.requestfactory.gwt.client.RequestBatcherTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryChainedContextTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryExceptionHandlerTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryExceptionPropagationTest; @@ -29,6 +29,7 @@ import com.google.web.bindery.requestfactory.shared.ComplexKeysTest; import com.google.web.bindery.requestfactory.shared.FanoutReceiverTest; import com.google.web.bindery.requestfactory.shared.LocatorTest; +import com.google.web.bindery.requestfactory.shared.MultipleFactoriesTest; import com.google.web.bindery.requestfactory.shared.ServiceInheritanceTest; import com.google.web.bindery.requestfactory.shared.impl.RequestPayloadTest; @@ -48,6 +49,7 @@ suite.addTestSuite(FanoutReceiverTest.class); suite.addTestSuite(FindServiceTest.class); suite.addTestSuite(LocatorTest.class); + suite.addTestSuite(MultipleFactoriesTest.class); suite.addTestSuite(RequestFactoryTest.class); suite.addTestSuite(RequestFactoryChainedContextTest.class); suite.addTestSuite(RequestFactoryExceptionHandlerTest.class);
diff --git a/user/test/com/google/web/bindery/requestfactory/server/MultipleFactoriesJreTest.java b/user/test/com/google/web/bindery/requestfactory/server/MultipleFactoriesJreTest.java new file mode 100644 index 0000000..4da66a7 --- /dev/null +++ b/user/test/com/google/web/bindery/requestfactory/server/MultipleFactoriesJreTest.java
@@ -0,0 +1,37 @@ +/* + * Copyright 2012 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.web.bindery.requestfactory.server; + +import com.google.web.bindery.requestfactory.shared.MultipleFactoriesTest; + +/** + * A JRE version of {@link MultipleFactoriesTest}. + */ +public class MultipleFactoriesJreTest extends MultipleFactoriesTest { + + @Override + public String getModuleName() { + return null; + } + + @Override + protected Factory1 createFactory1() { + return RequestFactoryJreTest.createInProcess(Factory1.class); + } + + @Override + protected Factory2 createFactory2() { + return RequestFactoryJreTest.createInProcess(Factory2.class); + } +}
diff --git a/user/test/com/google/web/bindery/requestfactory/shared/MultipleFactoriesTest.java b/user/test/com/google/web/bindery/requestfactory/shared/MultipleFactoriesTest.java new file mode 100644 index 0000000..481aa4e --- /dev/null +++ b/user/test/com/google/web/bindery/requestfactory/shared/MultipleFactoriesTest.java
@@ -0,0 +1,153 @@ +/* + * Copyright 2012 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.web.bindery.requestfactory.shared; + +import com.google.gwt.core.client.GWT; +import com.google.gwt.junit.client.GWTTestCase; +import com.google.web.bindery.event.shared.SimpleEventBus; + +/** + * Contains a set of checks of using multiple request factories simultaneously. + */ +public class MultipleFactoriesTest extends GWTTestCase { + + /** + * The domain type. + */ + protected static class Entity { + static final Entity SINGLETON = new Entity(); + + public String getString1() { + return EXPECTED_STRING_1; + } + + public String getString2() { + return EXPECTED_STRING_2; + } + } + + /** + * The first RequestFactory. + */ + protected interface Factory1 extends RequestFactory { + Context1 context(); + } + + /** + * The second RequestFactory. + */ + protected interface Factory2 extends RequestFactory { + Context2 context(); + } + + /** + * The service method implementations. + */ + protected static class ServiceImpl { + public static Entity getEntity() { + return Entity.SINGLETON; + } + } + + @Service(ServiceImpl.class) + interface Context1 extends RequestContext { + Request<Proxy1> getEntity(); + } + + @Service(ServiceImpl.class) + interface Context2 extends RequestContext { + Request<Proxy2> getEntity(); + } + + @ProxyFor(Entity.class) + interface Proxy1 extends ValueProxy { + String getString1(); + } + @ProxyFor(Entity.class) + interface Proxy2 extends ValueProxy { + String getString2(); + } + + static abstract class TestReceiver<T> extends Receiver<T> { + @Override + public void onFailure(ServerFailure error) { + fail(error.getMessage()); + } + } + + private static final String EXPECTED_STRING_1 = "hello world 1"; + private static final String EXPECTED_STRING_2 = "hello world 2"; + private static final int TEST_DELAY = 5000; + + private Factory1 factory1; + private Factory2 factory2; + + @Override + public String getModuleName() { + return "com.google.web.bindery.requestfactory.gwt.RequestFactorySuite"; + } + + /** + * Tests that the 2 calls with 2 RequestFactory with 2 differents Proxy on the same domain class + * succeed. + */ + public void test() { + delayTestFinish(TEST_DELAY); + context1().getEntity().fire(new TestReceiver<Proxy1>() { + @Override + public void onSuccess(Proxy1 response) { + assertEquals(EXPECTED_STRING_1, response.getString1()); + + // test 2 + context2().getEntity().to(new TestReceiver<Proxy2>() { + @Override + public void onSuccess(Proxy2 response) { + assertEquals(EXPECTED_STRING_2, response.getString2()); + } + }).fire(new TestReceiver<Void>() { + @Override + public void onSuccess(Void response) { + finishTest(); + } + }); + } + }); + } + + protected Factory1 createFactory1() { + Factory1 toReturn = GWT.create(Factory1.class); + toReturn.initialize(new SimpleEventBus()); + return toReturn; + } + + protected Factory2 createFactory2() { + Factory2 toReturn = GWT.create(Factory2.class); + toReturn.initialize(new SimpleEventBus()); + return toReturn; + } + + @Override + protected void gwtSetUp() { + factory1 = createFactory1(); + factory2 = createFactory2(); + } + + private Context1 context1() { + return factory1.context(); + } + + private Context2 context2() { + return factory2.context(); + } +}
diff --git a/user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java b/user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java index 503d026..e928579 100644 --- a/user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java +++ b/user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java
@@ -20,6 +20,7 @@ import com.google.web.bindery.requestfactory.server.FanoutReceiverJreTest; import com.google.web.bindery.requestfactory.server.FindServiceJreTest; import com.google.web.bindery.requestfactory.server.LocatorJreTest; +import com.google.web.bindery.requestfactory.server.MultipleFactoriesJreTest; import com.google.web.bindery.requestfactory.server.RequestFactoryChainedContextJreTest; import com.google.web.bindery.requestfactory.server.RequestFactoryExceptionPropagationJreTest; import com.google.web.bindery.requestfactory.server.RequestFactoryJreTest; @@ -47,6 +48,7 @@ suite.addTestSuite(FanoutReceiverJreTest.class); suite.addTestSuite(FindServiceJreTest.class); suite.addTestSuite(LocatorJreTest.class); + suite.addTestSuite(MultipleFactoriesJreTest.class); suite.addTestSuite(RequestFactoryChainedContextJreTest.class); suite.addTestSuite(RequestFactoryExceptionPropagationJreTest.class); suite.addTestSuite(RequestFactoryJreTest.class);
diff --git a/user/test/com/google/web/bindery/requestfactory/vm/impl/ClassComparatorTest.java b/user/test/com/google/web/bindery/requestfactory/vm/impl/ClassComparatorTest.java new file mode 100644 index 0000000..2732997 --- /dev/null +++ b/user/test/com/google/web/bindery/requestfactory/vm/impl/ClassComparatorTest.java
@@ -0,0 +1,55 @@ +/* + * Copyright 2012 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.web.bindery.requestfactory.vm.impl; + +import junit.framework.TestCase; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.TreeSet; + +/** + * Tests class comparison used to merge {@link Deobfuscator}. + */ +public class ClassComparatorTest extends TestCase { + public void testCompare() { + List<Class<?>> classes = new ArrayList<Class<?>>(); + classes.add(String.class); + classes.add(Integer.class); + classes.add(Object.class); + classes.add(CharSequence.class); + classes.add(Object.class); + classes.add(Number.class); + classes.add(Long.class); + classes.add(Number.class); + + // add class names to treeset + ClassLoader classLoader = this.getClass().getClassLoader(); + TreeSet<String> orderedClasses = new TreeSet<String>(new ClassComparator(classLoader)); + for (Class<?> clazz : classes) { + orderedClasses.add(clazz.getName()); + } + + // check ordering and duplication + assertEquals(6, orderedClasses.size()); + Iterator<String> it = orderedClasses.iterator(); + assertEquals(Integer.class.getName(), it.next()); + assertEquals(Long.class.getName(), it.next()); + assertEquals(Number.class.getName(), it.next()); + assertEquals(String.class.getName(), it.next()); + assertEquals(CharSequence.class.getName(), it.next()); + assertEquals(Object.class.getName(), it.next()); + } +}