Issue 618 in google-guice: Make guice-servlet more adaptable
google...@googlecode.com
Owner: ----
Labels: Type-Patch Extension.Servlet
New issue 618 by mccu...@gmail.com: Make guice-servlet more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
We're using guice-servlet in a multi-injector scenario where we want to
chain the different filter/servlet pipelines under a single instance of a
subclass of GuiceFilter. To get this working without co-locating our custom
filter under the same package as GuiceFilter we need to make a couple of
visibility changes (including removing final from an injected field).
The attached patch contains the necessary changes, comments welcome :)
Attachments:
GUICE_SERVLET_CODE_VISIBILITY.patch 1.6 KB
google...@googlecode.com
Comment #1 on issue 618 by sberlin: Make guice-servlet more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Can you describe a little more about what the end goal is?
google...@googlecode.com
Comment #2 on issue 618 by mccu...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Basically we have multiple injectors (each one with servlet modules) and
every injector should participate in filtering/serving up content. So any
incoming request should be passed onto each injector pipeline in turn,
finally delegating to the static web.xml (ie. much like the single injector
case, but abstracted over multiple injectors).
So 1) we want to (re)set the injected pipeline in the GuiceFilter subclass
to use our chained implementation
... and 2) we want to provide our own implementation of FilterPipeline, so
we need access to the interface
This is the cleanest way we've found to use guice-servlet in a
multi-injector scenario.
google...@googlecode.com
Cc: isaac.q....@gmail.com
Comment #3 on issue 618 by sberlin: Make guice-servlet more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Isaac, I think you made some changes relating to this. Can you help Stuart
out here & figure out the best way to do this (which may mean applying the
patch)?
google...@googlecode.com
Comment #4 on issue 618 by isaac.q....@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Instead of changing visibility, can you instead create a composite Filter
that chains the individually created GuiceFilters together?
new ChainedFilter(
injector1.getInstance(GuiceFilter.class),
...
injectorN.getInstance(GuiceFilter.class));
google...@googlecode.com
Comment #5 on issue 618 by mccu...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Unfortunately that won't work for our use case, which involves dynamic
injectors that can be added/removed during the lifetime of the application.
We also want to consider all the injected pipelines before finally falling
back to the static filter chain.
Btw, with the recent changes to trunk we now only require two trivial
changes to the servlet extension:
1) make the FilterPipeline interface public
2) make the GuiceFilter(FilterPipeline) constructor protected
with these simple changes we can customise the pipeline to properly handle
our dynamic environment.
google...@googlecode.com
Comment #6 on issue 618 by DavidB...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
This issue really needs to be fixed. As comment 5 states, with the October
2011 changes, making FilterPipeline public and the
GuiceFilter(FilterPipeline) constructor visible (ideally public, but
protected is workable as well) is sufficient.
The JVM-scope staticness (actually, classloader-scope staticness) of
GuiceFilter is a real blight on the library. The code is in place to not
rely on it accept for backwards compatibility (i.e., dynamic
filterpipelines are supported; the constructor is just not visible). Please
make it visible.
google...@googlecode.com
Comment #7 on issue 618 by claude.h...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Almost a year and still open? Do I need to fork the whole Guice Project?
google...@googlecode.com
Comment #8 on issue 618 by claude.h...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
A year has passed, and nothing happened... Is Guice a dead project!?
google...@googlecode.com
Comment #9 on issue 618 by mccu...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Attached latest version of proposed patch, tested locally - this also
provides a way to solve issue 635
Attachments:
GUICE_618_20120729.txt 1.6 KB
google...@googlecode.com
Comment #10 on issue 618 by cow...@bbs.darktech.org: Make guice-servlet
more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
This is a nice patch and I support its integration, but frankly I fail to
see how it addresses issue 635.
google...@googlecode.com
Comment #11 on issue 618 by mccu...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
It lets you subclass GuiceFilter with your own custom filter pipeline that
avoids the static-ness causing issue 635.
google...@googlecode.com
Comment #12 on issue 618 by cow...@bbs.darktech.org: Make guice-servlet
more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
So you're proposing we leave the static field in place, but override the
method in the subclass to remove its use? That would work, but I'd rather
fix the official implementation than asking everyone to create their own
subclass.
I fully appreciate your need for subclassing GuiceFilter, but I believe
issue 635 is a separate problem that deserves its own solution.
google...@googlecode.com
Comment #13 on issue 618 by pillingworthz: Make guice-servlet more adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
I am getting a similar problem (or it might be more like issue 635) when I
am trying to use GuiceFilter in an OSGi environment where there is a single
Guice bundle shared by multiple web applications. I want to keep my web
applications completely separate. Looking at the comments it seems I too
need to set the injectedPipeline.
The only workaround I can think of is to bundle Guice and Guice Servlet
into my web bundle - thereby partially defeating the point OSGi.
google...@googlecode.com
Comment #14 on issue 618 by queen...@arcbees.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
Hasn't this been fixed?
--
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
google...@googlecode.com
Comment #15 on issue 618 by cristian...@gmail.com: Make guice-servlet more
adaptable
http://code.google.com/p/google-guice/issues/detail?id=618
it seems it won't be fixed in 4.0 either...