Make guice-servlet more adaptable

[gissuebot]

From mcculls on March 22, 2011 13:51:37

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

Attachment: gist    _GUICE_SERVLET_CODE_VISIBILITY.patch_

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

[gissuebot]

From sberlin on March 22, 2011 11:58:39

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

[gissuebot]

From mcculls on March 22, 2011 13:22:37

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.

[gissuebot]

From sberlin on October 16, 2011 09:37:36

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

Cc: isaac.q.shum

[gissuebot]

From isaac.q.shum on October 17, 2011 15:56:00

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

[gissuebot]

From mcculls on October 17, 2011 16:16:45

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.

[gissuebot]

From DavidBild on January 31, 2012 22:48:15

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.

[gissuebot]

From claude.houle on March 26, 2012 11:50:45

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

[gissuebot]

From mcculls on July 29, 2012 12:18:36

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

Attachment: gist    _GUICE_618_20120729.txt_

[gissuebot]

From cowwoc@bbs.darktech.org on September 22, 2012 23:00:53

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

[gissuebot]

From mcculls on September 24, 2012 03:44:20

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

[gissuebot]

From cowwoc@bbs.darktech.org on September 24, 2012 22:12:50

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.

[gissuebot]

From pillingworthz on December 17, 2012 03:20:52

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.

[gissuebot]

From queenbee@arcbees.com on August 15, 2013 12:19:44

Hasn't this been fixed?

[gissuebot]

From cristian.prevedello on January 24, 2014 01:28:54

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