Support chained requests where the returned proxy type is not reachable from the canonical RequestContext. Issue 6641. Patch by: bobv Review by: rjrjr Review at http://gwt-code-reviews.appspot.com/1499810 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10499 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java b/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java index c7bc3f0..fa3d929 100644 --- a/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java +++ b/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
@@ -61,6 +61,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -98,6 +99,12 @@ * Encapsulates all state contained by the AbstractRequestContext. */ protected static class State { + /** + * Supports the case where chained contexts are used and a response comes + * back from the server with a proxy type not reachable from the canonical + * context. + */ + public Set<AbstractRequestContext> appendedContexts; public final AbstractRequestContext canonical; public final DialectImpl dialect; public FanoutReceiver<Void> fanout; @@ -137,8 +144,19 @@ public State(AbstractRequestFactory requestFactory, DialectImpl dialect, AbstractRequestContext canonical) { this.requestFactory = requestFactory; - this.dialect = dialect; this.canonical = canonical; + this.dialect = dialect; + } + + public void addContext(AbstractRequestContext ctx) { + if (appendedContexts == null) { + appendedContexts = Collections.singleton(ctx); + } else { + if (appendedContexts.size() == 1) { + appendedContexts = new LinkedHashSet<AbstractRequestContext>(appendedContexts); + } + appendedContexts.add(ctx); + } } public AbstractRequestContext getCanonicalContext() { @@ -231,7 +249,7 @@ (Class<BaseProxy>) state.invocations.get(0).getRequestData().getReturnType(); SimpleProxyId<BaseProxy> id = getRequestFactory().allocateId(target); - AutoBean<BaseProxy> bean = createProxy(target, id); + AutoBean<BaseProxy> bean = createProxy(target, id, true); // XXX expose this as a proper API ((AbstractAutoBean<?>) bean).setData(result); // AutoBeanCodex.decodeInto(result, bean); @@ -453,7 +471,7 @@ private State state; protected AbstractRequestContext(AbstractRequestFactory factory, Dialect dialect) { - this.state = new State(factory, dialect.create(this), this); + setState(new State(factory, dialect.create(this), this)); } public <T extends RequestContext> T append(T other) { @@ -465,7 +483,7 @@ if (!child.state.isClean()) { throw new IllegalStateException("The provided RequestContext has been changed"); } - child.state = state; + child.setState(state); return other; } @@ -476,22 +494,10 @@ checkLocked(); SimpleProxyId<T> id = state.requestFactory.allocateId(clazz); - AutoBean<T> created = createProxy(clazz, id); + AutoBean<T> created = createProxy(clazz, id, false); return takeOwnership(created); } - /** - * Creates a new proxy with an assigned ID. - */ - public <T extends BaseProxy> AutoBean<T> createProxy(Class<T> clazz, SimpleProxyId<T> id) { - AutoBean<T> created = getAutoBeanFactory().create(clazz); - if (created == null) { - throw new IllegalArgumentException("Unknown proxy type " + clazz.getName()); - } - created.setTag(STABLE_ID, id); - return created; - } - public <T extends BaseProxy> T edit(T object) { return editProxy(object); } @@ -532,8 +538,8 @@ @Override protected RequestData makeRequestData() { // This method is normally generated, hence the ugly constructor - return new RequestData(Constants.FIND_METHOD_OPERATION, - new Object[] {proxyId}, propertyRefs, proxyId.getProxyClass(), null); + return new RequestData(Constants.FIND_METHOD_OPERATION, new Object[] {proxyId}, + propertyRefs, proxyId.getProxyClass(), null); } }; } @@ -642,7 +648,7 @@ */ public boolean isValueType(Class<?> clazz) { return state.requestFactory.isValueType(clazz); - }; + } public void setFireDisabled(boolean disabled) { state.fireDisabled = disabled; @@ -653,6 +659,39 @@ */ protected void addInvocation(AbstractRequest<?> request) { state.dialect.addInvocation(request); + }; + + /** + * Creates a new proxy with an assigned ID. + * + * @param clazz The proxy type + * @param id The id to be assigned to the new proxy + * @param useAppendedContexts if {@code true} use the AutoBeanFactory types + * associated with any contexts that have been passed into + * {@link #append(RequestContext)}. If {@code false}, this method + * will only create proxy types reachable from the implemented + * RequestContext interface. + * @throws IllegalArgumentException if the requested proxy type cannot be + * created + */ + protected <T extends BaseProxy> AutoBean<T> createProxy(Class<T> clazz, SimpleProxyId<T> id, + boolean useAppendedContexts) { + AutoBean<T> created = null; + if (useAppendedContexts) { + for (AbstractRequestContext ctx : state.appendedContexts) { + created = ctx.getAutoBeanFactory().create(clazz); + if (created != null) { + break; + } + } + } else { + created = getAutoBeanFactory().create(clazz); + } + if (created != null) { + created.setTag(STABLE_ID, id); + return created; + } + throw new IllegalArgumentException("Unknown proxy type " + clazz.getName()); } /** @@ -747,7 +786,7 @@ AutoBean<Q> bean = (AutoBean<Q>) state.returnedProxies.get(id); if (bean == null) { Class<Q> proxyClass = id.getProxyClass(); - bean = createProxy(proxyClass, id); + bean = createProxy(proxyClass, id, true); state.returnedProxies.put(id, bean); } @@ -770,7 +809,7 @@ AutoBean<?> parent; if (stableId.isEphemeral()) { // Newly-created object, use a blank object to compare against - parent = createProxy(stableId.getProxyClass(), stableId); + parent = createProxy(stableId.getProxyClass(), stableId, true); // Newly-created objects go into the persist operation bucket operation.setOperation(WriteOperation.PERSIST); @@ -779,7 +818,7 @@ operation.setStrength(Strength.EPHEMERAL); } else if (stableId.isSynthetic()) { // Newly-created object, use a blank object to compare against - parent = createProxy(stableId.getProxyClass(), stableId); + parent = createProxy(stableId.getProxyClass(), stableId, true); // Newly-created objects go into the persist operation bucket operation.setOperation(WriteOperation.PERSIST); @@ -1200,6 +1239,11 @@ state.locked = false; } + private void setState(State state) { + this.state = state; + state.addContext(this); + } + /** * Make the EnityProxy bean edited and owned by this RequestContext. */
diff --git a/user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java b/user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java index ae23ac2..6dcedca 100644 --- a/user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java +++ b/user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java
@@ -203,9 +203,10 @@ } @Override - public <T extends BaseProxy> AutoBean<T> createProxy(Class<T> clazz, SimpleProxyId<T> id) { + protected <T extends BaseProxy> AutoBean<T> createProxy(Class<T> clazz, SimpleProxyId<T> id, + boolean useAppendedContexts) { if (tokenResolver.isReferencedType(clazz.getName())) { - return super.createProxy(clazz, id); + return super.createProxy(clazz, id, useAppendedContexts); } throw new IllegalArgumentException("Unknown proxy type " + clazz.getName()); }
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 fbe2f56..7062b9c 100644 --- a/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java +++ b/user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java
@@ -18,6 +18,7 @@ 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.RequestFactoryChainedContextTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryExceptionHandlerTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryExceptionPropagationTest; import com.google.web.bindery.requestfactory.gwt.client.RequestFactoryPolymorphicTest; @@ -47,6 +48,7 @@ suite.addTestSuite(FindServiceTest.class); suite.addTestSuite(LocatorTest.class); suite.addTestSuite(RequestFactoryTest.class); + suite.addTestSuite(RequestFactoryChainedContextTest.class); suite.addTestSuite(RequestFactoryExceptionHandlerTest.class); suite.addTestSuite(RequestFactoryExceptionPropagationTest.class); suite.addTestSuite(RequestFactoryPolymorphicTest.class);
diff --git a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java new file mode 100644 index 0000000..4606883 --- /dev/null +++ b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java
@@ -0,0 +1,214 @@ +/* + * 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 + * + * 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.gwt.client; + +import com.google.gwt.core.client.GWT; +import com.google.web.bindery.event.shared.SimpleEventBus; +import com.google.web.bindery.requestfactory.shared.ProxyFor; +import com.google.web.bindery.requestfactory.shared.Receiver; +import com.google.web.bindery.requestfactory.shared.Request; +import com.google.web.bindery.requestfactory.shared.RequestContext; +import com.google.web.bindery.requestfactory.shared.RequestFactory; +import com.google.web.bindery.requestfactory.shared.Service; +import com.google.web.bindery.requestfactory.shared.SimpleBarProxy; +import com.google.web.bindery.requestfactory.shared.SimpleBarRequest; +import com.google.web.bindery.requestfactory.shared.SimpleFooProxy; +import com.google.web.bindery.requestfactory.shared.SimpleFooRequest; +import com.google.web.bindery.requestfactory.shared.ValueProxy; + +/** + * Tests various aspects of how {@code RequestContext.append()} behaves. + */ +public class RequestFactoryChainedContextTest extends RequestFactoryTestBase { + /** + * A RequestFactory where the contained RequestContext types have disjoint + * reachable proxy types. + */ + protected interface Factory extends RequestFactory { + ACtx a(); + + BCtx b(); + } + + /** + * Mandatory javadoc comment. + */ + public static class A { + public static A a() { + return new A(); + } + } + + @Service(A.class) + interface ACtx extends RequestContext { + Request<AProxy> a(); + } + + @ProxyFor(A.class) + interface AProxy extends ValueProxy { + } + + /** + * Mandatory javadoc comment. + */ + public static class B { + public static B b() { + return new B(); + } + } + + @Service(B.class) + interface BCtx extends RequestContext { + Request<BProxy> b(); + } + + @ProxyFor(B.class) + interface BProxy extends ValueProxy { + } + + private static final int DELAY_TEST_FINISH = 5000; + + @Override + public String getModuleName() { + return "com.google.web.bindery.requestfactory.gwt.RequestFactorySuite"; + } + + /** + * Basic functional test of the append method. + */ + public void testAppend() { + delayTestFinish(DELAY_TEST_FINISH); + SimpleFooRequest fooCtx1 = req.simpleFooRequest(); + SimpleFooProxy foo1 = fooCtx1.create(SimpleFooProxy.class); + SimpleBarRequest barCtx = fooCtx1.append(req.simpleBarRequest()); + SimpleFooRequest fooCtx2 = barCtx.append(req.simpleFooRequest()); + + assertNotSame(fooCtx1, fooCtx2); + assertSame(foo1, barCtx.edit(foo1)); + assertSame(foo1, fooCtx2.edit(foo1)); + + SimpleBarProxy foo2 = barCtx.create(SimpleBarProxy.class); + assertSame(foo2, fooCtx1.edit(foo2)); + assertSame(foo2, fooCtx2.edit(foo2)); + + SimpleFooProxy foo3 = fooCtx2.create(SimpleFooProxy.class); + assertSame(foo3, fooCtx1.edit(foo3)); + assertSame(foo3, barCtx.edit(foo3)); + + try { + // Throws exception because c3 has already accumulated some state + req.simpleValueContext().append(fooCtx2); + fail("Should have thrown IllegalStateException"); + } catch (IllegalStateException expected) { + } + + try { + // Throws exception because a different RequestFactory instance is used + fooCtx2.append(createFactory().simpleFooRequest()); + fail("Should have thrown IllegalStateException"); + } catch (IllegalStateException expected) { + } + + // Queue up two invocations, and test that both Receivers are called + final boolean[] seen = {false, false}; + fooCtx1.add(1, 2).to(new Receiver<Integer>() { + @Override + public void onSuccess(Integer response) { + seen[0] = true; + assertEquals(3, response.intValue()); + } + }); + barCtx.countSimpleBar().to(new Receiver<Long>() { + @Override + public void onSuccess(Long response) { + seen[1] = true; + assertEquals(2, response.longValue()); + } + }); + + // It doesn't matter which context instance is fired + barCtx.fire(new Receiver<Void>() { + @Override + public void onSuccess(Void response) { + assertTrue(seen[0]); + assertTrue(seen[1]); + finishTestAndReset(); + } + }); + + /* + * Since the common State has been locked, calling any other + * context-mutation methods should fail. + */ + try { + fooCtx1.fire(); + fail("Should have thrown exception"); + } catch (IllegalStateException expected) { + } + try { + fooCtx2.fire(); + fail("Should have thrown exception"); + } catch (IllegalStateException expected) { + } + try { + fooCtx2.create(SimpleFooProxy.class); + fail("Should have thrown exception"); + } catch (IllegalStateException expected) { + } + } + + /** + * Ensure that a method invoked on an appended context can result in the + * creation of a proxy not reachable from canonical context. + */ + public void testChainedProxyInstantiation() { + delayTestFinish(DELAY_TEST_FINISH); + Factory f = createChainedFactory(); + + ACtx aCtx = f.a(); + checkReachableTypes(aCtx, AProxy.class, BProxy.class); + + RequestContext ctx = aCtx.a().to(new Receiver<AProxy>() { + @Override + public void onSuccess(AProxy response) { + assertNotNull(response); + } + }); + + BCtx bCtx = ctx.append(f.b()); + checkReachableTypes(aCtx, AProxy.class, BProxy.class); + checkReachableTypes(bCtx, BProxy.class, AProxy.class); + + bCtx.b().to(new Receiver<BProxy>() { + @Override + public void onSuccess(BProxy response) { + assertNotNull(response); + } + }); + ctx.fire(new Receiver<Void>() { + @Override + public void onSuccess(Void response) { + finishTest(); + } + }); + } + + protected Factory createChainedFactory() { + Factory f = GWT.create(Factory.class); + f.initialize(new SimpleEventBus()); + return f; + } +}
diff --git a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java index 9bcbc55..f9935a4 100644 --- a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java +++ b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
@@ -242,87 +242,6 @@ }); } - public void testAppend() { - delayTestFinish(DELAY_TEST_FINISH); - SimpleFooRequest c1 = req.simpleFooRequest(); - SimpleFooProxy foo1 = c1.create(SimpleFooProxy.class); - SimpleBarRequest c2 = c1.append(req.simpleBarRequest()); - SimpleFooRequest c3 = c2.append(req.simpleFooRequest()); - - assertNotSame(c1, c3); - assertSame(foo1, c2.edit(foo1)); - assertSame(foo1, c3.edit(foo1)); - - SimpleBarProxy foo2 = c2.create(SimpleBarProxy.class); - assertSame(foo2, c1.edit(foo2)); - assertSame(foo2, c3.edit(foo2)); - - SimpleFooProxy foo3 = c3.create(SimpleFooProxy.class); - assertSame(foo3, c1.edit(foo3)); - assertSame(foo3, c2.edit(foo3)); - - try { - // Throws exception because c3 has already accumulated some state - req.simpleValueContext().append(c3); - fail("Should have thrown IllegalStateException"); - } catch (IllegalStateException expected) { - } - - try { - // Throws exception because a different RequestFactory instance is used - c3.append(createFactory().simpleFooRequest()); - fail("Should have thrown IllegalStateException"); - } catch (IllegalStateException expected) { - } - - // Queue up two invocations, and test that both Receivers are called - final boolean[] seen = {false, false}; - c1.add(1, 2).to(new Receiver<Integer>() { - @Override - public void onSuccess(Integer response) { - seen[0] = true; - assertEquals(3, response.intValue()); - } - }); - c2.countSimpleBar().to(new Receiver<Long>() { - @Override - public void onSuccess(Long response) { - seen[1] = true; - assertEquals(2, response.longValue()); - } - }); - - // It doesn't matter which context instance is fired - c2.fire(new Receiver<Void>() { - @Override - public void onSuccess(Void response) { - assertTrue(seen[0]); - assertTrue(seen[1]); - finishTestAndReset(); - } - }); - - /* - * Since the common State has been locked, calling any other - * context-mutation methods should fail. - */ - try { - c1.fire(); - fail("Should have thrown exception"); - } catch (IllegalStateException expected) { - } - try { - c3.fire(); - fail("Should have thrown exception"); - } catch (IllegalStateException expected) { - } - try { - c3.create(SimpleFooProxy.class); - fail("Should have thrown exception"); - } catch (IllegalStateException expected) { - } - } - /** * Test that we can commit child objects. */
diff --git a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java index 43f9681..340c7d4 100644 --- a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java +++ b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java
@@ -27,6 +27,7 @@ import com.google.web.bindery.requestfactory.shared.EntityProxyChange; import com.google.web.bindery.requestfactory.shared.ProxySerializer; import com.google.web.bindery.requestfactory.shared.Receiver; +import com.google.web.bindery.requestfactory.shared.RequestContext; import com.google.web.bindery.requestfactory.shared.SimpleFooRequest; import com.google.web.bindery.requestfactory.shared.SimpleRequestFactory; import com.google.web.bindery.requestfactory.shared.impl.BaseProxyCategory; @@ -87,6 +88,29 @@ } /** + * Check that some proxy type can be created by the given context and that + * some other proxy type can't. + */ + protected void checkReachableTypes(RequestContext ctx, Class<? extends BaseProxy> shouldWork, + Class<? extends BaseProxy> shouldFail) { + assertNotNull(ctx.create(shouldWork)); + try { + // Metadata computation has only RequestFactory resolution + // http://code.google.com/p/google-web-toolkit/issues/detail?id=6658 + if (GWT.isClient()) { + ctx.create(shouldFail); + fail(); + } else { + assertNotNull(ctx.create(shouldFail)); + } + } catch (IllegalArgumentException expected) { + if (!GWT.isClient()) { + fail("Expect the create call to always work in RFSource implementation"); + } + } + } + + /** * Run the given proxy through a ProxySerializer and verify that the * before-and-after values match. */
diff --git a/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryChainedContextJreTest.java b/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryChainedContextJreTest.java new file mode 100644 index 0000000..3b8a9f0 --- /dev/null +++ b/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryChainedContextJreTest.java
@@ -0,0 +1,40 @@ +/* + * Copyright 2010 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.gwt.client.RequestFactoryChainedContextTest; +import com.google.web.bindery.requestfactory.shared.SimpleRequestFactory; + +/** + * Runs the RequestFactoryChainedContext tests in-process. + */ +public class RequestFactoryChainedContextJreTest extends RequestFactoryChainedContextTest { + + @Override + public String getModuleName() { + return null; + } + + @Override + protected Factory createChainedFactory() { + return RequestFactoryJreTest.createInProcess(Factory.class); + } + + @Override + protected SimpleRequestFactory createFactory() { + return RequestFactoryJreTest.createInProcess(SimpleRequestFactory.class); + } +}
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 cbc311f..ba26e92 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.RequestFactoryChainedContextJreTest; import com.google.web.bindery.requestfactory.server.RequestFactoryExceptionPropagationJreTest; import com.google.web.bindery.requestfactory.server.RequestFactoryInterfaceValidatorTest; import com.google.web.bindery.requestfactory.server.RequestFactoryJreTest; @@ -46,6 +47,7 @@ suite.addTestSuite(FanoutReceiverJreTest.class); suite.addTestSuite(FindServiceJreTest.class); suite.addTestSuite(LocatorJreTest.class); + suite.addTestSuite(RequestFactoryChainedContextJreTest.class); suite.addTestSuite(RequestFactoryExceptionPropagationJreTest.class); suite.addTestSuite(RequestFactoryInterfaceValidatorTest.class); suite.addTestSuite(RequestFactoryJreTest.class);