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

Simplify ServletScopes.scopeRequest() #680

Open
gissuebot opened this issue Jul 7, 2014 · 1 comment
Open

Simplify ServletScopes.scopeRequest() #680

gissuebot opened this issue Jul 7, 2014 · 1 comment
Labels

Comments

@gissuebot
Copy link

From cowwoc@bbs.darktech.org on January 25, 2012 00:56:37

I spent a week trying to figure out how to get ServletScopes.scopeRequest() working. The only code samples I could find were the unit tests and even then I found the API very confusing (see [1] for a detailed explanation).

We need to differentiate between two types of variables being passed to the Callable:

  1. Injected stuff (configured by the main Module)
  2. User-supplied arguments

This is similar to AssistedInject in that #1 comes from a Module and #2 comes from the user. The problem is that ServletScopes.scopeRequest()'s seedMap forces us to reference #2 in the Module (where it doesn't really belong).

I'd like to propose an alternative API (see attachment) with the following characteristics:

  1. The main module is kept free of user-supplied variables.
  2. Replace seedMap with a child Module. The Binder API is easier to use and more flexible.
  3. In the old API, the user Callable is injected outside the request scope, while call() is invoked inside the request scope. This forces users to inject Provider<Foo> instead of Foo. The new API allows users to inject Foo directly.

What do you think?

[1] The current API is extremely fragile. If the user misconfigures the slightest thing in the Module he'll get a cryptic error or the wrong values will get injected.

Common configuration mistakes:

  1. Define user-supplied variables in seedMap but not in main Module. Guice will complain:

  No implementation for java.lang.String annotated with @com.google.inject.name.Named(value=username) was bound.

  1. If user uses the following code (based on the unit tests):

  bindConstant().annotatedWith(Names.named("username")).to("wrong_value");
  bind(String.class).in(RequestScoped.class);

Username will get value "wrong_value".

  1. The magical formula to get this working is:

* Put user-supplied variables into seedMap.

  • Remove bindConstant() and use the following code in the main Module:

  bind(String.class).annotatedWith(Names.named("username")).to(String.class).in(RequestScoped.class);

Anything else will result in runtime exceptions or the variable getting the wrong value. It's very unintuitive.

Attachment: gist
   Testcase.java

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

@gissuebot
Copy link
Author

From cowwoc@bbs.darktech.org on February 02, 2012 16:46:21

I forgot to attach RequestInjector...

Attachment: gist
   RequestInjector.java

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

No branches or pull requests

1 participant