Refactor ProxyingLinkRewriter into a Visitor

This Visitor performs content-reference proxying, splitting logic off from HTMLContentRewriter. The logic is the same: upon matching a given supported tag with a Uri-type attribute, make the attribute's value a proxied version of itself.

ProxyUriManager is used to perform the Uri operation. The implementation uses the DomWalker.Visitor pattern.

[johnfargo, 2010-03-02 10:21:26]

Adding DomWalker test base class updates.

[johnfargo, 2010-03-02 10:23:31]

...and the Content Reference Rewriter implementation using the new Visitor.

[johnfargo, 2010-03-03 19:50:11]

Synced to head.

[zhoresh, 2010-03-05 23:30:47]

LGTM

http://codereview.appspot.com/224094/diff/2001/2002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingLinkRewriter.java
(left):

http://codereview.appspot.com/224094/diff/2001/2002#oldcode67
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingLinkRewriter.java:67:
result.append(rewriterFeature.getFingerprint());
There is no "fingerprint" in new code. Is that cover by versioning? are you plan
to implement it in shindig?

[johnfargo, 2010-03-08 22:37:30]

On Fri, Mar 5, 2010 at 3:30 PM, <zhoresh@gmail.com> wrote:

> LGTM
>
>
> http://codereview.appspot.com/224094/diff/2001/2002
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingLinkRewriter.java
> (left):
>
> http://codereview.appspot.com/224094/diff/2001/2002#oldcode67
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingLinkRewriter.java:67:
> result.append(rewriterFeature.getFingerprint());
> There is no "fingerprint" in new code. Is that cover by versioning? are
> you plan to implement it in shindig?


Yes, the idea here was to version the rewriters themselves. An
implementation may choose to do this if deemed important. It would be
trivial to add to Shindig... I'll put it on the TODO list.


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

[johnfargo, 2010-03-08 22:40:20]

Renamed ProxyingLinkRewriter to ProxyingVisitor to avoid conflation w/ previous
classes.

[johnfargo, 2010-03-09 01:33:32]

Integration test stub, renaming for consistency.

[zhoresh, 2010-03-09 02:07:55]

On 2010/03/09 01:33:32, johnfargo wrote:
> Integration test stub, renaming for consistency.

LGTM

[zhoresh, 2010-03-09 02:08:37]

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?

[fargo, 2010-03-09 02:15:50]

Thanks again Ziv! Comments inline.

On Mon, Mar 8, 2010 at 6:08 PM, <zhoresh@gmail.com> 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
>