custom scope is ignored for classes with no dependencies

[Dmitry Skavish]

Well the subject tells it all, but here are some details.

I have root module: RootModule and child module: ChildModule. I create
root injector from RootModule and two child injectors from
ChildModule. Every ChildModule defines its own scope, say ChildScope.
So I annotate some components with ChildScoped. If a component has
injectable dependencies (has constructor with Inject and with
parameters) then it all works as it should, the components are created
in a child injector and added to the scope. The problem starts if a
components (annotated with ChildScope) has constructor with no
arguments or if all its dependencies are from root injector. I think
in this case Guice thinks that it can create it in root injector, but
there is no ChildScope in root injector, so the component gets created
everytime it is asked.

I can fix it by specifying .in(scope) for this component, but I
already annotated the component with ChildScoped once. Why should I
specify the same info again?

I guess the more general question is how can I force a component to be
created in child injector? Thank you!

[Sam Berlin]

I'm not positive, but I believe a way to force it is to specify the binding in the child module.  Even if it's just:

  bind(MyChildClass.class);

Sam

[Dmitry Skavish]

I am specifying it in the child module. It does not seem to matter.
The only thing which matters is component's dependencies. If they all
can be injected from the parent then the component will be created in
parent. I believe this is a bug. If I explicitly bind component in a
child it should be created in a child. I will try to create a test
case for that and post it here.

On Jun 2, 4:53 pm, Sam Berlin <sber...@gmail.com> wrote:
> I'm not positive, but I believe a way to force it is to specify the binding
> in the child module.  Even if it's just:
>
>   bind(MyChildClass.class);
>
> Sam
>

[limpb...@gmail.com]

On Jun 2, 2:00 pm, Dmitry Skavish <skav...@gmail.com> wrote:
> I am specifying it in the child module. It does not seem to matter.
> The only thing which matters is component's dependencies. If they all
> can be injected from the parent then the component will be created in
> parent. I believe this is a bug. If I explicitly bind component in a
> child it should be created in a child. I will try to create a test
> case for that and post it here.

We've got fairly extensive tests for this stuff. Anything that's bound
in a child injector will be created in the child injector, so long as
the child injector exists. I wrote a test for the ChildScope issue; it
demonstrates that an annotated class is created in the child injector
as you've requested.

http://code.google.com/p/google-guice/source/diff?spec=svn993&r=993&format=side&path=/trunk/test/com/google/inject/ParentInjectorTest.java

I admit that this behaviour can be confusing - it's difficult to guess
which injector will own the binding. For that reason I strongly
recommend using explicit bindings for everything that should be
satisfied by child injectors.

The reason I chose this behaviour is that it's the least bad of two
potentially surprising behaviours. Although it can be frustrating that
child injectors share singletons by default, it may be dangerous if
they didn't.

[Dmitry Skavish]

Please take a look at the following test case. There are 4 tests
there. I believe you are talking about testChildInjectorBound, that is
if you bind a class in a child module it won't be retrieved from the
parent. My case is testChildImplicitScope (only this one fails here).
The child here is not scoped even though it is bound in the child
module and the childscope is bound there too.

In two others tests testChildImplicitScopeWithChildDep and
testChildExplicitScope it works fine (the child is scoped) because in
testChildImplicitScopeWithChildDep the child has dependency on another
child's bound class and in testChildExplicitScope I explicitly bind
the child to the scope.

I believe this is bug and not a design decision (correct me if I
wrong). Basically whether a child will be scoped or not depends on its
dependencies! I already annotated this class with ChildScoped, it
should be more than enough.

Please let me know what you think. Thanks!

package guice.test;

import com.google.common.collect.Maps;
import com.google.inject.*;
import junit.framework.TestSuite;
import junit.framework.TestCase;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import java.lang.annotation.Retention;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import java.lang.annotation.Target;
import java.util.Map;

/**
*
*/
public class ChildScopeTest extends TestCase {

public void testChildImplicitScope() {
Injector parent = Guice.createInjector();

Injector childInjector = parent.createChildInjector(new
AbstractModule() {
protected void configure() {
bindScope(ChildScoped.class, new ChildScope());

bind(Child.class).to(ChildImpl.class);
}
});

Child child1 = childInjector.getInstance(Child.class);
Child child2 = childInjector.getInstance(Child.class);
assertSame(child1, child2);
}

public void testChildImplicitScopeWithChildDep() {
Injector parent = Guice.createInjector();

Injector childInjector = parent.createChildInjector(new
AbstractModule() {
protected void configure() {
bindScope(ChildScoped.class, new ChildScope());

bind(Dep.class);
bind(Child.class).to(ChildImpl2.class);
}
});

Child child1 = childInjector.getInstance(Child.class);
Child child2 = childInjector.getInstance(Child.class);
assertSame(child1, child2);
}

public void testChildExplicitScope() {
Injector parent = Guice.createInjector();

Injector childInjector = parent.createChildInjector(new
AbstractModule() {
protected void configure() {
ChildScope childScope = new ChildScope();
bindScope(ChildScoped.class, childScope);

bind(Child.class).to(ChildImpl.class).in(childScope);
}
});

Child child1 = childInjector.getInstance(Child.class);
Child child2 = childInjector.getInstance(Child.class);
assertSame(child1, child2);
}

public void testChildInjectorBound() {
Injector parentInjector = Guice.createInjector();

Injector childInjector = parentInjector.createChildInjector
(new AbstractModule() {
protected void configure() {
ChildScope childScope = new ChildScope();
bindScope(ChildScoped.class, childScope);

bind(Child.class).to(ChildImpl.class).in(childScope);
}
});

try {
parentInjector.getInstance(Child.class);
fail();
} catch (ConfigurationException e) {
// success
}
assertNotNull(childInjector.getInstance(Child.class));
}
}

@Target({TYPE, METHOD}) @Retention(RUNTIME) @ScopeAnnotation
@interface ChildScoped { }

class ChildScope implements Scope {

private final Map<Key<?>, Object> values = Maps.newHashMap();

public <T> Provider<T> scope(final Key<T> key, final Provider<T>
unscoped) {
return new Provider<T>() {
public T get() {
@SuppressWarnings("unchecked")
T current = (T) values.get(key);
if (current == null) {
current = unscoped.get();
values.put(key, current);
}
return current;
}
};
}
}

interface Child {}

@ChildScoped
class ChildImpl implements Child {
}

class Dep {}

@ChildScoped
class ChildImpl2 implements Child {
Dep dep;

@Inject
public ChildImpl2(Dep dep) {
this.dep = dep;
}
}

[limpb...@gmail.com]

Dmitry,

This is indeed a bug. I'm quite sorry about this! Apparently our code
that detects misplaced scopes doesn't fire errors when it's supposed
to. And the absence of an error means that the binding can be
"successfully" created in a parent injector. Oh dear.

I've implemented a fix and the corresponding tests.
http://code.google.com/p/google-guice/source/detail?r=994

If you don't mind running from HEAD, you can crate a .jar that
contains the fix. I'm deeply sorry about this, if only because it's
the first tarnish in our shiny new 2.0 release. I suppose it was
bound to happen eventually.

Cheers,
Jesse

[Dmitry Skavish]

Jesse, thanks for the quick fix! it works as it should now.