Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move constructionContext.removeCurrentReference from ConstructorInjector.construct to ConstructorInjector.provision? #743

Closed
gissuebot opened this issue Jul 7, 2014 · 5 comments

Comments

@gissuebot
Copy link

From mcculls on March 07, 2013 10:54:21

See https://groups.google.com/d/topic/google-guice/OeEEg2ELQ0M/discussion TestCase from https://gist.github.com/electrotype/5108107 :

//-------------------------------------------------------------------
import static org.junit.Assert.*;
 
import java.util.UUID;
 
import org.junit.Test;
 
import com.google.inject.AbstractModule;
import com.google.inject.Binding;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.spi.ProvisionListener;
 
public class ExampleTest
{
    public static class ClassAAA
    {
        private final String id;
        private final boolean myParam;
        private final IClassBBBFactory classBBBFactory;
        private ClassBBB clasBBB;
        
        @Inject
        public ClassAAA(@Assisted boolean myParam,
                        IClassBBBFactory classBBBFactory)
        {
            this.myParam = myParam;
            this.id = UUID.randomUUID().toString();
            this.classBBBFactory = classBBBFactory;
        }
        
        public void init()
        {
            if(this.myParam)
            {
                this.clasBBB = this.classBBBFactory.create(UUID.randomUUID().toString());
            }
        }
        
        public String getId()
        {
            return this.id;
        }
        
        public ClassBBB getClassBBB()
        {
            return this.clasBBB;
        }
    }
    
    public static class ClassBBB
    {
        private final IClassAAAFactory classAAAFactory;
        private ClassAAA clasAAACreatedByClassBBB;
        
        @Inject
        public ClassBBB(@Assisted String myParam,
                        IClassAAAFactory classAAAFactory)
        {
            this.classAAAFactory = classAAAFactory;
        }
        
        public void init()
        {
            // The problem is here. Even if the factory should return a new instance
            // of ClassAAA, it returns the one that is creating ClassBBB!
            this.clasAAACreatedByClassBBB = this.classAAAFactory.create(false);
        }
 
        public ClassAAA getClasAAACreatedByClassBBB()
        {
            return this.clasAAACreatedByClassBBB;
        }
    }
    
    public static interface IClassAAAFactory
    {
        public ClassAAA create(boolean someParam);
    }
    
    public static interface IClassBBBFactory
    {
        public ClassBBB create(String someParam);
    }
    
    @Test
    public void bugGuiceConstructorInjector()
    {
        Injector injector = Guice.createInjector(new AbstractModule()
        {
            @Override
            protected void configure()
            {
                bindListener(new AbstractMatcher<Binding<?>>()
                             {
                                @Override
                      ...

Original issue: http://code.google.com/p/google-guice/issues/detail?id=743

@gissuebot
Copy link
Author

From mcculls on March 21, 2013 08:51:38

Update: I've not seen any issues testing this locally, so I might commit this fix to the sisu-guice branch and let it bake some more.

@gissuebot
Copy link
Author

From sberlin on March 21, 2013 08:55:14

Thanks, Stuart -- I completely forgot about this, so haven't had a chance to test it internally.  I'll give it a go this weekend & see how things fare.

@gissuebot
Copy link
Author

From electrotype on March 30, 2013 07:51:46

Is this fix going to be included in the 3.1.0 release? I have to say I begin to have a lot of lazy init() calls to workaround the issue. Thanks!

@gissuebot
Copy link
Author

From sberlin on March 30, 2013 11:18:31

FYI, things are looking pretty good, I don't see any breakages from the change so far, so I'm inclined to commit it.  FWIW, I rewrote the test a bit so it doesn't require assistedinject.  See below

--

  public void testProvisionIsNotifiedAfterContextsClear() {
    Injector injector = Guice.createInjector(new AbstractModule() {
      @Override
      protected void configure() {
        bindListener(Matchers.any(), new ProvisionListener() {
          @Override
          public <T> void onProvision(ProvisionInvocation<T> provision) {
            Object provisioned = provision.provision();
            if (provisioned instanceof X) {
              ((X)provisioned).init();
            } else if (provisioned instanceof Y) {
              X.createY = false;
              ((Y)provisioned).init();
            }
          }
        });
      }
    });

    X.createY = true;
    X x = injector.getInstance(X.class);
    assertNotSame(x, x.y.x);
    assertFalse("x.ID: " + x.ID + ", x.y.x.iD: " + x.y.x.ID, x.ID == x.y.x.ID);
  }

  private static class X {
    final static Random RND = new Random();
    static boolean createY;

    final int ID = RND.nextInt();
    final Provider<Y> yProvider;
    Y y;

    @Inject X(Provider<Y> yProvider) {
      this.yProvider = yProvider;
    }

    void init() {
      if (createY) {
        this.y = yProvider.get();
      }
    }
  }

  private static class Y {
    final Provider<X> xProvider;
    X x;

    @Inject Y(Provider<X> xProvider) {
      this.xProvider = xProvider;
    }

    void init() {
      this.x = xProvider.get();
    }
  }

@gissuebot
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant