xiaodududu / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

AssistedInject inconsistency for Singleton return types #742

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
FactoryModuleBuilder doesn't respect Singleton annotations of returned types. I 
know this is not the intended usage of FactoryModuleBuilder but due to 
refactorings we encountered this situtation on site. It could have failed early 
instead of creating new types.

Following unit test fails, instead of throwing an exception during injector 
creation:

public class FactoryModuleBuilderSingletonTest {

    @Singleton
    public static class ASingleton {

    }

    public static interface SingletonsFactory {
        ASingleton getSingleton();
    }

    @Test
    public void factoryModuleBuilderShouldRespectSingleton() {
        Injector injector = Guice.createInjector(new AbstractModule() {
            @Override
            protected void configure() {
                install(new FactoryModuleBuilder().build(SingletonsFactory.class));
            }
        });
        assertSame(injector.getInstance(ASingleton.class), injector.getInstance(ASingleton.class));
        assertSame(injector.getInstance(ASingleton.class), injector.getInstance(SingletonsFactory.class).getSingleton());
    }
}

Original issue reported on code.google.com by bahri.ge...@gmail.com on 15 Feb 2013 at 9:57

GoogleCodeExporter commented 9 years ago
I agree outright failing would be better, but I'm kinda scared to try changing 
it.  I'll give it a go over Google's code and see how bad the fallout is.  

Original comment by sberlin on 15 Feb 2013 at 2:34

GoogleCodeExporter commented 9 years ago
FYI, I've fixed this internally.  Once we manage to fix our sync procedure, 
we'll push it out to this repository.

For reference, the change is simply adding:
        Class<? extends Annotation> scope =
            Annotations.findScopeAnnotation(errors, implementation.getRawType());
        if (scope != null) {
          errors.addMessage("Found scope annotation [%s] on implementation class "
              + "[%s] of AssistedInject factory [%s].\nThis is not allowed, please"
              + " remove the scope annotation.",
              scope, implementation.getRawType(), factoryType);
        }

around line 244 of FactoryProvider2.java.

Original comment by sberlin on 28 Feb 2013 at 9:20

GoogleCodeExporter commented 9 years ago
Thanks.

Original comment by bahri.ge...@gmail.com on 29 Mar 2013 at 9:31

GoogleCodeExporter commented 9 years ago
Fixed in 
https://code.google.com/p/google-guice/source/detail?r=45d86df69be98dc64a455e397
ae6c492f803771e

Original comment by cgruber@google.com on 16 May 2013 at 6:43