Caja does not support proxy url gadgets

* do not render type url gadgets if using caja since proxy url gadgets bypass the entire rewriting stack including the cajoler

[Jasvir, 2010-03-10 01:21:50]

http://codereview.appspot.com/380041/diff/1/2
File
java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java
(right):

http://codereview.appspot.com/380041/diff/1/2#newcode205
java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java:205:
}
I'd really like to include a test for proxied gadgets here too, however, proxied
gadgets give me a "gadgets" not found error.

[johnfargo, 2010-03-10 01:56:06]

LGTM

...with a few minor suggestions.

http://codereview.appspot.com/380041/diff/1/4
File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
(right):

http://codereview.appspot.com/380041/diff/1/4#newcode81
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:81:
if (gadget.getCurrentView().getType() == View.ContentType.URL &&
FYI: this isn't proxied, this is type=url. Logic is fine, CL description a
little off ;)

http://codereview.appspot.com/380041/diff/1/4#newcode83
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:83:
"1".equals(gadget.getContext().getParameter("caja")))) {
we should probably move this check to a utility method

http://codereview.appspot.com/380041/diff/1/2
File
java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java
(right):

http://codereview.appspot.com/380041/diff/1/2#newcode205
java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java:205:
}
Interested to hear more -- where is this error thrown? What's in the output?

[chirag, 2010-03-10 08:28:29]

http://codereview.appspot.com/380041/diff/1/4
File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
(right):

http://codereview.appspot.com/380041/diff/1/4#newcode81
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:81:
if (gadget.getCurrentView().getType() == View.ContentType.URL &&
Minor nit:
The check "gadget.getCurrentView().getType() == View.ContentType.URL" is already
being done on line 88.

Would it make sense to simply throw an exception if caja is enabled at line 89?

http://codereview.appspot.com/380041/diff/1/4#newcode83
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:83:
"1".equals(gadget.getContext().getParameter("caja")))) {
On 2010/03/10 01:56:06, johnfargo wrote:
> we should probably move this check to a utility method
+1

[Jasvir, 2010-03-10 09:22:00]

http://codereview.appspot.com/380041/diff/1/4
File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
(right):

http://codereview.appspot.com/380041/diff/1/4#newcode81
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:81:
if (gadget.getCurrentView().getType() == View.ContentType.URL &&
On 2010/03/10 01:56:06, johnfargo wrote:
> FYI: this isn't proxied, this is type=url. Logic is fine, CL description a
> little off ;)

Done.  Fixed the CL description.

http://codereview.appspot.com/380041/diff/1/4#newcode81
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:81:
if (gadget.getCurrentView().getType() == View.ContentType.URL &&
On 2010/03/10 08:28:29, chirag wrote:
> Minor nit:
> The check "gadget.getCurrentView().getType() == View.ContentType.URL" is
already
> being done on line 88.
> 
> Would it make sense to simply throw an exception if caja is enabled at line
89?

Done.

http://codereview.appspot.com/380041/diff/1/4#newcode83
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:83:
"1".equals(gadget.getContext().getParameter("caja")))) {
Good idea.  Done.

On 2010/03/10 01:56:06, johnfargo wrote:
> we should probably move this check to a utility method

[johnfargo, 2010-03-10 20:15:43]

LGTM

http://codereview.appspot.com/380041/diff/8001/9003
File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
(right):

http://codereview.appspot.com/380041/diff/8001/9003#newcode82
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java:82:
if (requiresCaja(gadget)) {
looks fine, though in a followup we should use this (static-ified) method in the
other places in code that duplicate it ;)