According to http://google-guice.googlecode.com/svn/trunk/latest-javadoc/com/google/inject/multibindings/Multibinder.html
the ordering is consistent with the binding ordering/sequence: "The set's iteration order is consistent with the binding order" so I guess the ordering issue shouldnt be a problem. - Henry On Thu, Nov 4, 2010 at 1:09 AM, Kai Feng Zhang <[email protected]> wrote: > hi Paul and Gagandeep, > > Seems we have no other options. Could I propose to code review my patch? > https://issues.apache.org/jira/browse/SHINDIG-1456 > > If the new rewriter to be appended is simple enough ,it's does not matter on > the rewriters ordering issue I think. Otherwise we need to provide > Comparable implementation for rewriters. > > > On Tue, Nov 2, 2010 at 12:51 PM, Gagandeep singh <[email protected]>wrote: > >> Hi Kai >> >> Thanks for finding this. Sad that the Modules.override approach had a bug >> :( >> >> On Tue, Nov 2, 2010 at 7:46 AM, Kai Feng Zhang <[email protected]> wrote: >> >>> Seems this is a known issue, I found this: >>> http://code.google.com/p/google-guice/issues/detail?id=263 >>> >>> I am not sure if this fix has gone into Guice 2.0 which is in classpath of >>> Shindig. If it's not, then I think I still need to propose my patch, which >>> will provide capability of extending/appending rewriters for Shindig gadget >>> renderer. >>> >> Would it be possible to go through this >> codereview<http://codereview.appspot.com/2058042/diff/123001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java>( >> maybe patch it in your client and play with it ) to see if it solves your >> problem ? >> >> >>> https://issues.apache.org/jira/browse/SHINDIG-1456 >>> >>> Any comments? Thanks. >>> >>> <http://code.google.com/p/google-guice/issues/detail?id=263> >>> Thanks, >>> >>> Kevin Zhang (凯峰) >>> Gtalk: [email protected] >>> Blog: http://www.zhangkf.com >>> Twitter: http://twitter.com/zhangkf >>> >>> >>> On Mon, Nov 1, 2010 at 11:50 AM, Kai Feng Zhang <[email protected]>wrote: >>> >>>> Hi Gagandeep, >>>> >>>> Thank you for the detailed sample code. >>>> >>>> I tried with your module override code, but seems it does not work, there >>>> are binding exception if I replace the DefaultGuiceModule with my new >>>> CustomGuiceModule in web.xml. >>>> >>>> ============ Exception stack ================= >>>> 2010-11-1 11:19:53 org.apache.catalina.core.StandardContext listenerStart >>>> com.google.inject.CreationException: Guice creation errors: >>>> >>>> 1) A binding to java.util.Set<java.lang.String> annotated with >>>> @com.google.inject.name.Named(value=org.apache.shindig.features-extended) >>>> was already configured at >>>> org.apache.shindig.gadgets.DefaultGuiceModule.registerFeatureHandlers(DefaultGuiceModule.java:124). >>>> at >>>> org.apache.shindig.extras.ShindigExtrasGuiceModule.configureExtraFeatures(ShindigExtrasGuiceModule.java:41) >>>> >>>> 2) A binding to java.util.Set<java.lang.Object> annotated with >>>> @com.google.inject.name.Named(value=org.apache.shindig.handlers) was >>>> already >>>> configured at >>>> org.apache.shindig.gadgets.DefaultGuiceModule.registerGadgetHandlers(DefaultGuiceModule.java:105). >>>> at >>>> org.apache.shindig.social.core.config.SocialApiGuiceModule.configure(SocialApiGuiceModule.java:76) >>>> >>>> 2 errors >>>> at >>>> com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:354) >>>> at >>>> com.google.inject.InjectorBuilder.initializeStatically(InjectorBuilder.java:152) >>>> at com.google.inject.InjectorBuilder.build(InjectorBuilder.java:105) >>>> at com.google.inject.Guice.createInjector(Guice.java:92) >>>> at >>>> org.apache.shindig.common.servlet.GuiceServletContextListener.contextInitialized(GuiceServletContextListener.java:73) >>>> at >>>> org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:3972) >>>> at >>>> org.apache.catalina.core.StandardContext.start(StandardContext.java:4467) >>>> at >>>> org.apache.catalina.core.ContainerBase.start(ContainerBase.java:1045) >>>> at org.apache.catalina.core.StandardHost.start(StandardHost.java:785) >>>> at >>>> org.apache.catalina.core.ContainerBase.start(ContainerBase.java:1045) >>>> at org.apache.catalina.core.StandardEngine.start(StandardEngine.java:443) >>>> at >>>> org.apache.catalina.core.StandardService.start(StandardService.java:519) >>>> at org.apache.catalina.core.StandardServer.start(StandardServer.java:710) >>>> at org.apache.catalina.startup.Catalina.start(Catalina.java:581) >>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) >>>> at >>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) >>>> at >>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) >>>> at java.lang.reflect.Method.invoke(Method.java:592) >>>> at org.apache.catalina.startup.Bootstrap.start(Bootstrap.java:289) >>>> at org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:414) >>>> 2010-11-1 11:19:53 org.apache.catalina.core.StandardContext start >>>> >>>> ==================CustomGuiceModule======================= >>>> >>>> protected void configure() { >>>> install(Modules.override(new DefaultGuiceModule()).with(new >>>> AbstractModule() { >>>> >>>> @Override >>>> protected void configure() { >>>> // TODO Auto-generated method stub >>>> } >>>> >>>> @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, >>>> CustomGadgetRewriter customGadgetRewriter) { >>>> return ImmutableList.of(pipelineRewriter, templateRewriter, >>>> absolutePathRewriter, styleTagExtractorRewriter, >>>> styleAdjacencyRewriter, proxyingRewriter, >>>> cajaRewriter, sanitizedRewriter, renderingRewriter, >>>> i18nRewriter, customGadgetRewriter); >>>> } >>>> >>>> })); >>>> >>>> } >>>> >>>> ================web.xml===================== >>>> <context-param> >>>> <param-name>guice-modules</param-name> >>>> <param-value> >>>> org.apache.shindig.common.PropertiesModule: >>>> org.apache.shindig.extras.CustomGuiceModule: >>>> org.apache.shindig.social.core.config.SocialApiGuiceModule: >>>> org.apache.shindig.social.sample.SampleModule: >>>> org.apache.shindig.gadgets.oauth.OAuthModule: >>>> org.apache.shindig.common.cache.ehcache.EhCacheModule: >>>> org.apache.shindig.sample.shiro.ShiroGuiceModule: >>>> org.apache.shindig.sample.container.SampleContainerGuiceModule: >>>> org.apache.shindig.extras.ShindigExtrasGuiceModule: >>>> org.apache.shindig.extras.as.ActivityStreamsGuiceModule >>>> </param-value> >>>> </context-param> >>>> >>>> ================================== >>>> From the exception, it says 2 binding are already configured in >>>> DefaultGuiceModule, then they should not be configured again in other >>>> modules, but these 2 binding are multibinding. I am curious why this would >>>> happen. >>>> >>>> If this kind of override module way works, it does be helpful for my >>>> case, since I even do not need to change any code of existing Shindig >>>> code(any modules). Thanks. >>>> >>>> >>>> Thanks, >>>> >>>> Kevin Zhang (凯峰) >>>> Gtalk: [email protected] >>>> Blog: http://www.zhangkf.com >>>> Twitter: http://twitter.com/zhangkf >>>> >>>> >>>> On Mon, Nov 1, 2010 at 12:31 AM, Gagandeep Singh >>>> <[email protected]>wrote: >>>> >>>>> Hi Kai >>>>> >>>>> >>>>> On Sun, Oct 31, 2010 at 5:38 PM, Kai Feng Zhang <[email protected]>wrote: >>>>> >>>>>> Hey Gagandeep, >>>>>> >>>>>> Thank you, it seems your solution is cool thing with more flexibility. >>>>>> >>>>>> What I am asking is to *append* a rewriter to current rewriters list >>>>>> from DefaultGuiceModule, b/c I think the existing rewriters are good >>>>>> enough >>>>>> for my case. And as I know, DefaultGuiceModule does a lot of things >>>>>> (installing different modules etc), but not only install RewriterModule >>>>>> for >>>>>> binding rewiters. >>>>>> >>>>> >>>>> Your module (say MyGuiceModule) would look like: >>>>> class MyGuiceModule extends AbstractModule { >>>>> public void configure() { >>>>> install(Modules.override(new DefaultGuiceModule()).with(new >>>>> MySpecialModule())); >>>>> } >>>>> } >>>>> >>>>> This will provide everything that DefaultGuiceModule already does. In >>>>> addition, it will provide the bindings you specifically provided in >>>>> MySpecialModule and override these when they conflict with ones provided >>>>> by DefaultGuiceModule. >>>>> Take a look at >>>>> Modules.override<http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/util/Modules.html#override(com.google.inject.Module...)>documentation. >>>>> >>>>> As far as appending a rewriter to existing list is concerned, can't >>>>> help you much there. >>>>> This sample code: >>>>> class T { >>>>> public final String name; >>>>> public T(String name) { >>>>> this.name = name; >>>>> } >>>>> } >>>>> >>>>> class A extends AbstractModule { >>>>> @Override >>>>> protected void configure() { >>>>> bind(T.class).annotatedWith(Names.named("hello")).toInstance(new >>>>> T("hello")); >>>>> } >>>>> } >>>>> class B extends AbstractModule { >>>>> @Override >>>>> protected void configure() { >>>>> } >>>>> >>>>> @Provides @Named("hello") @Inject >>>>> public T provideBuffalo(@Named("hello") T helloT) { >>>>> return new T("buffalo"); >>>>> } >>>>> } >>>>> >>>>> Injector injector = Guice.createInjector(Modules.override(new >>>>> A()).with(new B())); >>>>> T h = injector.getInstance(Key.get(T.class, Names.named("hello"))); >>>>> assertEquals("buffalo", h.name); >>>>> >>>>> throws StackOverflowException, because guice is trying to access the >>>>> same @Named("hello")that its trying to provide. >>>>> So you'l have to enumerate the rewriters you need to append your >>>>> rewriter to again in your module. >>>>> >>>>> >>>>>> So I think replace DefaultGuiceModule with custom module class in >>>>>> web.xml maybe not enough, you still need to do other same work as >>>>>> DefaultGuiceModule in your module.Please note that RewriterModule >>>>>> installed >>>>>> in DefaultGuiceModule is not configured in web.xml. >>>>>> >>>>>> Any comments? Thanks a lot. >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Kevin Zhang (凯峰) >>>>>> Gtalk: [email protected] >>>>>> Blog: http://www.zhangkf.com >>>>>> Twitter: http://twitter.com/zhangkf >>>>>> >>>>>> >>>>>> On Sat, Oct 30, 2010 at 10:28 AM, Gagandeep singh < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Kai, Paul >>>>>>> >>>>>>> You are both right :) The order of rewriters matters. >>>>>>> >>>>>>> I have a patch< >>>>>>> http://codereview.appspot.com/2058042/diff/123001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java >>>>>>> > >>>>>>> >>>>>>> (which >>>>>>> JohnH has been reviewing and providing ideas for) which does something >>>>>>> similar. The idea is, we create a map binder from container + flow id >>>>>>> -> * >>>>>>> provider* of list of rewriters. >>>>>>> And we provide the list of rewriters similarly to how we providing >>>>>>> currently. Since we add provider of rewriter list, we can provide a >>>>>>> different list without having to add it to the map binder separately >>>>>>> (map >>>>>>> binder will cry if you add same key twice). >>>>>>> Any new binding can be added to map binder in the custom module. >>>>>>> >>>>>>> The way ppl can override this is, in your new module, you again >>>>>>> provide a >>>>>>> new list of rewriters. And then you override the DefaultGuiceModule >>>>>>> with >>>>>>> your module. >>>>>>> >>>>>>> Say you were providing a default list of GadgetRewriters in >>>>>>> DefaultGuiceModule: >>>>>>> @Named("gadget.rewriters") List<GadgetRewriter> provideDef(Rewriter1 >>>>>>> r1) { >>>>>>> } >>>>>>> >>>>>>> and you want to change this list of provide Rewriter2 as well. You >>>>>>> create >>>>>>> your module called MyModule and provide: >>>>>>> @Named("gadget.rewriters") List<GadgetRewriter> provideDef(Rewriter1 >>>>>>> r1, >>>>>>> Rewriter2 r2) { >>>>>>> } >>>>>>> >>>>>>> And then early on in the code, you override default guice module with >>>>>>> your >>>>>>> module: >>>>>>> Modules.override(new GuiceModule()).with(new MyModule()); >>>>>>> >>>>>>> I think your facing the problem because maybe web.xml does not allow >>>>>>> you to >>>>>>> override a module with a new one. And when 2 modules provide same >>>>>>> binding, >>>>>>> guice dies. >>>>>>> A quick solution for your case would be to change web.xml to provide >>>>>>> your >>>>>>> module in place of DefaultGuiceModule, and change your module to >>>>>>> install DefaultGuiceModule after overriding it with your rewriter >>>>>>> list. >>>>>>> Something like: >>>>>>> >>>>>>> web.xml: >>>>>>> <context-param> >>>>>>> <param-name>guice-modules</param-name> >>>>>>> <param-value> >>>>>>> org.apache.shindig.common.PropertiesModule: >>>>>>> org.apache.shindig.gadgets.*MyGuiceModule*: >>>>>>> org.apache.shindig.social.core.config.SocialApiGuiceModule: >>>>>>> ...... >>>>>>> >>>>>>> MyGuiceModule.java: >>>>>>> protected void configure() { >>>>>>> Modules.override(new DefaultGuiceModule()).with(new AbstractModule() >>>>>>> { >>>>>>> protected void configure() { >>>>>>> } >>>>>>> �...@provides @Named("gadget.rewriters") List<GadgetRewriter> >>>>>>> .... >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> I hope im making sense. >>>>>>> >>>>>>> On Fri, Oct 29, 2010 at 5:52 PM, Paul Lindner <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>> > I think this is a good idea, however I'm concerned that the ordering >>>>>>> of >>>>>>> > Rewriters may be important. >>>>>>> > >>>>>>> > I'm guessing that we could make a SortedSet if we had a Comparable<> >>>>>>> > implementation of all rewriters. >>>>>>> > >>>>>>> > On Fri, Oct 29, 2010 at 1:23 AM, Kai Feng Zhang <[email protected] >>>>>>> > >>>>>>> > wrote: >>>>>>> > >>>>>>> > > I found a possible solution to extend Shindig rewriter capability, >>>>>>> using >>>>>>> > > multibinding in Guice. >>>>>>> > > >>>>>>> > > Please see: >>>>>>> > > >>>>>>> > > >>>>>>> > >>>>>>> http://google-guice.googlecode.com/svn/trunk/latest-javadoc/com/google/inject/multibindings/Multibinder.html >>>>>>> > > >>>>>>> > > I created a jira for this, see >>>>>>> > > https://issues.apache.org/jira/browse/SHINDIG-1456 >>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> > > On Fri, Oct 29, 2010 at 3:02 PM, Kai Feng Zhang < >>>>>>> [email protected]> >>>>>>> > > wrote: >>>>>>> > > >>>>>>> > > > Hi, >>>>>>> > > > >>>>>>> > > > I'd like to add a custom rewriter into Shindig, with requirement >>>>>>> of no >>>>>>> > > need >>>>>>> > > > to change Shindig rendering gadget server side code directly. I >>>>>>> want to >>>>>>> > > add >>>>>>> > > > it as a new feature in extras, so when this feature is required >>>>>>> by >>>>>>> > > gadget, >>>>>>> > > > the custom rewriter will do some special work when rendering >>>>>>> gadget at >>>>>>> > > > server side. >>>>>>> > > > >>>>>>> > > > But I checked Shindig code, RewriteModule @Provides all >>>>>>> predefined >>>>>>> > > > rewriters with @Named("shindig.rewriters.gadget") , and >>>>>>> > > > then GadgetRewritersProvider will provide all rewriters as per >>>>>>> the same >>>>>>> > > > @Name when rendering gadget. >>>>>>> > > > >>>>>>> > > > If I create a new Module in shindig extras, and @Provides custom >>>>>>> > rewriter >>>>>>> > > > with same @Name, and startup server, there will be error saying >>>>>>> the >>>>>>> > same >>>>>>> > > > @Name is configured already by RewriteModule. >>>>>>> > > > >>>>>>> > > > Can anyone please help if there is some way to append such a >>>>>>> rewriter >>>>>>> > > > without touching any existing Shindig gadget rendering code? Or >>>>>>> that is >>>>>>> > > the >>>>>>> > > > only way to extend a new rewriter for gadget rendering? >>>>>>> > > > >>>>>>> > > > Thanks a lot! >>>>>>> > > > >>>>>>> > > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > -- >>>>>>> > Paul Lindner -- [email protected] -- linkedin.com/in/plindner >>>>>>> > >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> > -- Thanks, Henry
