From 842f351c4b2b3b0a90d9f3bcf164d8fd19aede6c Mon Sep 17 00:00:00 2001 From: Sam Berlin Date: Wed, 9 Jul 2014 20:30:23 -0400 Subject: [PATCH] Add support for OptionalBinder to link to normal bindings of that type if neither setDefault nor setBinding are called. From the javadoc, example: * 

* public class FrameworkModule extends AbstractModule {
*   protected void configure() {
*     OptionalBinder.newOptionalBinder(binder(), Renamer.class);
*   }
* }
* *

With this module, an {@link Optional}{@code } can now be * injected. With no other bindings, the optional will be absent. * Users can specify bindings in one of two ways: * *

Option 1: * 


* public class UserRenamerModule extends AbstractModule {
*   protected void configure() {
*     bind(Renamer.class).to(ReplacingRenamer.class);
*   }
* }
* *

or Option 2: * 


* public class UserRenamerModule extends AbstractModule {
*   protected void configure() {
*     OptionalBinder.newOptionalBinder(binder(), Renamer.class)
*         .setBinding().to(ReplacingRenamer.class);
*   }
* }
* With both options, the {@code Optional} will be present and supply the * ReplacingRenamer. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=70835975 --- .../inject/multibindings/OptionalBinder.java | 79 +++++++++++++------ .../multibindings/OptionalBinderTest.java | 62 +++++++++------ .../google/inject/multibindings/SpiUtils.java | 44 ++++++++--- 3 files changed, 129 insertions(+), 56 deletions(-) diff --git a/extensions/multibindings/src/com/google/inject/multibindings/OptionalBinder.java b/extensions/multibindings/src/com/google/inject/multibindings/OptionalBinder.java index 4d4dfac638..4a8a488ac1 100644 --- a/extensions/multibindings/src/com/google/inject/multibindings/OptionalBinder.java +++ b/extensions/multibindings/src/com/google/inject/multibindings/OptionalBinder.java @@ -76,9 +76,11 @@ * setBinding to a Provider that returns null will not cause OptionalBinder * to fall back to the setDefault binding. * - *

If neither setDefault nor setBinding are called, the optionals will be - * absent. Otherwise, the optionals will return present if they are bound - * to a non-null value. + *

If neither setDefault nor setBinding are called, it will try to link to a + * user-supplied binding of the same type. If no binding exists, the optionals + * will be absent. Otherwise, if a user-supplied binding of that type exists, + * or if setBinding or setDefault are called, the optionals will return present + * if they are bound to a non-null value. * *

Values are resolved at injection time. If a value is bound to a * provider, that provider's get method will be called each time the optional @@ -96,9 +98,18 @@ * } * *

With this module, an {@link Optional}{@code } can now be - * injected. With no other bindings, the optional will be absent. However, - * once a user adds a binding: + * injected. With no other bindings, the optional will be absent. + * Users can specify bindings in one of two ways: * + *

Option 1: + * 


+ * public class UserRenamerModule extends AbstractModule {
+ *   protected void configure() {
+ *     bind(Renamer.class).to(ReplacingRenamer.class);
+ *   }
+ * }
+ * + *

or Option 2: * 


  * public class UserRenamerModule extends AbstractModule {
  *   protected void configure() {
@@ -106,8 +117,8 @@
  *         .setBinding().to(ReplacingRenamer.class);
  *   }
  * }
- * .. then the {@code Optional} will be present and supply the - * ReplacingRenamer. + * With both options, the {@code Optional} will be present and supply the + * ReplacingRenamer. * *

Default values can be supplied using: * 


@@ -127,6 +138,24 @@
  *   }
  * }
* ... which will override the default value. + * + *

If one module uses setDefault the only way to override the default is to use setBinding. + * It is an error for a user to specify the binding without using OptionalBinder if + * setDefault or setBinding are called. For example, + * 


+ * public class FrameworkModule extends AbstractModule {
+ *   protected void configure() {
+ *     OptionalBinder.newOptionalBinder(binder(), Key.get(String.class, LookupUrl.class))
+ *         .setDefault().to(DEFAULT_LOOKUP_URL);
+ *   }
+ * }
+ * public class UserLookupModule extends AbstractModule {
+ *   protected void configure() {
+ *     bind(Key.get(String.class, LookupUrl.class)).to(CUSTOM_LOOKUP_URL);
+ *   } 
+ * }
+ * ... would generate an error, because both the framework and the user are trying to bind + * {@code @LookupUrl String}. * * @author sameb@google.com (Sam Berlin) */ @@ -257,7 +286,7 @@ private RealOptionalBinder(Binder binder, Key typeKey) { */ private void addDirectTypeBinding(Binder binder) { binder.bind(typeKey).toProvider(new RealOptionalBinderProviderWithDependencies(typeKey) { - public T get() { + @Override public T get() { Optional> optional = optionalProviderT.get(); if (optional.isPresent()) { return optional.get().get(); @@ -268,7 +297,7 @@ public T get() { return null; } - public Set> getDependencies() { + @Override public Set> getDependencies() { return dependencies; } }); @@ -286,7 +315,7 @@ public Set> getDependencies() { return binder.bind(actualKey); } - public void configure(Binder binder) { + @Override public void configure(Binder binder) { checkConfiguration(!isInitialized(), "OptionalBinder was already initialized"); binder.bind(optionalProviderKey).toProvider( @@ -297,6 +326,7 @@ public void configure(Binder binder) { RealOptionalBinder.this.binder = null; actualBinding = injector.getExistingBinding(actualKey); defaultBinding = injector.getExistingBinding(defaultKey); + Binding userBinding = injector.getExistingBinding(typeKey); Binding binding = null; if (actualBinding != null) { // TODO(sameb): Consider exposing an option that will allow @@ -306,6 +336,12 @@ public void configure(Binder binder) { binding = actualBinding; } else if (defaultBinding != null) { binding = defaultBinding; + } else if (userBinding != null) { + // If neither the actual or default is set, then we fallback + // to the value bound to the type itself and consider that the + // "actual binding" for the SPI. + binding = userBinding; + actualBinding = userBinding; } if (binding != null) { @@ -321,11 +357,11 @@ public void configure(Binder binder) { } } - public Optional> get() { + @Override public Optional> get() { return optional; } - public Set> getDependencies() { + @Override public Set> getDependencies() { return providerDependencies; } }); @@ -348,7 +384,7 @@ private class RealOptionalKeyProvider super(typeKey); } - public Optional get() { + @Override public Optional get() { Optional> optional = optionalProviderT.get(); if (optional.isPresent()) { return Optional.fromNullable(optional.get().get()); @@ -357,11 +393,12 @@ public Optional get() { } } - public Set> getDependencies() { + @Override public Set> getDependencies() { return dependencies; } @SuppressWarnings("unchecked") + @Override public R acceptExtensionVisitor(BindingTargetVisitor visitor, ProviderInstanceBinding binding) { if (visitor instanceof MultibindingsTargetVisitor) { @@ -371,11 +408,11 @@ public R acceptExtensionVisitor(BindingTargetVisitor visitor, } } - public Key> getKey() { + @Override public Key> getKey() { return optionalKey; } - public Binding getActualBinding() { + @Override public Binding getActualBinding() { if (isInitialized()) { return actualBinding; } else { @@ -384,7 +421,7 @@ public Binding getActualBinding() { } } - public Binding getDefaultBinding() { + @Override public Binding getDefaultBinding() { if (isInitialized()) { return defaultBinding; } else { @@ -393,7 +430,7 @@ public Binding getDefaultBinding() { } } - public boolean containsElement(Element element) { + @Override public boolean containsElement(Element element) { Key elementKey; if (element instanceof Binding) { elementKey = ((Binding) element).getKey(); @@ -445,14 +482,12 @@ public RealOptionalBinderProviderWithDependencies(Object equality) { this.equality = equality; } - @Override - public boolean equals(Object obj) { + @Override public boolean equals(Object obj) { return this.getClass() == obj.getClass() && equality.equals(((RealOptionalBinderProviderWithDependencies) obj).equality); } - @Override - public int hashCode() { + @Override public int hashCode() { return equality.hashCode(); } } diff --git a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java index f4c46ca9ae..d2bceb8ce5 100644 --- a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java +++ b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java @@ -126,10 +126,10 @@ public void testOptionalIsAbsentByDefault() { injector.getInstance(Key.get(optionalOfJavaxProviderString)); assertFalse(optionalJxP.isPresent()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, null); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, null, null); } - public void testAbsentWithUserBoundValue() { + public void testUsesUserBoundValue() { Module module = new AbstractModule() { @Override protected void configure() { OptionalBinder.newOptionalBinder(binder(), String.class); @@ -141,16 +141,22 @@ public void testAbsentWithUserBoundValue() { assertEquals("foo", injector.getInstance(String.class)); Optional optional = injector.getInstance(Key.get(optionalOfString)); - assertFalse(optional.isPresent()); + assertEquals("foo", optional.get()); Optional> optionalP = injector.getInstance(Key.get(optionalOfProviderString)); - assertFalse(optionalP.isPresent()); + assertEquals("foo", optionalP.get().get()); Optional> optionalJxP = injector.getInstance(Key.get(optionalOfJavaxProviderString)); - assertFalse(optionalJxP.isPresent()); + assertEquals("foo", optionalJxP.get().get()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, null); + assertOptionalVisitor(stringKey, + setOf(module), + VisitType.BOTH, + 0, + null, + null, + providerInstance("foo")); } public void testSetDefault() { @@ -175,7 +181,7 @@ public void testSetDefault() { assertTrue(optionalJxP.isPresent()); assertEquals("a", optionalJxP.get().get()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, instance("a"), null); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, instance("a"), null, null); } public void testSetBinding() { @@ -200,7 +206,7 @@ public void testSetBinding() { assertTrue(optionalJxP.isPresent()); assertEquals("a", optionalJxP.get().get()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, instance("a")); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, instance("a"), null); } public void testSetBindingOverridesDefault() { @@ -233,7 +239,8 @@ public void testSetBindingOverridesDefault() { VisitType.BOTH, 0, instance("a"), - instance("b")); + instance("b"), + null); } public void testSpreadAcrossModules() { @@ -274,7 +281,8 @@ public void testSpreadAcrossModules() { VisitType.BOTH, 0, instance("a"), - instance("b")); + instance("b"), + null); } public void testExactSameBindingCollapses_defaults() { @@ -302,7 +310,7 @@ public void testExactSameBindingCollapses_defaults() { assertTrue(optionalJxP.isPresent()); assertEquals("a", optionalJxP.get().get()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, instance("a"), null); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, instance("a"), null, null); } public void testExactSameBindingCollapses_actual() { @@ -330,7 +338,7 @@ public void testExactSameBindingCollapses_actual() { assertTrue(optionalJxP.isPresent()); assertEquals("a", optionalJxP.get().get()); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, instance("a")); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 0, null, instance("a"), null); } public void testDifferentBindingsFail_defaults() { @@ -443,7 +451,8 @@ protected void configure() { VisitType.BOTH, 0, instance("a"), - instance("b")); + instance("b"), + null); } public void testMultipleDifferentOptionals() { @@ -464,10 +473,10 @@ public void testMultipleDifferentOptionals() { assertEquals("b", injector.getInstance(bKey)); assertEquals("c", injector.getInstance(cKey)); - assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 3, instance("a"), null); - assertOptionalVisitor(intKey, setOf(module), VisitType.BOTH, 3, instance(1), null); - assertOptionalVisitor(bKey, setOf(module), VisitType.BOTH, 3, instance("b"), null); - assertOptionalVisitor(cKey, setOf(module), VisitType.BOTH, 3, instance("c"), null); + assertOptionalVisitor(stringKey, setOf(module), VisitType.BOTH, 3, instance("a"), null, null); + assertOptionalVisitor(intKey, setOf(module), VisitType.BOTH, 3, instance(1), null, null); + assertOptionalVisitor(bKey, setOf(module), VisitType.BOTH, 3, instance("b"), null, null); + assertOptionalVisitor(cKey, setOf(module), VisitType.BOTH, 3, instance("c"), null, null); } public void testOptionalIsAppropriatelyLazy() { @@ -533,6 +542,7 @@ public void testLinkedToNullProvidersMakeAbsentValuesAndPresentProviders_default VisitType.BOTH, 0, SpiUtils.providerInstance(null), + null, null); } @@ -563,7 +573,8 @@ public void testLinkedToNullProvidersMakeAbsentValuesAndPresentProviders_actual( VisitType.BOTH, 0, null, - SpiUtils.providerInstance(null)); + SpiUtils.providerInstance(null), + null); } // TODO(sameb): Maybe change this? @@ -595,7 +606,8 @@ public void testLinkedToNullActualDoesntFallbackToDefault() { VisitType.BOTH, 0, instance("a"), - SpiUtils.providerInstance(null)); + SpiUtils.providerInstance(null), + null); } public void testSourceLinesInException() { @@ -699,7 +711,8 @@ public void testModuleOverrideRepeatedInstalls_toInstance() { VisitType.BOTH, 0, instance("A"), - instance("B")); + instance("B"), + null); } public void testModuleOverrideRepeatedInstalls_toKey() { @@ -726,7 +739,8 @@ public void testModuleOverrideRepeatedInstalls_toKey() { VisitType.BOTH, 0, linked(aKey), - linked(bKey)); + linked(bKey), + null); } public void testModuleOverrideRepeatedInstalls_toProviderInstance() { @@ -751,7 +765,8 @@ public void testModuleOverrideRepeatedInstalls_toProviderInstance() { VisitType.BOTH, 0, providerInstance("A"), - providerInstance("B")); + providerInstance("B"), + null); } private static class AStringProvider implements Provider { @@ -785,7 +800,8 @@ public void testModuleOverrideRepeatedInstalls_toProviderKey() { VisitType.BOTH, 0, providerKey(Key.get(AStringProvider.class)), - providerKey(Key.get(BStringProvider.class))); + providerKey(Key.get(BStringProvider.class)), + null); } private static class StringGrabber { diff --git a/extensions/multibindings/test/com/google/inject/multibindings/SpiUtils.java b/extensions/multibindings/test/com/google/inject/multibindings/SpiUtils.java index 95174254af..7e34ffefd8 100644 --- a/extensions/multibindings/test/com/google/inject/multibindings/SpiUtils.java +++ b/extensions/multibindings/test/com/google/inject/multibindings/SpiUtils.java @@ -466,32 +466,43 @@ private static void setModuleTest(Key setKey, TypeLiteral elementType, * @param expectedOtherOptionalBindings the # of other optional bindings we expect to see. * @param expectedDefault the expected default binding, or null if none * @param expectedActual the expected actual binding, or null if none + * @param expectedUserLinkedActual the user binding that is the actual binding, used if + * neither the default nor actual are set and a user binding existed for the type. */ static void assertOptionalVisitor(Key keyType, Iterable modules, VisitType visitType, int expectedOtherOptionalBindings, BindResult expectedDefault, - BindResult expectedActual) { + BindResult expectedActual, + BindResult expectedUserLinkedActual) { if (visitType == null) { fail("must test something"); } if (visitType == BOTH || visitType == INJECTOR) { optionalInjectorTest(keyType, modules, expectedOtherOptionalBindings, expectedDefault, - expectedActual); + expectedActual, expectedUserLinkedActual); } if (visitType == BOTH || visitType == MODULE) { optionalModuleTest(keyType, modules, expectedOtherOptionalBindings, expectedDefault, - expectedActual); + expectedActual, expectedUserLinkedActual); } } @SuppressWarnings("unchecked") - private static void optionalInjectorTest(Key keyType, Iterable modules, - int expectedOtherOptionalBindings, BindResult expectedDefault, - BindResult expectedActual) { + private static void optionalInjectorTest(Key keyType, + Iterable modules, + int expectedOtherOptionalBindings, + BindResult expectedDefault, + BindResult expectedActual, + BindResult expectedUserLinkedActual) { + if (expectedUserLinkedActual != null) { + assertNull("cannot have actual if expecting user binding", expectedActual); + assertNull("cannot have default if expecting user binding", expectedDefault); + } + Key> optionalKey = keyType.ofType(OptionalBinder.optionalOf(keyType.getTypeLiteral())); Injector injector = Guice.createInjector(modules); @@ -510,12 +521,16 @@ private static void optionalInjectorTest(Key keyType, Iterable void optionalInjectorTest(Key keyType, Iterable void optionalModuleTest(Key keyType, Iterable modules, - int expectedOtherOptionalBindings, BindResult expectedDefault, - BindResult expectedActual) { + private static void optionalModuleTest(Key keyType, + Iterable modules, + int expectedOtherOptionalBindings, + BindResult expectedDefault, + BindResult expectedActual, + BindResult expectedUserLinkedActual) { + if (expectedUserLinkedActual != null) { + assertNull("cannot have actual if expecting user binding", expectedActual); + assertNull("cannot have default if expecting user binding", expectedDefault); + } Set elements = ImmutableSet.copyOf(Elements.getElements(modules)); Map, Binding> indexed = index(elements); Key> optionalKey =