Implement caching of cajoled output

* Implements caching/reusing cajoled output from shindig
* Adds style position:relative so that offsetParent operates correctly

@ r826796

[Jasvir, 2009-10-12 21:17:32]

Sorry there were extraneous files for a different CL in my client.

Snapshot.

[Jasvir, 2009-10-12 21:35:53]

http://codereview.appspot.com/129072/diff/22/1001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/129072/diff/22/1001#newcode114
Line 114: String key = HashUtil.rawChecksum(content.getContent().getBytes());
I probably should not serialize here to compute the checksum.  Can someone
suggest an alternative?

[johnfargo, 2009-10-12 22:09:54]

We should add some tests to this at some point as well.

http://codereview.appspot.com/129072/diff/22/1001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/129072/diff/22/1001#newcode72
Line 72: System.err.println("Cajoled cache created" + cajoledCache);
use logger please ;)

http://codereview.appspot.com/129072/diff/22/1001#newcode114
Line 114: String key = HashUtil.rawChecksum(content.getContent().getBytes());
Two comments on selection of key:
1) can you augment DocumentGadgetRewriter to compute one from its input,
augmenting during the rewriting pass? that avoids the
serialization/deserialization set you're doing here.
2) DGR could/should also include some notion of Caja version in its key
computation, to avoid stale entries in the event that CacheProvider might offer
a distributed cache.

http://codereview.appspot.com/129072/diff/22/1001#newcode124
Line 124: HtmlSerialization.attach(doc, new CajaHtmlSerializer(), null);
nit: you could pull these 3 lines into a helper method referenced here and
below.

http://codereview.appspot.com/129072/diff/22/1001#newcode143
Line 143: cajoledOutput.setAttribute("style", "position: relative;");
curious, why doesn't rewriteContent do this boilerplate, even as a helper method
(eg. wrapInContainer or something)?

http://codereview.appspot.com/129072/diff/22/1001#newcode152
Line 152: createContainerFor(doc, cajoledOutput);
nit: could you rename this "createIframeDocumentFor" for clarity?

[louiscryan, 2009-10-12 22:19:57]

http://codereview.appspot.com/129072/diff/22/1001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/129072/diff/22/1001#newcode114
Line 114: String key = HashUtil.rawChecksum(content.getContent().getBytes());
If serialization is significantly faster than cajoling we can worry about the
key later as a TODO. I would put a more efficient key generation method on
MutableContent that does not require large buffer allocations for serialized
documents.


On 2009/10/12 22:09:54, johnfargo wrote:
> Two comments on selection of key:
> 1) can you augment DocumentGadgetRewriter to compute one from its input,
> augmenting during the rewriting pass? that avoids the
> serialization/deserialization set you're doing here.
> 2) DGR could/should also include some notion of Caja version in its key
> computation, to avoid stale entries in the event that CacheProvider might
offer
> a distributed cache.

[Jasvir, 2009-10-12 22:55:00]

snapshot

http://codereview.appspot.com/129072/diff/22/1001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/129072/diff/22/1001#newcode72
Line 72: System.err.println("Cajoled cache created" + cajoledCache);
Leftover debugging.  Nuked.
On 2009/10/12 22:09:54, johnfargo wrote:
> use logger please ;)

http://codereview.appspot.com/129072/diff/22/1001#newcode114
Line 114: String key = HashUtil.rawChecksum(content.getContent().getBytes());
On 2009/10/12 22:09:54, johnfargo wrote:
> Two comments on selection of key:
> 1) can you augment DocumentGadgetRewriter to compute one from its input,
> augmenting during the rewriting pass? that avoids the
> serialization/deserialization set you're doing here.

No in that case DGR would still need to perform the entire pass to determine if
the document is in the cache.  Ideally the has should be computed and maintained
by MutableContent based on a hash of Document.

> 2) DGR could/should also include some notion of Caja version in its key
> computation, to avoid stale entries in the event that CacheProvider might
offer
> a distributed cache.

Good point - caja version number should be part of the key.

http://codereview.appspot.com/129072/diff/22/1001#newcode124
Line 124: HtmlSerialization.attach(doc, new CajaHtmlSerializer(), null);
On 2009/10/12 22:09:54, johnfargo wrote:
> nit: you could pull these 3 lines into a helper method referenced here and
> below.

Done.

http://codereview.appspot.com/129072/diff/22/1001#newcode143
Line 143: cajoledOutput.setAttribute("style", "position: relative;");
Since this the caja's Default/Gadget/Rewriter, it could do that boilerplate. 
Gadgets are not the only input target for the cajoler in which case the cajoler
doesn't know how the output will be used (the div maybe static html for
example).

It could certainly be a helper method though.

On 2009/10/12 22:09:54, johnfargo wrote:
> curious, why doesn't rewriteContent do this boilerplate, even as a helper
method
> (eg. wrapInContainer or something)?

http://codereview.appspot.com/129072/diff/22/1001#newcode152
Line 152: createContainerFor(doc, cajoledOutput);
On 2009/10/12 22:09:54, johnfargo wrote:
> nit: could you rename this "createIframeDocumentFor" for clarity? 

Done.

[Jasvir, 2009-10-12 22:59:40]

http://codereview.appspot.com/129072/diff/22/1001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/129072/diff/22/1001#newcode114
Line 114: String key = HashUtil.rawChecksum(content.getContent().getBytes());
On 2009/10/12 22:19:57, louiscryan wrote:
> If serialization is significantly faster than cajoling we can worry about the
> key later as a TODO. I would put a more efficient key generation method on
> MutableContent that does not require large buffer allocations for serialized
> documents.

Awesome.  Alternatively, walk the Document to compute a hash from that.  I'll
leave it as is for now unless you suggest otherwise.

> 
> 
> On 2009/10/12 22:09:54, johnfargo wrote:
> > Two comments on selection of key:
> > 1) can you augment DocumentGadgetRewriter to compute one from its input,
> > augmenting during the rewriting pass? that avoids the
> > serialization/deserialization set you're doing here.
> > 2) DGR could/should also include some notion of Caja version in its key
> > computation, to avoid stale entries in the event that CacheProvider might
> offer
> > a distributed cache.
> 
>

[johnfargo, 2009-10-13 00:48:03]

Looks fine to me. Louis?