Revision: 1316
Author: sberlin
Date: Sun Oct 24 19:57:48 2010
Log: issue 428 - better error validation for AssistedInject, specifically
with public factories & non-public method return types that can lead to
IllegalAccessErrors.
http://code.google.com/p/google-guice/source/detail?r=1316
Modified:
/trunk/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryModuleBuilderTest.java
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryProvider2Test.java
=======================================
---
/trunk/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
Sat Oct 16 18:31:25 2010
+++
/trunk/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
Sun Oct 24 19:57:48 2010
@@ -197,6 +197,10 @@
Class<F> factoryRawType = (Class) factoryType.getRawType();
try {
+ if(!factoryRawType.isInterface()) {
+ throw errors.addMessage("%s must be an interface.",
factoryRawType).toException();
+ }
+
ImmutableMap.Builder<Method, AssistData> assistDataBuilder =
ImmutableMap.builder();
// TODO: also grab methods from superinterfaces
for (Method method : factoryRawType.getMethods()) {
@@ -213,6 +217,7 @@
throw ce;
}
}
+ validateFactoryReturnType(errors,
returnType.getTypeLiteral().getRawType(), factoryRawType);
List<TypeLiteral<?>> params =
factoryType.getParameterTypes(method);
Annotation[][] paramAnnotations = method.getParameterAnnotations();
int p = 0;
@@ -310,6 +315,16 @@
}
return visitor.visit(binding);
}
+
+ private void validateFactoryReturnType(Errors errors, Class<?>
returnType, Class<?> factoryType) {
+ if (Modifier.isPublic(factoryType.getModifiers())
+ && !Modifier.isPublic(returnType.getModifiers())) {
+ errors.addMessage("%s is public, but has a method that returns a
non-public type: %s. "
+ + "Due to limitations with java.lang.reflect.Proxy, this is not
allowed. "
+ + "Please either make the factory non-public or the return type
public.",
+ factoryType, returnType);
+ }
+ }
/**
* Returns true if the ConfigurationException is due to an error of
TypeLiteral not being fully
=======================================
---
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryModuleBuilderTest.java
Wed Sep 22 20:16:22 2010
+++
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryModuleBuilderTest.java
Sun Oct 24 19:57:48 2010
@@ -434,4 +434,28 @@
@AssistedInject Cat(@Assisted byte a, @Named("catfail") String b) {}
// not a dependency!
@Inject void register(@Named("cat3") String a) {}
}
-}
+
+ public void testFactoryPublicAndReturnTypeNotPublic() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ install(new FactoryModuleBuilder()
+ .implement(Hidden.class, HiddenImpl.class)
+ .build(NotHidden.class));
+ }
+ });
+ } catch(CreationException ce) {
+ assertEquals(NotHidden.class.getName() + " is public, but has a
method that returns a non-public type: "
+ + Hidden.class.getName() + ". Due to limitations with
java.lang.reflect.Proxy, this is not allowed. "
+ + "Please either make the factory non-public or the return type
public.",
+ Iterables.getOnlyElement(ce.getErrorMessages()).getMessage());
+ }
+ }
+
+ interface Hidden {}
+ public static class HiddenImpl implements Hidden {}
+ public interface NotHidden {
+ Hidden create();
+ }
+}
=======================================
---
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryProvider2Test.java
Sat Jul 31 09:08:27 2010
+++
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoryProvider2Test.java
Sun Oct 24 19:57:48 2010
@@ -40,6 +40,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
+...@suppresswarnings("deprecation")
public class FactoryProvider2Test extends TestCase {
public void testAssistedFactory() {
@@ -86,7 +87,7 @@
assertEquals(250, redCamaro.horsePower);
}
- interface Car {}
+ public interface Car {}
interface ColoredCarFactory {
Car create(Color color);
@@ -338,7 +339,6 @@
}
public static class Prius implements Car {
- @SuppressWarnings("unused")
final Color color;
@Inject
@@ -446,9 +446,7 @@
public static class SteeringWheel {}
public static class Fiat implements Car {
- @SuppressWarnings("unused")
private final SteeringWheel steeringWheel;
- @SuppressWarnings("unused")
private final Color color;
@Inject
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
