Comment #5 on issue 603 by [email protected]:
GuiceServletContextListener.getInjector should have one param:
servletContext
http://code.google.com/p/google-guice/issues/detail?id=603
It would indeed be a much nicer API if getInjector took a ServletContext
parameter. The existing workaround of overriding contextInitialized and
caching the value is clumsy and unintuitive. It also places the burden on
the subclass of ensuring that the methods are invoked in the expected order
and behaving sensibly if they are not.
This is especially critical given that the without this, the obvious way of
getting a ServletContext is to @Inject it into a module, which leads to a
deprecation warning and the possibility of bugs due to it being injected
into singleton contexts.
It would have been really easy to have included this parameter when the
class was first written, now of course the problem is how to include it
without breaking existing users. Perhaps the cleanest option would be:
1. make the existing getInjector method non-abstract and mark it as
deprecated.
2. add a new getInjector(ServletContext)
3. if the old method returns null, call the new method.
4. after a couple of years, kill the old method.
E.g.
public abstract class GuiceServletContextListener
implements ServletContextListener {
static final String INJECTOR_NAME = Injector.class.getName();
public void contextInitialized(ServletContextEvent servletContextEvent) {
final ServletContext servletContext =
servletContextEvent.getServletContext();
// Set the Servletcontext early for those people who are using this
class.
// NOTE(dhanji): This use of the servletContext is deprecated.
GuiceFilter.servletContext = new
WeakReference<ServletContext>(servletContext);
Injector injector = getInjector();
if (injector == null) {
injector = getInjector(servletContext);
}
injector.getInstance(InternalServletModule.BackwardsCompatibleServletContextProvider.class)
.set(servletContext);
servletContext.setAttribute(INJECTOR_NAME, injector);
}
public void contextDestroyed(ServletContextEvent servletContextEvent) {
ServletContext servletContext = servletContextEvent.getServletContext();
servletContext.removeAttribute(INJECTOR_NAME);
}
/**
* This method will be removed soon, please use
getInjector(ServletContext) instead.
*/
@Deprecated
protected Injector getInjector() {
return null;
}
/**
* Override this method to create (or otherwise obtain a reference to)
your
* injector.
*
* @param servletContext the ServletContext of the current servlet.
*/
protected Injector getInjector(ServletContext context) {
return null;
}
}
This has the advantage that it exposes a cleaner API that makes it much
easier to figure out where and how you should obtain the ServletContext
whilst not breaking any existing users.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/google-guice-dev.
For more options, visit https://groups.google.com/d/optout.
