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