> On 2012-03-20 01:23:55, Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResponseRewriter.java, > > line 30 > > <https://reviews.apache.org/r/4284/diff/4/?file=93735#file93735line30> > > > > Since most implementation will simply call the other method with Gadget > > in the input argument, maybe we can just remove this method signature? > > Ryan Baxter wrote: > You are hard core. even when your sick your still reviewing code :) > > We could, but I think there may still be use cases where there may be no > gadget involved in the request and you still want to rewrite the content, so > the API may still be required.
Easier to read code than spec changes =P We can get away with @Nullable with the gadget in the new API. The reason is that it looks weird to have 2 methods that have the same name and only differ in additional input param. If there is no tech limitation I think we should just have one rewrite method. Simplification of how Shindig can be extended always good. - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4284/#review6104 ----------------------------------------------------------- On 2012-03-20 19:45:08, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4284/ > ----------------------------------------------------------- > > (Updated 2012-03-20 19:45:08) > > > Review request for shindig. > > > Summary > ------- > > When the MakeRequestHandler or HttpRequestHandler rewrites HTML content it > does not use the gadget context of the gadget making the request. The > downside is that if the gadget specifies specific URLs that should not be > rewriten via the content-rewrite feature the rewrite feature will not honor > those URLs. > > > This addresses bug SHINDIG-1730. > https://issues.apache.org/jira/browse/SHINDIG-1730 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/logging/i18n/MessageKeys.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/resources/org/apache/shindig/common/logging/i18n/resource.properties > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/resources/org/apache/shindig/common/logging/i18n/resource_en_US.properties > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/CajaResponseRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BaseTagRemoverRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContextAwareRegistry.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultResponseRewriterRegistry.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomWalker.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResponseRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResponseRewriterRegistry.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/CajaResponseRewriterTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriterTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseTagRemoverRewriterTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ContextAwareRegistryTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriterTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistryTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java > 1302466 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HttpRequestHandlerTest.java > 1302466 > > Diff: https://reviews.apache.org/r/4284/diff > > > Testing > ------- > > Updated unit tests > > > Thanks, > > Ryan > >
