Thanks again Ziv! Comments inline. On Mon, Mar 8, 2010 at 6:08 PM, <[email protected]> wrote:
> LGTM > > (Sorry previous mail didn't take comment...) > > > http://codereview.appspot.com/224094/diff/16001/17004 > File > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingContentRewriter.java > (right): > > http://codereview.appspot.com/224094/diff/16001/17004#newcode32 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingContentRewriter.java:32: > public class ProxyingContentRewriter extends DomWalker.Rewriter { > Just a suggestions, there a repeated boiler plate code for multiple > rewriters. Maybe it is possible to make it a generic that take in a > visitor class and a uri manager? I played around with this a bit but didn't find an especially clean way to do it. The biggest pain is that ContentRewriterFeature.Config is created per-request, so @Inject'ing it isn't quite so easy (without lots of Guice-fu). A generic base class could be possible but would require (AFAICT) either A) custom reflection/Guice-like code, or B) passing the Guice injector through Gadget[Context] and/or HttpRequest.getAttribute(...), then using that w/ a custom Module to create each Visitor. Then again, there may be some much more clever way to handle this... but I've not figured it out yet. Suggestions mas welcome. > > > http://codereview.appspot.com/224094/show >
