Hi Paul: Fundamental problem here is that rewriter ordering is important: a Set obviously doesn't offer that guarantee.
I'd recommend we rejigger this implementation so that the Set-provided rewriters come as an addendum to those provided as "core" by Shindig. If relative ordering of *those* Rewriters is important, we can have pre-core and post-core Sets. Apologies for not having seen this in JIRA; my mailbox has gotten so overstuffed of late that some of the messages get lost in the aether. --j On Thu, Nov 4, 2010 at 3:58 PM, <[email protected]> wrote: > Author: lindner > Date: Thu Nov 4 22:58:54 2010 > New Revision: 1031332 > > URL: http://svn.apache.org/viewvc?rev=1031332&view=rev > Log: > SHINDIG-1456 | Patch from Kai Feng Zhang | Allow modules to add new > rewriters > > Modified: > > > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java > > > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java > > > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/HtmlRendererTest.java > > Modified: > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java?rev=1031332&r1=1031331&r2=1031332&view=diff > > ============================================================================== > --- > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java > (original) > +++ > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java > Thu Nov 4 22:58:54 2010 > @@ -19,13 +19,13 @@ > > package org.apache.shindig.gadgets.render; > > -import com.google.inject.Inject; > -import com.google.inject.name.Named; > +import java.util.Set; > > import org.apache.shindig.gadgets.GadgetContext; > import org.apache.shindig.gadgets.rewrite.GadgetRewriter; > > -import java.util.List; > +import com.google.inject.Inject; > +import com.google.inject.name.Named; > > /** > * Class to provide list of rewriters according to gadget request. > @@ -34,15 +34,14 @@ import java.util.List; > * @since 2.0.0 > */ > public class GadgetRewritersProvider { > - private final List<GadgetRewriter> renderRewriters; > + private final Set<GadgetRewriter> renderRewriters; > > @Inject > - public GadgetRewritersProvider( > - @Named("shindig.rewriters.gadget") List<GadgetRewriter> > renderRewriters) { > + public GadgetRewritersProvider(@Named("shindig.rewriters.gadget") > Set<GadgetRewriter> renderRewriters) { > this.renderRewriters = renderRewriters; > } > > - public List<GadgetRewriter> getRewriters(GadgetContext context) { > + public Set<GadgetRewriter> getRewriters(GadgetContext context) { > return renderRewriters; > } > } > > Modified: > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java?rev=1031332&r1=1031331&r2=1031332&view=diff > > ============================================================================== > --- > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java > (original) > +++ > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java > Thu Nov 4 22:58:54 2010 > @@ -18,12 +18,8 @@ > */ > package org.apache.shindig.gadgets.rewrite; > > -import com.google.common.collect.ImmutableList; > -import com.google.inject.AbstractModule; > -import com.google.inject.Provides; > -import com.google.inject.Singleton; > -import com.google.inject.name.Named; > -import com.google.inject.name.Names; > +import java.util.List; > + > import org.apache.shindig.gadgets.parse.GadgetHtmlParser; > import org.apache.shindig.gadgets.render.CajaResponseRewriter; > import org.apache.shindig.gadgets.render.OpenSocialI18NGadgetRewriter; > @@ -33,7 +29,13 @@ import org.apache.shindig.gadgets.render > import org.apache.shindig.gadgets.rewrite.image.BasicImageRewriter; > import org.apache.shindig.gadgets.servlet.CajaContentRewriter; > > -import java.util.List; > +import com.google.common.collect.ImmutableList; > +import com.google.inject.AbstractModule; > +import com.google.inject.Provides; > +import com.google.inject.Singleton; > +import com.google.inject.multibindings.Multibinder; > +import com.google.inject.name.Named; > +import com.google.inject.name.Names; > > /** > * Guice bindings for the rewrite package. > @@ -45,25 +47,23 @@ public class RewriteModule extends Abstr > bind(ResponseRewriterRegistry.class) > > .annotatedWith(Names.named("shindig.accelerate.response.rewriter.registry")) > .to(AccelResponseRewriterRegistry.class); > + > + configureRewriters(); > + > } > > - @Provides > - @Singleton > - @Named("shindig.rewriters.gadget") > - protected List<GadgetRewriter> provideGadgetRewriters( > - PipelineDataGadgetRewriter pipelineRewriter, > - TemplateRewriter templateRewriter, > - AbsolutePathReferenceRewriter absolutePathRewriter, > - StyleTagExtractorContentRewriter styleTagExtractorRewriter, > - StyleAdjacencyContentRewriter styleAdjacencyRewriter, > - ProxyingContentRewriter proxyingRewriter, > - CajaContentRewriter cajaRewriter, > - SanitizingGadgetRewriter sanitizedRewriter, > - RenderingGadgetRewriter renderingRewriter, > - OpenSocialI18NGadgetRewriter i18nRewriter) { > - return ImmutableList.of(pipelineRewriter, templateRewriter, > - absolutePathRewriter, styleTagExtractorRewriter, > styleAdjacencyRewriter, proxyingRewriter, > - cajaRewriter, sanitizedRewriter, renderingRewriter, i18nRewriter); > + private void configureRewriters() { > + Multibinder<GadgetRewriter> multibinder = > Multibinder.newSetBinder(binder(), GadgetRewriter.class, > Names.named("shindig.rewriters.gadget")); > + multibinder.addBinding().to(PipelineDataGadgetRewriter.class); > + multibinder.addBinding().to(TemplateRewriter.class); > + multibinder.addBinding().to(AbsolutePathReferenceRewriter.class); > + multibinder.addBinding().to(StyleTagExtractorContentRewriter.class); > + multibinder.addBinding().to(StyleAdjacencyContentRewriter.class); > + multibinder.addBinding().to(ProxyingContentRewriter.class); > + multibinder.addBinding().to(CajaContentRewriter.class); > + multibinder.addBinding().to(SanitizingGadgetRewriter.class); > + multibinder.addBinding().to(RenderingGadgetRewriter.class); > + multibinder.addBinding().to(OpenSocialI18NGadgetRewriter.class); > } > > @Provides > > Modified: > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/HtmlRendererTest.java > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/HtmlRendererTest.java?rev=1031332&r1=1031331&r2=1031332&view=diff > > ============================================================================== > --- > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/HtmlRendererTest.java > (original) > +++ > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/HtmlRendererTest.java > Thu Nov 4 22:58:54 2010 > @@ -42,6 +42,7 @@ import java.util.Collection; > import java.util.concurrent.Callable; > > import com.google.common.collect.ImmutableList; > +import com.google.common.collect.ImmutableSet; > > /** > * Tests for HtmlRenderer > @@ -84,7 +85,7 @@ public class HtmlRendererTest { > @Before > public void setUp() throws Exception { > renderer = new HtmlRenderer(preloaderService, proxyRenderer, > - new GadgetRewritersProvider(ImmutableList.of((GadgetRewriter) > captureRewriter)), > + new GadgetRewritersProvider(ImmutableSet.of((GadgetRewriter) > captureRewriter)), > null); > > } > > >
