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. http://codereview.appspot.com/129072
