Make guice-servlet more adaptable

[GoogleCodeExporter]

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 :)

Original issue reported on code.google.com by mccu...@gmail.com on 22 Mar 2011 at 5:51

Attachments:

• GUICE_SERVLET_CODE_VISIBILITY.patch

[GoogleCodeExporter]

Can you describe a little more about what the end goal is?

Original comment by sberlin on 22 Mar 2011 at 6:58

[GoogleCodeExporter]

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.

Original comment by mccu...@gmail.com on 22 Mar 2011 at 8:22

[GoogleCodeExporter]

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)?

Original comment by sberlin on 16 Oct 2011 at 4:37

[GoogleCodeExporter]

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));

Original comment by isaac.q....@gmail.com on 17 Oct 2011 at 10:56

[GoogleCodeExporter]

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.

Original comment by mccu...@gmail.com on 17 Oct 2011 at 11:16

[GoogleCodeExporter]

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.

Original comment by DavidB...@gmail.com on 1 Feb 2012 at 6:48

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

A year has passed, and nothing happened... Is Guice a dead project!?

Original comment by claude.h...@gmail.com on 26 Mar 2012 at 6:50

[GoogleCodeExporter]

Attached latest version of proposed patch, tested locally - this also provides 
a way to solve issue 635

Original comment by mccu...@gmail.com on 29 Jul 2012 at 7:18

Attachments:

• GUICE_618_20120729.txt

[GoogleCodeExporter]

This is a nice patch and I support its integration, but frankly I fail to see 
how it addresses issue 635.

Original comment by cow...@bbs.darktech.org on 23 Sep 2012 at 6:00

[GoogleCodeExporter]

It lets you subclass GuiceFilter with your own custom filter pipeline that 
avoids the static-ness causing issue 635.

Original comment by mccu...@gmail.com on 24 Sep 2012 at 10:44

[GoogleCodeExporter]

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.

Original comment by cow...@bbs.darktech.org on 25 Sep 2012 at 5:12

[GoogleCodeExporter]

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.

Original comment by pillingworthz on 17 Dec 2012 at 11:20

[GoogleCodeExporter]

Hasn't this been fixed?

Original comment by christia...@arcbees.com on 15 Aug 2013 at 7:19

[GoogleCodeExporter]

it seems it won't be fixed in 4.0 either...

Original comment by cristian...@gmail.com on 24 Jan 2014 at 9:28