Ensure that primitive values can be used as RequestFactory proxy properties.
Make RequestFactoryInterfaceValidator stricter in handling mismatches.
Issue 5357.
Patch by: bobv
Review by: rjrjr, rchandia
Review at http://gwt-code-reviews.appspot.com/1216801
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9414 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidator.java b/user/src/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidator.java
index 6b64aea..fb10d5e 100644
--- a/user/src/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidator.java
+++ b/user/src/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidator.java
@@ -148,7 +148,7 @@
this.parent = parent;
this.validator = parent.validator;
}
-
+
public void poison(String msg, Object... args) {
poison();
logger.logp(Level.SEVERE, currentType(), currentMethod(),
@@ -173,7 +173,7 @@
toReturn.currentType = type;
return toReturn;
}
-
+
public void spam(String msg, Object... args) {
logger.logp(Level.FINEST, currentType(), currentMethod(),
String.format(msg, args));
@@ -309,7 +309,7 @@
} else {
return;
}
-
+
/*
* The input is a source name, so we need to convert it to an
* internal name. We'll do this by substituting dollar signs for the
@@ -555,6 +555,18 @@
System.exit(validator.isPoisoned() ? 1 : 0);
}
+ static String messageCouldNotFindMethod(Type domainType,
+ List<? extends Method> methods) {
+ StringBuilder sb = new StringBuilder();
+ sb.append(String.format(
+ "Could not find matching method in %s.\nPossible matches:\n",
+ print(domainType)));
+ for (Method domainMethod : methods) {
+ sb.append(" ").append(print(domainMethod)).append("\n");
+ }
+ return sb.toString();
+ }
+
private static String print(Method method) {
StringBuilder sb = new StringBuilder();
sb.append(print(method.getReturnType())).append(" ").append(
@@ -632,6 +644,7 @@
* A map of a type to all types that it could be assigned to.
*/
private final Map<Type, List<Type>> supertypes = new HashMap<Type, List<Type>>();
+
/**
* The type {@link ValueProxy}.
*/
@@ -646,7 +659,7 @@
* Contains vaue types (e.g. Integer).
*/
private final Set<Type> valueTypes = new HashSet<Type>();
-
+
/**
* Maps a domain object to the type returned from its getId method.
*/
@@ -657,13 +670,13 @@
valueTypes.add(Type.getType(clazz));
}
}
-
+
public RequestFactoryInterfaceValidator(Logger logger, Loader loader) {
this.parentLogger = new ErrorContext(logger);
parentLogger.setValidator(this);
this.loader = loader;
}
-
+
/**
* Visible for testing.
*/
@@ -790,7 +803,8 @@
for (RFMethod method : getMethodsInHierarchy(logger, requestContextType)) {
// Ignore methods in RequestContext itself
- if (findCompatibleMethod(logger, requestContextIntf, method, false, true) != null) {
+ if (findCompatibleMethod(logger, requestContextIntf, method, false, true,
+ true) != null) {
continue;
}
@@ -961,7 +975,8 @@
Method searchFor = createDomainMethod(logger, new Method(method.getName(),
returnType, method.getArgumentTypes()));
- RFMethod found = findCompatibleMethod(logger, domainServiceType, searchFor);
+ RFMethod found = findCompatibleServiceMethod(logger, domainServiceType,
+ searchFor);
if (found != null) {
boolean isInstance = isAssignable(logger, instanceRequestIntf,
@@ -1048,7 +1063,7 @@
Method clientPropertyMethod, Type domainType) {
logger = logger.setMethod(clientPropertyMethod);
- findCompatibleMethod(logger, domainType,
+ findCompatiblePropertyMethod(logger, domainType,
createDomainMethod(logger, clientPropertyMethod));
}
@@ -1108,20 +1123,14 @@
* hierarchy that is assignment-compatible with the given Method.
*/
private RFMethod findCompatibleMethod(final ErrorContext logger,
- Type domainType, Method searchFor) {
- return findCompatibleMethod(logger, domainType, searchFor, true, false);
- }
-
- /**
- * Finds a compatible method declaration in <code>domainType</code>'s
- * hierarchy that is assignment-compatible with the given Method.
- */
- private RFMethod findCompatibleMethod(final ErrorContext logger,
Type domainType, Method searchFor, boolean mustFind,
- boolean allowOverloads) {
+ boolean allowOverloads, boolean boxReturnTypes) {
String methodName = searchFor.getName();
Type[] clientArgs = searchFor.getArgumentTypes();
Type clientReturnType = searchFor.getReturnType();
+ if (boxReturnTypes) {
+ clientReturnType = maybeBoxType(clientReturnType);
+ }
// Pull all methods out of the domain type
Map<String, List<RFMethod>> domainLookup = new LinkedHashMap<String, List<RFMethod>>();
for (RFMethod method : getMethodsInHierarchy(logger, domainType)) {
@@ -1155,12 +1164,16 @@
// Check each overloaded name
for (RFMethod domainMethod : methods) {
- // Box any primitive types to simplify compotibility check
Type[] domainArgs = domainMethod.getArgumentTypes();
- for (int i = 0, j = domainArgs.length; i < j; i++) {
- domainArgs[i] = maybeBoxType(domainArgs[i]);
+ Type domainReturnType = domainMethod.getReturnType();
+ if (boxReturnTypes) {
+ /*
+ * When looking for the implementation of a Request<Integer>, we want to
+ * match either int or Integer, so we'll box the domain method's return
+ * type.
+ */
+ domainReturnType = maybeBoxType(domainReturnType);
}
- Type domainReturnType = maybeBoxType(domainMethod.getReturnType());
/*
* Make sure the client args can be passed into the domain args and the
@@ -1175,19 +1188,32 @@
}
}
if (mustFind) {
- StringBuilder sb = new StringBuilder();
- sb.append(String.format(
- "Could not find matching method in %s:\nPossible matches:\n",
- print(domainType)));
- for (RFMethod domainMethod : methods) {
- sb.append(" ").append(print(domainMethod)).append("\n");
- }
- logger.poison(sb.toString());
+ logger.poison(messageCouldNotFindMethod(domainType, methods));
}
return null;
}
/**
+ * Finds a compatible method declaration in <code>domainType</code>'s
+ * hierarchy that is assignment-compatible with the given Method.
+ */
+ private RFMethod findCompatiblePropertyMethod(final ErrorContext logger,
+ Type domainType, Method searchFor) {
+ return findCompatibleMethod(logger, domainType, searchFor, true, false,
+ false);
+ }
+
+ /**
+ * Finds a compatible method declaration in <code>domainType</code>'s
+ * hierarchy that is assignment-compatible with the given Method.
+ */
+ private RFMethod findCompatibleServiceMethod(final ErrorContext logger,
+ Type domainType, Method searchFor) {
+ return findCompatibleMethod(logger, domainType, searchFor, true, false,
+ true);
+ }
+
+ /**
* This looks like it should be a utility method somewhere else, but I can't
* find it.
*/
@@ -1341,14 +1367,6 @@
return true;
}
- // Box primitive types to make this simple
- if (possibleSupertype.getSort() != Type.OBJECT) {
- possibleSupertype = getBoxedType(possibleSupertype);
- }
- if (possibleSubtype.getSort() != Type.OBJECT) {
- possibleSubtype = getBoxedType(possibleSubtype);
- }
-
// Supertype calculation is cached
List<Type> allSupertypes = getSupertypes(logger, possibleSubtype);
return allSupertypes.contains(possibleSupertype);
@@ -1368,7 +1386,8 @@
return true;
}
- private boolean isCollectionType(@SuppressWarnings("unused") ErrorContext logger, Type type) {
+ private boolean isCollectionType(
+ @SuppressWarnings("unused") ErrorContext logger, Type type) {
// keeping the logger arg just for internal consistency for our small minds
return "java/util/List".equals(type.getInternalName())
|| "java/util/Set".equals(type.getInternalName());
diff --git a/user/src/com/google/gwt/requestfactory/server/SimpleRequestProcessor.java b/user/src/com/google/gwt/requestfactory/server/SimpleRequestProcessor.java
index 2835056..2fde11d 100644
--- a/user/src/com/google/gwt/requestfactory/server/SimpleRequestProcessor.java
+++ b/user/src/com/google/gwt/requestfactory/server/SimpleRequestProcessor.java
@@ -186,7 +186,7 @@
processOperationMessages(state, message);
List<Object> decoded = decodeInvocationArguments(state,
message.getInvocations().get(0).getParameters(),
- new Class<?>[]{proxyType}, new Type[]{domainClass});
+ new Class<?>[] {proxyType}, new Type[] {domainClass});
@SuppressWarnings("unchecked")
List<T> toReturn = (List<T>) decoded;
@@ -234,7 +234,8 @@
private AutoBean<ServerFailureMessage> createFailureMessage(
ReportableException e) {
- ServerFailure failure = exceptionHandler.createServerFailure(e.getCause());
+ ServerFailure failure = exceptionHandler.createServerFailure(e.getCause() == null
+ ? e : e.getCause());
AutoBean<ServerFailureMessage> bean = FACTORY.failure();
ServerFailureMessage msg = bean.as();
msg.setExceptionType(failure.getExceptionType());
diff --git a/user/test/com/google/gwt/requestfactory/RequestFactoryJreSuite.java b/user/test/com/google/gwt/requestfactory/RequestFactoryJreSuite.java
index 61d7071..85d7956 100644
--- a/user/test/com/google/gwt/requestfactory/RequestFactoryJreSuite.java
+++ b/user/test/com/google/gwt/requestfactory/RequestFactoryJreSuite.java
@@ -16,6 +16,7 @@
package com.google.gwt.requestfactory;
import com.google.gwt.requestfactory.rebind.model.RequestFactoryModelTest;
+import com.google.gwt.requestfactory.server.BoxesAndPrimitivesJreTest;
import com.google.gwt.requestfactory.server.ComplexKeysJreTest;
import com.google.gwt.requestfactory.server.FindServiceJreTest;
import com.google.gwt.requestfactory.server.LocatorJreTest;
@@ -34,6 +35,7 @@
public static Test suite() {
TestSuite suite = new TestSuite(
"requestfactory package tests that require the JRE");
+ suite.addTestSuite(BoxesAndPrimitivesJreTest.class);
suite.addTestSuite(ComplexKeysJreTest.class);
suite.addTestSuite(FindServiceJreTest.class);
suite.addTestSuite(LocatorJreTest.class);
diff --git a/user/test/com/google/gwt/requestfactory/RequestFactorySuite.java b/user/test/com/google/gwt/requestfactory/RequestFactorySuite.java
index 884d005..72cad85 100644
--- a/user/test/com/google/gwt/requestfactory/RequestFactorySuite.java
+++ b/user/test/com/google/gwt/requestfactory/RequestFactorySuite.java
@@ -22,6 +22,7 @@
import com.google.gwt.requestfactory.client.RequestFactoryTest;
import com.google.gwt.requestfactory.client.RequestFactoryUnicodeEscapingTest;
import com.google.gwt.requestfactory.client.ui.EditorTest;
+import com.google.gwt.requestfactory.shared.BoxesAndPrimitivesTest;
import com.google.gwt.requestfactory.shared.ComplexKeysTest;
import com.google.gwt.requestfactory.shared.LocatorTest;
@@ -34,6 +35,7 @@
public static Test suite() {
GWTTestSuite suite = new GWTTestSuite(
"Test suite for requestfactory gwt code.");
+ suite.addTestSuite(BoxesAndPrimitivesTest.class);
suite.addTestSuite(ComplexKeysTest.class);
suite.addTestSuite(EditorTest.class);
suite.addTestSuite(FindServiceTest.class);
diff --git a/user/test/com/google/gwt/requestfactory/server/BoxesAndPrimitivesJreTest.java b/user/test/com/google/gwt/requestfactory/server/BoxesAndPrimitivesJreTest.java
new file mode 100644
index 0000000..c8b178f
--- /dev/null
+++ b/user/test/com/google/gwt/requestfactory/server/BoxesAndPrimitivesJreTest.java
@@ -0,0 +1,123 @@
+/*
+ * 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.gwt.requestfactory.server;
+
+import com.google.gwt.dev.asm.Type;
+import com.google.gwt.dev.asm.commons.Method;
+import com.google.gwt.requestfactory.server.RequestFactoryInterfaceValidatorTest.VisibleErrorContext;
+import com.google.gwt.requestfactory.shared.BoxesAndPrimitivesTest;
+import com.google.gwt.requestfactory.shared.EntityProxy;
+import com.google.gwt.requestfactory.shared.ProxyFor;
+import com.google.gwt.requestfactory.shared.Request;
+import com.google.gwt.requestfactory.shared.RequestContext;
+import com.google.gwt.requestfactory.shared.Service;
+
+import java.util.Arrays;
+import java.util.logging.Logger;
+
+/**
+ * A JRE version of {@link BoxesAndPrimitivesTest} with additional validation
+ * tests.
+ */
+public class BoxesAndPrimitivesJreTest extends BoxesAndPrimitivesTest {
+
+ @Service(ServiceImpl.class)
+ interface ContextMismatchedParameterA extends RequestContext {
+ Request<Void> checkBoxed(int value);
+ }
+
+ @Service(ServiceImpl.class)
+ interface ContextMismatchedParameterB extends RequestContext {
+ Request<Void> checkPrimitive(Integer value);
+ }
+
+ @ProxyFor(Entity.class)
+ interface ProxyMismatchedGetterA extends EntityProxy {
+ int getBoxed();
+ }
+
+ @ProxyFor(Entity.class)
+ interface ProxyMismatchedGetterB extends EntityProxy {
+ Integer getPrimitive();
+ }
+
+ private VisibleErrorContext errors;
+ private RequestFactoryInterfaceValidator v;
+
+ @Override
+ public String getModuleName() {
+ return null;
+ }
+
+ public void test() {
+ RequestFactoryInterfaceValidator v = new RequestFactoryInterfaceValidator(
+ Logger.getAnonymousLogger(),
+ new RequestFactoryInterfaceValidator.ClassLoaderLoader(
+ getClass().getClassLoader()));
+ v.validateRequestFactory(Factory.class.getName());
+ assertFalse(v.isPoisoned());
+ }
+
+ /**
+ * Tests that mismatched primitive verses boxed getters are correctly
+ * reported.
+ */
+ public void testMismatchedGetters() {
+ v.validateEntityProxy(ProxyMismatchedGetterA.class.getName());
+ v.validateEntityProxy(ProxyMismatchedGetterB.class.getName());
+ assertTrue(v.isPoisoned());
+
+ String getBoxedMessage = RequestFactoryInterfaceValidator.messageCouldNotFindMethod(
+ Type.getType(Entity.class),
+ Arrays.asList(new Method("getBoxed", "()Ljava/lang/Integer;")));
+ String getPrimitiveMessage = RequestFactoryInterfaceValidator.messageCouldNotFindMethod(
+ Type.getType(Entity.class),
+ Arrays.asList(new Method("getPrimitive", "()I")));
+ assertEquals(Arrays.asList(getBoxedMessage, getPrimitiveMessage),
+ errors.logs);
+ }
+
+ /**
+ * Tests that mismatched parameter types are correctly reported.
+ */
+ public void testMismatchedParameters() {
+ v.validateRequestContext(ContextMismatchedParameterA.class.getName());
+ v.validateRequestContext(ContextMismatchedParameterB.class.getName());
+
+ String checkBoxedMessage = RequestFactoryInterfaceValidator.messageCouldNotFindMethod(
+ Type.getType(ServiceImpl.class),
+ Arrays.asList(new Method("checkBoxed", "(Ljava/lang/Integer;)V")));
+ String checkPrimitiveMessage = RequestFactoryInterfaceValidator.messageCouldNotFindMethod(
+ Type.getType(ServiceImpl.class),
+ Arrays.asList(new Method("checkPrimitive", "(I)V")));
+ assertEquals(Arrays.asList(checkBoxedMessage, checkPrimitiveMessage),
+ errors.logs);
+ }
+
+ @Override
+ protected Factory createFactory() {
+ return RequestFactoryJreTest.createInProcess(Factory.class);
+ }
+
+ @Override
+ protected void gwtSetUp() throws Exception {
+ super.gwtSetUp();
+ errors = new VisibleErrorContext(Logger.getAnonymousLogger());
+ v = new RequestFactoryInterfaceValidator(errors,
+ new RequestFactoryInterfaceValidator.ClassLoaderLoader(
+ getClass().getClassLoader()));
+ }
+}
diff --git a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
index afcd18e..be2632b 100644
--- a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
+++ b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
@@ -783,7 +783,7 @@
this.simpleValueField = simpleValueField.get(0);
}
- public void setUnpersisted(Boolean unpersisted) {
+ public void setUnpersisted(boolean unpersisted) {
this.unpersisted = unpersisted;
}
diff --git a/user/test/com/google/gwt/requestfactory/shared/BaseFooProxy.java b/user/test/com/google/gwt/requestfactory/shared/BaseFooProxy.java
index 2e98883..869368e 100644
--- a/user/test/com/google/gwt/requestfactory/shared/BaseFooProxy.java
+++ b/user/test/com/google/gwt/requestfactory/shared/BaseFooProxy.java
@@ -75,7 +75,7 @@
List<SimpleValueProxy> getSimpleValues();
- Boolean getUnpersisted();
+ boolean getUnpersisted();
String getUserName();
@@ -127,7 +127,7 @@
void setSimpleValues(List<SimpleValueProxy> value);
- void setUnpersisted(Boolean unpersisted);
+ void setUnpersisted(boolean unpersisted);
void setUserName(String userName);
}
diff --git a/user/test/com/google/gwt/requestfactory/shared/BoxesAndPrimitivesTest.java b/user/test/com/google/gwt/requestfactory/shared/BoxesAndPrimitivesTest.java
new file mode 100644
index 0000000..ddcea1d
--- /dev/null
+++ b/user/test/com/google/gwt/requestfactory/shared/BoxesAndPrimitivesTest.java
@@ -0,0 +1,204 @@
+/*
+ * 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.gwt.requestfactory.shared;
+
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.event.shared.SimpleEventBus;
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Contains a set of checks of how primitive and boxed method declarations
+ * interact.
+ */
+public class BoxesAndPrimitivesTest extends GWTTestCase {
+
+ /**
+ * The domain type.
+ */
+ protected static class Entity {
+ static final Entity SINGLETON = new Entity();
+
+ public static Entity findEntity(int id) {
+ return SINGLETON;
+ }
+
+ public Integer getBoxed() {
+ return EXPECTED_BOXED;
+ }
+
+ public int getId() {
+ return 0;
+ }
+
+ public int getPrimitive() {
+ return EXPECTED;
+ }
+
+ public int getVersion() {
+ return 0;
+ }
+
+ public void setBoxed(Integer value) {
+ assertEquals(EXPECTED_BOXED, value);
+ }
+
+ public void setPrimitive(int value) {
+ assertEquals(EXPECTED, value);
+ }
+ }
+
+ /**
+ * The RequestFactory.
+ */
+ protected interface Factory extends RequestFactory {
+ Context context();
+ }
+
+ /**
+ * The service method implementations.
+ */
+ protected static class ServiceImpl {
+ public static void checkBoxed(Integer value) {
+ assertEquals(EXPECTED_BOXED, value);
+ }
+
+ public static void checkPrimitive(int value) {
+ assertEquals(EXPECTED, value);
+ }
+
+ public static Integer getBoxed() {
+ return EXPECTED_BOXED;
+ }
+
+ public static Entity getEntity() {
+ return Entity.SINGLETON;
+ }
+
+ public static int getPrimitive() {
+ return EXPECTED;
+ }
+ }
+
+ @Service(ServiceImpl.class)
+ interface Context extends RequestContext {
+ Request<Void> checkBoxed(Integer value);
+
+ Request<Void> checkPrimitive(int value);
+
+ Request<Integer> getBoxed();
+
+ Request<Proxy> getEntity();
+
+ Request<Integer> getPrimitive();
+ }
+
+ @ProxyFor(Entity.class)
+ interface Proxy extends EntityProxy {
+ Integer getBoxed();
+
+ int getPrimitive();
+
+ void setBoxed(Integer value);
+
+ void setPrimitive(int value);
+ }
+
+ static abstract class TestReceiver<T> extends Receiver<T> {
+ @Override
+ public void onFailure(ServerFailure error) {
+ fail(error.getMessage());
+ }
+ }
+
+ private static final int EXPECTED = 42;
+ private static final Integer EXPECTED_BOXED = Integer.valueOf(EXPECTED);
+ private static final int TEST_DELAY = 5000;
+
+ private Factory factory;
+
+ @Override
+ public String getModuleName() {
+ return "com.google.gwt.requestfactory.RequestFactorySuite";
+ }
+
+ /**
+ * Tests that domain service methods that return a primitive type are upcast
+ * to the boxed type that the generic declaration requires. Also checks that
+ * primitive and boxed property types can be retrieved and that boxed and
+ * primitive method arguments work.
+ */
+ public void testReturnAndParamTypes() {
+ delayTestFinish(TEST_DELAY);
+ Context ctx = context();
+ // Boxed service method
+ ctx.getBoxed().to(new TestReceiver<Integer>() {
+ @Override
+ public void onSuccess(Integer response) {
+ assertEquals(EXPECTED_BOXED, response);
+ }
+ });
+ // Primitive service method
+ ctx.getPrimitive().to(new TestReceiver<Integer>() {
+ @Override
+ public void onSuccess(Integer response) {
+ assertEquals(EXPECTED_BOXED, response);
+ }
+ });
+ // Boxed and primitive properties
+ ctx.getEntity().to(new TestReceiver<Proxy>() {
+ @Override
+ public void onSuccess(Proxy response) {
+ assertEquals(EXPECTED_BOXED, response.getBoxed());
+ assertEquals(EXPECTED, response.getPrimitive());
+ }
+ });
+ // Boxed service argument
+ ctx.checkBoxed(EXPECTED_BOXED).to(new TestReceiver<Void>() {
+ @Override
+ public void onSuccess(Void response) {
+ // OK
+ }
+ });
+ // Primitive service argument
+ ctx.checkPrimitive(EXPECTED).to(new TestReceiver<Void>() {
+ @Override
+ public void onSuccess(Void response) {
+ // OK
+ }
+ });
+ ctx.fire(new TestReceiver<Void>() {
+ @Override
+ public void onSuccess(Void response) {
+ finishTest();
+ }
+ });
+ }
+
+ protected Factory createFactory() {
+ Factory toReturn = GWT.create(Factory.class);
+ toReturn.initialize(new SimpleEventBus());
+ return toReturn;
+ }
+
+ @Override
+ protected void gwtSetUp() throws Exception {
+ factory = createFactory();
+ }
+
+ private Context context() {
+ return factory.context();
+ }
+}
