diff --git a/README.md b/README.md index 75eb54f..7efebaa 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,10 @@ dependency cycle). You *can* do it, and break the cycle, if you exclude the `@Bean` type from the `Injector` bindings using the `@GuiceModule` exclude filters. +## Configurable Options + +* Binding Deduplication - When using `@EnableGuiceModules`, if a Spring `Bean` and a Guice `Binding` both exist for the same type and `Qualifier`, creation of the `Injector` will fail. You may instead prefer to keep Spring's instance of the type instead of receiving this error. To accomplish this, you may set the property `spring.guice.dedupeBindings=true`. + ## Limitations * So far there is no support for the Guice SPI methods in diff --git a/src/main/java/org/springframework/guice/annotation/ModuleRegistryConfiguration.java b/src/main/java/org/springframework/guice/annotation/ModuleRegistryConfiguration.java index b3f995a..9e3e964 100644 --- a/src/main/java/org/springframework/guice/annotation/ModuleRegistryConfiguration.java +++ b/src/main/java/org/springframework/guice/annotation/ModuleRegistryConfiguration.java @@ -14,10 +14,12 @@ package org.springframework.guice.annotation; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.stream.Collectors; import com.google.inject.Binding; import com.google.inject.Guice; @@ -61,7 +63,8 @@ import org.springframework.guice.module.SpringModule; class ModuleRegistryConfiguration implements BeanDefinitionRegistryPostProcessor, ApplicationContextAware, ApplicationListener { - ApplicationContext applicationContext; + private static final String SPRING_GUICE_DEDUPE_BINDINGS_PROPERTY_NAME = "spring.guice.dedup"; + private ApplicationContext applicationContext; private List modules; private ConfigurableListableBeanFactory beanFactory; @@ -88,6 +91,7 @@ class ModuleRegistryConfiguration implements BeanDefinitionRegistryPostProcessor if (injector == null) { injector = Guice.createInjector(modules); } + beanFactory.registerResolvableDependency(Injector.class, injector); beanFactory.registerSingleton("injector", injector); } @@ -95,7 +99,8 @@ class ModuleRegistryConfiguration implements BeanDefinitionRegistryPostProcessor BeanDefinitionRegistry registry) { for (Entry, Binding> entry : bindings.entrySet()) { if (entry.getKey().getTypeLiteral().getRawType().equals(Injector.class) - || "spring-guice".equals(entry.getValue().getSource().toString())) { + || SpringModule.SPRING_GUICE_SOURCE + .equals(entry.getValue().getSource().toString())) { continue; } @@ -113,9 +118,9 @@ class ModuleRegistryConfiguration implements BeanDefinitionRegistryPostProcessor ((ElementSource) source).getDeclaringSource().toString()); } else { - bean.setResourceDescription("spring-guice"); + bean.setResourceDescription(SpringModule.SPRING_GUICE_SOURCE); } - bean.setAttribute("spring-guice", true); + bean.setAttribute(SpringModule.SPRING_GUICE_SOURCE, true); registry.registerBeanDefinition(extractName(key), bean); } @@ -133,36 +138,101 @@ class ModuleRegistryConfiguration implements BeanDefinitionRegistryPostProcessor throws BeansException { modules = new ArrayList(((ConfigurableListableBeanFactory) registry) .getBeansOfType(Module.class).values()); - + modules.add(new SpringModule(this.applicationContext)); Map, Binding> bindings = new HashMap, Binding>(); - for (Element e : Elements.getElements(Stage.TOOL, modules)) { + List elements = Elements.getElements(Stage.TOOL, modules); + if (applicationContext.getEnvironment().getProperty( + SPRING_GUICE_DEDUPE_BINDINGS_PROPERTY_NAME, Boolean.class, false)) { + elements = removeDuplicates(elements); + modules = Collections.singletonList(Elements.getModule(elements)); + } + for (Element e : elements) { if (e instanceof Binding) { Binding binding = (Binding) e; bindings.put(binding.getKey(), binding); - } else if (e instanceof PrivateElements) { + } + else if (e instanceof PrivateElements) { extractPrivateElements(bindings, (PrivateElements) e); } } mapBindings(bindings, registry); - modules.add(new SpringModule(this.applicationContext)); + // This event can be published now and it wont actually be processed until later // (during onRefresh()). There's no other way to get a hook into this phase of the // lifecycle. applicationContext.publishEvent(new CreateInjectorSignalEvent()); } - private void extractPrivateElements(Map, Binding> bindings, PrivateElements privateElements) { + private void extractPrivateElements(Map, Binding> bindings, + PrivateElements privateElements) { List elements = privateElements.getElements(); for (Element e : elements) { - if (e instanceof Binding && privateElements.getExposedKeys().contains(((Binding) e).getKey())) { + if (e instanceof Binding && privateElements.getExposedKeys() + .contains(((Binding) e).getKey())) { Binding binding = (Binding) e; bindings.put(binding.getKey(), binding); - } else if (e instanceof PrivateElements) { + } + else if (e instanceof PrivateElements) { extractPrivateElements(bindings, (PrivateElements) e); } } } + /*** + * Remove guice-sourced bindings in favor of spring-sourced bindings, when both exist + * for a given binding key + */ + protected List removeDuplicates(List elements) { + List duplicateElements = elements.stream() + .filter(e -> e instanceof Binding).map(e -> (Binding) e) + .collect(Collectors.groupingBy(Binding::getKey)).entrySet().stream() + .filter(e -> e.getValue().size() > 1 && e.getValue().stream().anyMatch( + binding -> binding.getSource() != null && binding.getSource() + .toString().contains(SpringModule.SPRING_GUICE_SOURCE))) // find + // duplicates + .flatMap(e -> e.getValue().stream()) + .filter(e -> e.getSource() != null && !e.getSource().toString() + .contains(SpringModule.SPRING_GUICE_SOURCE)) + .collect(Collectors.toList()); + + @SuppressWarnings("unlikely-arg-type") + List dedupedElements = elements.stream().filter(e -> { + if (e instanceof Binding) { + return !duplicateElements + .contains(new SourceComparableBinding((Binding) e)); + } + else { + return true; + } + }).collect(Collectors.toList()); + return dedupedElements; + } + + private static class SourceComparableBinding { + private Binding binding; + + public SourceComparableBinding(Binding binding) { + this.binding = binding; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Binding) { + Binding compareTo = (Binding) obj; + if (compareTo.getSource() != null && this.binding != null) { + return binding.equals(compareTo) + && binding.getSource().equals(compareTo.getSource()); + } + else { + return binding.equals(compareTo); + } + } + else { + return false; + } + } + } + @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { diff --git a/src/main/java/org/springframework/guice/module/SpringModule.java b/src/main/java/org/springframework/guice/module/SpringModule.java index f0b2bfe..66f1f75 100644 --- a/src/main/java/org/springframework/guice/module/SpringModule.java +++ b/src/main/java/org/springframework/guice/module/SpringModule.java @@ -23,6 +23,17 @@ import java.util.Map; import javax.inject.Provider; +import com.google.inject.AbstractModule; +import com.google.inject.Binder; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.ProvisionException; +import com.google.inject.Stage; +import com.google.inject.TypeLiteral; +import com.google.inject.matcher.Matchers; +import com.google.inject.name.Names; +import com.google.inject.spi.ProvisionListener; + import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -31,24 +42,14 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.context.ApplicationContext; import org.springframework.util.ClassUtils; -import com.google.inject.AbstractModule; -import com.google.inject.Binder; -import com.google.inject.Injector; -import com.google.inject.Key; -import com.google.inject.ProvisionException; -import com.google.inject.Scopes; -import com.google.inject.Stage; -import com.google.inject.TypeLiteral; -import com.google.inject.matcher.Matchers; -import com.google.inject.name.Names; -import com.google.inject.spi.ProvisionListener; - /** * @author Dave Syer * */ public class SpringModule extends AbstractModule { + public static final String SPRING_GUICE_SOURCE = "spring-guice"; + private BindingTypeMatcher matcher = new GuiceModuleMetadata(); private Map> bound = new HashMap>(); @@ -96,15 +97,14 @@ public class SpringModule extends AbstractModule { private void bind(ConfigurableListableBeanFactory beanFactory) { for (String name : beanFactory.getBeanDefinitionNames()) { BeanDefinition definition = beanFactory.getBeanDefinition(name); - if(definition.hasAttribute("spring-guice")){ + if (definition.hasAttribute(SPRING_GUICE_SOURCE)) { continue; } if (definition.isAutowireCandidate() && definition.getRole() == AbstractBeanDefinition.ROLE_APPLICATION) { Class type = beanFactory.getType(name); - if(type == null) - { - continue; + if (type == null) { + continue; } final String beanName = name; Provider typeProvider = BeanFactoryProvider.typed(beanFactory, type); @@ -143,17 +143,17 @@ public class SpringModule extends AbstractModule { StageTypeKey stageTypeKey = new StageTypeKey(binder.currentStage(), type); if (this.bound.get(stageTypeKey) == null) { // Only bind one provider for each type - binder.withSource("spring-guice").bind(Key.get(type)) + binder.withSource(SPRING_GUICE_SOURCE).bind(Key.get(type)) .toProvider(typeProvider); this.bound.put(stageTypeKey, typeProvider); } // But allow binding to named beans - binder.withSource("spring-guice").bind(TypeLiteral.get(type)) + binder.withSource(SPRING_GUICE_SOURCE).bind(TypeLiteral.get(type)) .annotatedWith(Names.named(name)).toProvider(namedProvider); } - + private static class StageTypeKey { - + private final Stage stage; private final Type type; @@ -185,7 +185,8 @@ public class SpringModule extends AbstractModule { if (type == null) { if (other.type != null) return false; - } else if (!type.equals(other.type)) + } + else if (!type.equals(other.type)) return false; return true; } diff --git a/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000..35bd2ed --- /dev/null +++ b/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,6 @@ +{"properties": [{ + "name": "spring.guice.dedup", + "type": "java.lang.Boolean", + "description": "When using `@EnableGuiceModules`, if a Spring Bean and a Guice Binding both exist for the same type and Qualifier, the Spring Bean will be kept and the Guice Binding discarded.", + "defaultValue": "false" +}]} \ No newline at end of file diff --git a/src/test/java/org/springframework/guice/AdhocTestSuite.java b/src/test/java/org/springframework/guice/AdhocTestSuite.java new file mode 100644 index 0000000..8a7bd1d --- /dev/null +++ b/src/test/java/org/springframework/guice/AdhocTestSuite.java @@ -0,0 +1,36 @@ +/* + * Copyright 2012-2015 the original author or authors. + * + * 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 org.springframework.guice; + +import org.junit.Ignore; +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.junit.runners.Suite.SuiteClasses; + +import org.springframework.guice.annotation.EnableGuiceModulesTests; + +/** + * A test suite for probing weird ordering problems in the tests. + * + * @author Dave Syer + */ +@RunWith(Suite.class) +@SuiteClasses({ BindingDeduplicationTests.class, EnableGuiceModulesTests.class }) +@Ignore +public class AdhocTestSuite { + +} diff --git a/src/test/java/org/springframework/guice/BindingDeduplicationTests.java b/src/test/java/org/springframework/guice/BindingDeduplicationTests.java new file mode 100644 index 0000000..4c97722 --- /dev/null +++ b/src/test/java/org/springframework/guice/BindingDeduplicationTests.java @@ -0,0 +1,66 @@ +package org.springframework.guice; + +import com.google.inject.AbstractModule; +import com.google.inject.CreationException; +import com.google.inject.Module; + +import org.junit.AfterClass; +import org.junit.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.guice.BindingDeduplicationTests.SomeDependency; +import org.springframework.guice.annotation.EnableGuiceModules; + +import static org.junit.Assert.assertNotNull; + +public class BindingDeduplicationTests { + + @AfterClass + public static void cleanUp() { + System.clearProperty("spring.guice.dedup"); + } + + @Test + public void verifyNoDuplicateBindingErrorWhenDedupeEnabled() { + System.setProperty("spring.guice.dedup", "true"); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + BindingDeduplicationTestsConfig.class); + SomeDependency someDependency = context.getBean(SomeDependency.class); + assertNotNull(someDependency); + context.close(); + } + + @Test(expected = CreationException.class) + public void verifyDuplicateBindingErrorWhenDedupeNotEnabled() { + System.setProperty("spring.guice.dedup", "false"); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + BindingDeduplicationTestsConfig.class); + context.close(); + } + + public static class SomeDependency { + } + +} + +@EnableGuiceModules +@Configuration +class BindingDeduplicationTestsConfig { + + @Bean + public SomeDependency stringBean() { + return new SomeDependency(); + } + + @Bean + public Module module() { + return new AbstractModule() { + @Override + protected void configure() { + bind(SomeDependency.class).asEagerSingleton(); + } + }; + } +} \ No newline at end of file diff --git a/src/test/java/org/springframework/guice/ElementVisitorTests.java b/src/test/java/org/springframework/guice/ElementVisitorTests.java index 2185f82..3545230 100644 --- a/src/test/java/org/springframework/guice/ElementVisitorTests.java +++ b/src/test/java/org/springframework/guice/ElementVisitorTests.java @@ -1,23 +1,9 @@ package org.springframework.guice; -import static org.junit.Assert.assertEquals; - import java.util.List; import javax.inject.Inject; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.guice.ElementVisitorTests.DuplicateBean; -import org.springframework.guice.ElementVisitorTests.ElementVisitorTestGuiceBean; -import org.springframework.guice.ElementVisitorTests.ElementVisitorTestSpringBean; -import org.springframework.guice.annotation.EnableGuiceModules; -import org.springframework.guice.annotation.InjectorFactory; - import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; @@ -26,48 +12,68 @@ import com.google.inject.Stage; import com.google.inject.spi.Element; import com.google.inject.spi.Elements; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.guice.ElementVisitorTests.DuplicateBean; +import org.springframework.guice.ElementVisitorTests.ElementVisitorTestGuiceBean; +import org.springframework.guice.ElementVisitorTests.ElementVisitorTestSpringBean; +import org.springframework.guice.annotation.EnableGuiceModules; +import org.springframework.guice.annotation.InjectorFactory; + +import static org.junit.Assert.assertEquals; + public class ElementVisitorTests { private static AnnotationConfigApplicationContext context; - + @BeforeClass public static void init() { + System.setProperty("spring.guice.dedup", "true"); context = new AnnotationConfigApplicationContext(ElementVisitorTestConfig.class); } - + @AfterClass public static void cleanup() { - if(context != null) { + System.clearProperty("spring.guice.dedup"); + if (context != null) { context.close(); } } @Test public void verifySpringModuleDoesNotBreakWhenUsingElementVisitors() { - ElementVisitorTestSpringBean testSpringBean = context.getBean(ElementVisitorTestSpringBean.class); + ElementVisitorTestSpringBean testSpringBean = context + .getBean(ElementVisitorTestSpringBean.class); assertEquals("spring created", testSpringBean.toString()); - ElementVisitorTestGuiceBean testGuiceBean = context.getBean(ElementVisitorTestGuiceBean.class); + ElementVisitorTestGuiceBean testGuiceBean = context + .getBean(ElementVisitorTestGuiceBean.class); assertEquals("spring created", testGuiceBean.toString()); } - public static class ElementVisitorTestSpringBean { @Override public String toString() { return "default"; } } - + public static class ElementVisitorTestGuiceBean { @Inject ElementVisitorTestSpringBean springBean; + @Override public String toString() { return springBean.toString(); } } - public static class DuplicateBean {} + public static class DuplicateBean { + } } @EnableGuiceModules @@ -76,14 +82,14 @@ class ElementVisitorTestConfig { @Bean public ElementVisitorTestSpringBean testBean() { - return new ElementVisitorTestSpringBean(){ + return new ElementVisitorTestSpringBean() { @Override public String toString() { return "spring created"; } }; } - + @Bean public Module module() { return new AbstractModule() { @@ -94,23 +100,24 @@ class ElementVisitorTestConfig { } }; } - + @Bean public InjectorFactory injectorFactory() { - return new InjectorFactory() { + return new InjectorFactory() { @Override public Injector createInjector(List modules) { List elements = Elements.getElements(Stage.TOOL, modules); - return Guice.createInjector(Stage.PRODUCTION,Elements.getModule(elements)); + return Guice.createInjector(Stage.PRODUCTION, + Elements.getModule(elements)); } }; } - + @Bean public DuplicateBean dupeBean1() { return new DuplicateBean(); } - + @Bean public DuplicateBean dupeBean2() { return new DuplicateBean();