Convert RequestRewriters to ResponseRewriters

Part #2 of move to ResponseRewriter interfaces.

Converts RequestRewriter implementations to ResponseRewriters instead. Replaces RequestRewriterRegistry with ResponseRewriterRegistry, with pre-cached ResponseRewriterRegistry independently registered.

[Paul Lindner, 2010-06-01 17:47:17]

http://codereview.appspot.com/1304042/diff/1/30
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java
(right):

http://codereview.appspot.com/1304042/diff/1/30#newcode74
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:74:
// TODO: Clean this up.
details?
also change signature to providePreCacheResponseRewriters to not clash with
method below..

http://codereview.appspot.com/1304042/diff/1/30#newcode88
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:88:
protected List<ResponseRewriter> provideRequestRewriters(
rename to provideResponseRewriters

[fargo, 2010-06-01 18:23:36]

On Tue, Jun 1, 2010 at 11:17 PM, <lindner@inuus.com> wrote:

>
> http://codereview.appspot.com/1304042/diff/1/30
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java
> (right):
>
> http://codereview.appspot.com/1304042/diff/1/30#newcode74
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:74:
> // TODO: Clean this up.
> details?
>

Thinking about this more, I've slightly modified it (still not fully
satisfied though) to the following. As described in the updated TODO
comment, it just seems a little funny that I'm creating the registry via
"new" here. Perhaps I'm just missing the appriate Guice-fu.

  // TODO: Clean this up. Ideally we would let the ResponseRewriterRegistry
  // binding create the concrete object instance.
  @Provides
  @Singleton
  @Named("shindig.rewriters.response.pre-cache")
  protected ResponseRewriterRegistry
providePreCacheResponseRewritersRegistry(
      GadgetHtmlParser parser,
      @Named("shindig.rewriters.response.pre-cache") List<ResponseRewriter>
preCached) {
    return new DefaultResponseRewriterRegistry(preCached, parser);
  }

  @Provides
  @Singleton
  @Named("shindig.rewriters.response.pre-cache")
  protected List<ResponseRewriter> providePreCacheResponseRewriters(
      BasicImageRewriter imageRewriter) {
    return ImmutableList.<ResponseRewriter>of(imageRewriter);
  }


> also change signature to providePreCacheResponseRewriters to not clash
> with method below..
>

Done.


>
> http://codereview.appspot.com/1304042/diff/1/30#newcode88
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:88:
> protected List<ResponseRewriter> provideRequestRewriters(
> rename to provideResponseRewriters


Done.


>
>
> http://codereview.appspot.com/1304042/show
>

[chirag, 2010-06-01 18:44:36]

lgtm

http://codereview.appspot.com/1304042/diff/1/21
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java
(right):

http://codereview.appspot.com/1304042/diff/1/21#newcode42
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java:42:
public class SanitizingRequestRewriter implements ResponseRewriter {
What do you think about renaming the class name to SanitizingResponseRewriter.
We can make this change after the branch is cut.

s/SanitizingRequestRewriter/SanitizingResponseRewriter

http://codereview.appspot.com/1304042/diff/1/21#newcode115
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java:115:
resp.setContent("");
I'd log a debug message here

http://codereview.appspot.com/1304042/diff/1/4
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriterTest.java
(right):

http://codereview.appspot.com/1304042/diff/1/4#newcode40
java/gadgets/src/test/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriterTest.java:40:
public class SanitizingRequestRewriterTest extends RewriterTestBase {
s/SanitizingRequestRewriterTest/SanitizingResponseRewriterTest

[johnfargo, 2010-06-02 08:21:11]

Just committed a CL renaming all XRequestRewriter to XResponseRewriter. Thx-
John

On Wed, Jun 2, 2010 at 12:14 AM, <chiragshah1@gmail.com> wrote:

> lgtm
>
>
> http://codereview.appspot.com/1304042/diff/1/21
> File
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java
> (right):
>
> http://codereview.appspot.com/1304042/diff/1/21#newcode42
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java:42:
> public class SanitizingRequestRewriter implements ResponseRewriter {
> What do you think about renaming the class name to
> SanitizingResponseRewriter. We can make this change after the branch is
> cut.
>
> s/SanitizingRequestRewriter/SanitizingResponseRewriter
>
> http://codereview.appspot.com/1304042/diff/1/21#newcode115
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriter.java:115:
> resp.setContent("");
> I'd log a debug message here
>
> http://codereview.appspot.com/1304042/diff/1/4
> File
>
>
java/gadgets/src/test/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriterTest.java
> (right):
>
> http://codereview.appspot.com/1304042/diff/1/4#newcode40
>
>
java/gadgets/src/test/java/org/apache/shindig/gadgets/render/SanitizingRequestRewriterTest.java:40:
> public class SanitizingRequestRewriterTest extends RewriterTestBase {
> s/SanitizingRequestRewriterTest/SanitizingResponseRewriterTest
>
>
> http://codereview.appspot.com/1304042/show
>