Caja upgrade to r4094

* upgrade to latest rev of Caja
* small api change to wire HttpFetcher to Caja's UriFetcher for retrieving scripts
* exposes the cajoling service interface to shindig

[chirag, 2010-06-03 21:28:14]

http://codereview.appspot.com/1460041/diff/2001/3003
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaServlet.java
(right):

http://codereview.appspot.com/1460041/diff/2001/3003#newcode54
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaServlet.java:54:
public class CajaServlet extends InjectedServlet {
It'd be nice to have a unit test for this

http://codereview.appspot.com/1460041/diff/2001/3003#newcode60
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaServlet.java:60:
private BuildInfo buildInfo = BuildInfo.getInstance();
Could be final

http://codereview.appspot.com/1460041/diff/2001/3003#newcode81
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaServlet.java:81:
logger.info("Failed to retrieve: " + ref.toString());
Could we send a 4xx HTTP status code when this happens?

http://codereview.appspot.com/1460041/diff/2001/3003#newcode92
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaServlet.java:92:
servlet.doGet(req, resp);
Does the CajolingServlet set the HTTP Cache-Control max-age directive

[Paul Lindner, 2010-06-08 06:43:05]

seems fine on the surface.  However I applied this patch and commented out the
tests in CajaSocialMarkupHtmlParserTest -- instead of throwing NPEs they fail
with assertions like this:

java.lang.AssertionError: expected:<1> but was:<0>
        at org.junit.Assert.fail(Assert.java:91)
        at org.junit.Assert.failNotEquals(Assert.java:645)
        at org.junit.Assert.assertEquals(Assert.java:126)
        at org.junit.Assert.assertEquals(Assert.java:470)
        at org.junit.Assert.assertEquals(Assert.java:454)
        at
org.apache.shindig.gadgets.parse.AbstractSocialMarkupHtmlParserTest.testSocialData(AbstractSocialMarkupHtmlParserTest.java:62)
        at
org.apache.shindig.gadgets.parse.caja.CajaSocialMarkupHtmlParserTest.testSocialData(CajaSocialMarkupHtmlParserTest.java:37)


I'd love to see these tests working again...

Thanks!

[johnfargo, 2010-06-18 17:12:42]

I'm not sure why Paul got broken tests w/ this patch -- my equivalent
(http://codereview.appspot.com/1714043/show) sees them working nicely.

http://codereview.appspot.com/1460041/diff/9001/10002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/1460041/diff/9001/10002#newcode176
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:176:
response.getContentType(), response.getEncoding());
minor preference for getContentType being a helper method here rather than in
HttpResponse to prevent latter from getting too big

http://codereview.appspot.com/1460041/diff/9001/10002#newcode186
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:186:
return UriPolicy.IDENTITY;
FMI what's this do? Just returns same? Why remove the previous logic?

[fargo, 2010-06-18 18:03:34]

Committed.

On 2010/06/18 17:12:42, johnfargo wrote:
> I'm not sure why Paul got broken tests w/ this patch -- my equivalent
> (http://codereview.appspot.com/1714043/show) sees them working nicely.
> 
> http://codereview.appspot.com/1460041/diff/9001/10002
> File
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
> (right):
> 
> http://codereview.appspot.com/1460041/diff/9001/10002#newcode176
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:176:
> response.getContentType(), response.getEncoding());
> minor preference for getContentType being a helper method here rather than in
> HttpResponse to prevent latter from getting too big
> 
> http://codereview.appspot.com/1460041/diff/9001/10002#newcode186
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:186:
> return UriPolicy.IDENTITY;
> FMI what's this do? Just returns same? Why remove the previous logic?

[fargo, 2010-06-18 18:03:46]

Wrong CL, mine was committed!

On 2010/06/18 18:03:34, fargo wrote:
> Committed.
> 
> On 2010/06/18 17:12:42, johnfargo wrote:
> > I'm not sure why Paul got broken tests w/ this patch -- my equivalent
> > (http://codereview.appspot.com/1714043/show) sees them working nicely.
> > 
> > http://codereview.appspot.com/1460041/diff/9001/10002
> > File
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
> > (right):
> > 
> > http://codereview.appspot.com/1460041/diff/9001/10002#newcode176
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:176:
> > response.getContentType(), response.getEncoding());
> > minor preference for getContentType being a helper method here rather than
in
> > HttpResponse to prevent latter from getting too big
> > 
> > http://codereview.appspot.com/1460041/diff/9001/10002#newcode186
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:186:
> > return UriPolicy.IDENTITY;
> > FMI what's this do? Just returns same? Why remove the previous logic?