The nice part about the patch is that MultiBinding results in an Ordered Set
based on the order of the individual bind statements.

So this means a third party module can only add a rewriter at the beginning
of the pipeline (by using a Guice Module that loads before the
RewriterModule) or by adding a rewriter at the end of the pipeline (by
adding a Guice Module that runs after the Rewrite Module)

I suspect that one could also rejigger the order of the rewriters using
Modules.override(), but that remains to be seen.

On Mon, Nov 8, 2010 at 2:41 PM, John Hjelmstad <[email protected]> wrote:

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



-- 
Paul Lindner -- [email protected] -- linkedin.com/in/plindner

Reply via email to