From 86cb3bb7d9b662398dc2b71433a9f6c6a6392b5c Mon Sep 17 00:00:00 2001 From: limpbizkit Date: Wed, 15 Oct 2008 21:25:50 +0000 Subject: Refinements to PrivateModules: - new test cases - support for @Provides methods - error detection on keys that are exposed but not bound Also refactoring ProviderMethods to make this work. I'm beginning to run into a wall with what is possible with implementing private modules as an extension rather than as a part of core. In particular: - I need to hack ProviderMethods so they're not installed in the public scope - I need to hack BindingProcessor to tolerate doubly-bound keys for Private Modules The lack of core-support is also going to prevent private modules from having first-class support in the SPI. Ideally we should be able to have a binding type called "ChildBinding" to which the exposed keys belong. This binding would simply point at the child injector that implements the binding (and possibly the delegate binding also). It's also preventing me from being able to detect a small class of errors - when a child module binds a key that's exposed by one of its sibling modules, we don't detect the binding conflict. git-svn-id: https://google-guice.googlecode.com/svn/trunk@633 d779f126-a31b-0410-b53b-1d3aecad763e --- .../com/google/inject/privatemodules/Exposed.java | 32 ++++++ .../inject/privatemodules/PrivateModule.java | 101 ++++++++++++------ .../inject/privatemodules/PrivateModuleTest.java | 117 +++++++++++++++++++++ 3 files changed, 220 insertions(+), 30 deletions(-) create mode 100644 extensions/privatemodules/src/com/google/inject/privatemodules/Exposed.java (limited to 'extensions') diff --git a/extensions/privatemodules/src/com/google/inject/privatemodules/Exposed.java b/extensions/privatemodules/src/com/google/inject/privatemodules/Exposed.java new file mode 100644 index 00000000..264af8cc --- /dev/null +++ b/extensions/privatemodules/src/com/google/inject/privatemodules/Exposed.java @@ -0,0 +1,32 @@ +/** + * Copyright (C) 2008 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.inject.privatemodules; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import java.lang.annotation.Target; +import java.lang.annotation.Documented; + +/** + * Acccompanies a {@literal @}{@link com.google.inject.Provides Provides} method annotation in a + * private module to indicate that the provided binding is exposed. + * + * @author jessewilson@google.com (Jesse Wilson) + */ +@Target(ElementType.METHOD) @Retention(RUNTIME) @Documented +public @interface Exposed {} diff --git a/extensions/privatemodules/src/com/google/inject/privatemodules/PrivateModule.java b/extensions/privatemodules/src/com/google/inject/privatemodules/PrivateModule.java index e6660b1f..f469e415 100644 --- a/extensions/privatemodules/src/com/google/inject/privatemodules/PrivateModule.java +++ b/extensions/privatemodules/src/com/google/inject/privatemodules/PrivateModule.java @@ -20,27 +20,33 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import com.google.common.collect.Sets; import com.google.inject.Binder; +import com.google.inject.Binding; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.Scope; -import com.google.inject.Scopes; import com.google.inject.Stage; import com.google.inject.TypeLiteral; import com.google.inject.binder.AnnotatedBindingBuilder; import com.google.inject.binder.AnnotatedConstantBindingBuilder; import com.google.inject.binder.LinkedBindingBuilder; +import com.google.inject.internal.ProviderMethod; +import com.google.inject.internal.ProviderMethodsModule; import com.google.inject.internal.SourceProvider; import com.google.inject.internal.UniqueAnnotations; import com.google.inject.matcher.Matcher; +import com.google.inject.spi.DefaultElementVisitor; +import com.google.inject.spi.Element; +import com.google.inject.spi.ElementVisitor; import com.google.inject.spi.Elements; import com.google.inject.spi.Message; import com.google.inject.spi.ModuleWriter; import com.google.inject.spi.TypeConverter; import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.util.List; import java.util.Set; import org.aopalliance.intercept.MethodInterceptor; @@ -69,15 +75,24 @@ public abstract class PrivateModule implements Module { // otherwise we're being run for the private injector } else { checkState(this.privateBinder == null, "Re-entry is not allowed."); - this.privateBinder = binder.skipSources(PrivateModule.class); + privateBinder = binder.skipSources(PrivateModule.class); try { configurePrivateBindings(); + ProviderMethodsModule providerMethodsModule = ProviderMethodsModule.forPrivateModule(this); + for (ProviderMethod providerMethod + : providerMethodsModule.getProviderMethods(privateBinder)) { + providerMethod.configure(privateBinder); + if (providerMethod.getMethod().isAnnotationPresent(Exposed.class)) { + expose(providerMethod.getKey()); + } + } + for (Expose expose : exposes) { expose.initPrivateProvider(binder); } } finally { - this.privateBinder = null; + privateBinder = null; } } } @@ -87,10 +102,17 @@ public abstract class PrivateModule implements Module { Key readyKey = Key.get(Ready.class, UniqueAnnotations.create()); readyProvider = publicBinder.getProvider(readyKey); try { - // gather elements and exposes from the private module by being re-entrant on configure() - final Module privateModule = new ModuleWriter().create(Elements.getElements(this)); + List privateElements = Elements.getElements(this); // reentrant on configure() + Set> privatelyBoundKeys = getBoundKeys(privateElements); + final Module privateModule = new ModuleWriter().create(privateElements); + for (Expose expose : exposes) { - expose.configure(publicBinder); + if (!privatelyBoundKeys.contains(expose.key)) { + publicBinder.addError("Could not expose() at %s%n %s must be explicitly bound.", + expose.source, expose.key); + } else { + expose.configure(publicBinder); + } } // create the private injector while the public injector is injecting its members @@ -101,7 +123,7 @@ public abstract class PrivateModule implements Module { publicInjector.createChildInjector(privateModule); return new Ready(); } - }).in(Scopes.SINGLETON); + }).asEagerSingleton(); } finally { readyProvider = null; @@ -113,19 +135,19 @@ public abstract class PrivateModule implements Module { public abstract void configurePrivateBindings(); - protected void expose(Key key) { + protected final void expose(Key key) { checkState(exposes != null, "Cannot expose %s, private module is not ready"); exposes.add(new Expose(sourceProvider.get(), readyProvider, key)); } - protected ExposedKeyBuilder expose(Class type) { + protected final ExposedKeyBuilder expose(Class type) { checkState(exposes != null, "Cannot expose %s, private module is not ready"); Expose expose = new Expose(sourceProvider.get(), readyProvider, Key.get(type)); exposes.add(expose); return expose; } - protected ExposedKeyBuilder expose(TypeLiteral type) { + protected final ExposedKeyBuilder expose(TypeLiteral type) { checkState(exposes != null, "Cannot expose %s, private module is not ready"); Expose expose = new Expose(sourceProvider.get(), readyProvider, Key.get(type)); exposes.add(expose); @@ -164,7 +186,7 @@ public abstract class PrivateModule implements Module { /** Sets the provider in the private injector, to be used by the public injector */ private void initPrivateProvider(Binder privateBinder) { - privateProvider = privateBinder.getProvider(key); + privateProvider = privateBinder.withSource(source).getProvider(key); } /** Creates a binding in the public binder */ @@ -178,83 +200,102 @@ public abstract class PrivateModule implements Module { } } + /** + * Returns the set of keys bound by {@code elements}. + */ + private Set> getBoundKeys(Iterable elements) { + final Set> privatelyBoundKeys = Sets.newHashSet(); + ElementVisitor visitor = new DefaultElementVisitor() { + public Void visitBinding(Binding command) { + privatelyBoundKeys.add(command.getKey()); + return null; + } + }; + + for (Element element : elements) { + element.acceptVisitor(visitor); + } + + return privatelyBoundKeys; + } + // everything below is copied from AbstractModule - protected Binder binder() { + protected final Binder binder() { return privateBinder; } - protected void bindScope(Class scopeAnnotation, Scope scope) { + protected final void bindScope(Class scopeAnnotation, Scope scope) { privateBinder.bindScope(scopeAnnotation, scope); } - protected LinkedBindingBuilder bind(Key key) { + protected final LinkedBindingBuilder bind(Key key) { return privateBinder.bind(key); } - protected AnnotatedBindingBuilder bind(TypeLiteral typeLiteral) { + protected final AnnotatedBindingBuilder bind(TypeLiteral typeLiteral) { return privateBinder.bind(typeLiteral); } - protected AnnotatedBindingBuilder bind(Class clazz) { + protected final AnnotatedBindingBuilder bind(Class clazz) { return privateBinder.bind(clazz); } - protected AnnotatedConstantBindingBuilder bindConstant() { + protected final AnnotatedConstantBindingBuilder bindConstant() { return privateBinder.bindConstant(); } - protected void install(Module module) { + protected final void install(Module module) { privateBinder.install(module); } - protected void addError(String message, Object... arguments) { + protected final void addError(String message, Object... arguments) { privateBinder.addError(message, arguments); } - protected void addError(Throwable t) { + protected final void addError(Throwable t) { privateBinder.addError(t); } - protected void addError(Message message) { + protected final void addError(Message message) { privateBinder.addError(message); } - protected void requestInjection(Object... objects) { + protected final void requestInjection(Object... objects) { privateBinder.requestInjection(objects); } - protected void requestStaticInjection(Class... types) { + protected final void requestStaticInjection(Class... types) { privateBinder.requestStaticInjection(types); } - protected void bindInterceptor(Matcher> classMatcher, + protected final void bindInterceptor(Matcher> classMatcher, Matcher methodMatcher, MethodInterceptor... interceptors) { privateBinder.bindInterceptor(classMatcher, methodMatcher, interceptors); } - protected void requireBinding(Key key) { + protected final void requireBinding(Key key) { privateBinder.getProvider(key); } - protected void requireBinding(Class type) { + protected final void requireBinding(Class type) { privateBinder.getProvider(type); } - protected Provider getProvider(Key key) { + protected final Provider getProvider(Key key) { return privateBinder.getProvider(key); } - protected Provider getProvider(Class type) { + protected final Provider getProvider(Class type) { return privateBinder.getProvider(type); } - protected void convertToTypes(Matcher> typeMatcher, + protected final void convertToTypes(Matcher> typeMatcher, TypeConverter converter) { privateBinder.convertToTypes(typeMatcher, converter); } - protected Stage currentStage() { + protected final Stage currentStage() { return privateBinder.currentStage(); } } diff --git a/extensions/privatemodules/test/com/google/inject/privatemodules/PrivateModuleTest.java b/extensions/privatemodules/test/com/google/inject/privatemodules/PrivateModuleTest.java index 913f7a82..3fcf1d4d 100644 --- a/extensions/privatemodules/test/com/google/inject/privatemodules/PrivateModuleTest.java +++ b/extensions/privatemodules/test/com/google/inject/privatemodules/PrivateModuleTest.java @@ -17,10 +17,13 @@ package com.google.inject.privatemodules; import com.google.inject.AbstractModule; +import static com.google.inject.Asserts.assertContains; +import com.google.inject.CreationException; import com.google.inject.Guice; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Key; +import com.google.inject.Provides; import com.google.inject.name.Named; import static com.google.inject.name.Names.named; import junit.framework.TestCase; @@ -64,6 +67,120 @@ public class PrivateModuleTest extends TestCase { assertEquals("ii", ab2.b); } + public void testPrivateModulesAndProvidesMethods() { + Injector injector = Guice.createInjector(new AbstractModule() { + protected void configure() { + install(new PrivateModule() { + public void configurePrivateBindings() { + expose(String.class).annotatedWith(named("a")); + } + + @Provides @Named("a") String providePublicA() { + return "i"; + } + + @Provides @Named("b") String providePrivateB() { + return "private"; + } + }); + + install(new PrivateModule() { + public void configurePrivateBindings() {} + + @Provides @Named("a") String providePrivateA() { + return "private"; + } + + @Provides @Exposed @Named("b") String providePublicB() { + return "ii"; + } + }); + } + }); + + assertEquals("i", injector.getInstance(Key.get(String.class, named("a")))); + assertEquals("ii", injector.getInstance(Key.get(String.class, named("b")))); + } + + public void testCannotBindAKeyExportedByASibling() { + try { + Guice.createInjector(new AbstractModule() { + protected void configure() { + install(new PrivateModule() { + public void configurePrivateBindings() { + bind(String.class).toInstance("public"); + expose(String.class); + } + }); + + install(new PrivateModule() { + public void configurePrivateBindings() { + bind(String.class).toInstance("private"); + } + }); + } + }); + fail("KNOWN ISSUE: Binding to 'private' should conflict with binding to 'public'"); + } catch (CreationException expected) { + assertContains(expected.getMessage(), "Cannot bind String"); + } + } + + public void testExposeButNoBind() { + try { + Guice.createInjector(new AbstractModule() { + protected void configure() { + bind(String.class).annotatedWith(named("a")).toInstance("a"); + bind(String.class).annotatedWith(named("b")).toInstance("b"); + + install(new PrivateModule() { + public void configurePrivateBindings() { + expose(AB.class); + } + }); + } + }); + fail("AB was exposed but not bound"); + } catch (CreationException expected) { + assertContains(expected.getMessage(), "Could not expose() at ", + PrivateModuleTest.class.getName(), ".configurePrivateBindings(PrivateModuleTest.java:", + Key.get(AB.class).toString(), " must be explicitly bound."); + } + } + + public void testNestedPrivateInjectors() { + Injector injector = Guice.createInjector(new PrivateModule() { + public void configurePrivateBindings() { + expose(String.class); + + install(new PrivateModule() { + public void configurePrivateBindings() { + bind(String.class).toInstance("nested"); + expose(String.class); + } + }); + } + }); + + assertEquals("nested", injector.getInstance(String.class)); + } + + public void testInstallingRegularModulesFromPrivateModules() { + Injector injector = Guice.createInjector(new PrivateModule() { + public void configurePrivateBindings() { + expose(String.class); + + install(new AbstractModule() { + protected void configure() { + bind(String.class).toInstance("nested"); + } + }); + } + }); + + assertEquals("nested", injector.getInstance(String.class)); + } + static class AB { @Inject @Named("a") String a; @Inject @Named("b") String b; -- cgit v1.2.3