You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Ryan Baxter <rb...@gmail.com> on 2012/01/28 02:16:24 UTC

Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/
-----------------------------------------------------------

Review request for shindig.


Summary
-------

The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.


This addresses bug SHINDIG-1549.
    https://issues.apache.org/jira/browse/SHINDIG-1549


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 

Diff: https://reviews.apache.org/r/3670/diff


Testing
-------

Updated unit tests and tested in the various containers


Thanks,

Ryan


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4683
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
<https://reviews.apache.org/r/3670/#comment10425>

    I think we could just call this "iframeUrls". Should be implicitly imply all for the metadata call since we dont pass any view param.


- Henry


On 2012-01-28 01:17:22, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-28 01:17:22)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Ryan Baxter <rb...@gmail.com>.

> On 2012-01-28 15:17:28, Stanton Sievers wrote:
> > My biggest concern with this change is that it breaks backwards compatibility.  There is no longer "iframeurl" on the metadata responses.  This isn't a big deal if everyone is using the common container, since you've updated that portion, but if they aren't, then this will cause problems.  Maybe not a big deal, as the metadata request/response is not spec'd.  Something to consider and get more feedback on from the rest of the dev list.
> > 
> > Also, have you tried this out with requestNavigateTo functionality or the view switching functionality (as in the sample common container)?  With this patch the CC shouldn't have to re-issue any metadata requests because it has everything it needs.  I want to make sure that is indeed the case.

RE backwards compatibility
I realize this.  Since this is the first release of Shindig with the common container I think it is fine to break the "API".  However if someone have a need for that functionality I can add it back in.

RE requestNavigateTo
Even before this change we only ever made 1 request to get the metadata for the gadget.  When switching views we used the cached metadata.  This was part of the problem.  The cache is indexed by gadget URL so switching views and / or subsequent calls to requestNavigateTo always resulted in using the cached metadata.


> On 2012-01-28 15:17:28, Stanton Sievers wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js, line 169
> > <https://reviews.apache.org/r/3670/diff/3/?file=71190#file71190line169>
> >
> >     With your changes, uri is now the uri with render params and other query params set on it.  Is that necessary for the patch?  I'd like to not change this behavior if you don't have to.

It was not necessary, but it seemed odd to me that we were not using the actual URL in the iFrame when setting up relay uri.  If getIframeUrl_ changed the URL in some way that would effect setting up RPC then we might be in trouble.  I figured it would be best to use the same URL that the iFrame will contain.  


- Ryan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4672
-----------------------------------------------------------


On 2012-01-30 15:29:59, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-30 15:29:59)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4672
-----------------------------------------------------------


My biggest concern with this change is that it breaks backwards compatibility.  There is no longer "iframeurl" on the metadata responses.  This isn't a big deal if everyone is using the common container, since you've updated that portion, but if they aren't, then this will cause problems.  Maybe not a big deal, as the metadata request/response is not spec'd.  Something to consider and get more feedback on from the rest of the dev list.

Also, have you tried this out with requestNavigateTo functionality or the view switching functionality (as in the sample common container)?  With this patch the CC shouldn't have to re-issue any metadata requests because it has everything it needs.  I want to make sure that is indeed the case.


http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
<https://reviews.apache.org/r/3670/#comment10391>

    Can we strip out the gmodules language?  I've never noticed it being here before.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
<https://reviews.apache.org/r/3670/#comment10392>

    With your changes, uri is now the uri with render params and other query params set on it.  Is that necessary for the patch?  I'd like to not change this behavior if you don't have to.



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
<https://reviews.apache.org/r/3670/#comment10393>

    Trailing whitespace



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
<https://reviews.apache.org/r/3670/#comment10394>

    Make this protected so that anyone can override this functionality.  Prior to your changes they would have been able to do that by overriding makeRenderingUri().


- Stanton


On 2012-01-28 01:17:22, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-28 01:17:22)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4699
-----------------------------------------------------------


Committed revision 1238135.

- Ryan


On 2012-01-30 15:29:59, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-30 15:29:59)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4690
-----------------------------------------------------------

Ship it!


LGTM

- Stanton


On 2012-01-30 15:29:59, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-30 15:29:59)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/
-----------------------------------------------------------

(Updated 2012-01-30 15:29:59.387801)


Review request for shindig.


Changes
-------

Updated with Henry's and Stanton's suggestions


Summary
-------

The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.


This addresses bug SHINDIG-1549.
    https://issues.apache.org/jira/browse/SHINDIG-1549


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 

Diff: https://reviews.apache.org/r/3670/diff


Testing
-------

Updated unit tests and tested in the various containers


Thanks,

Ryan


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4684
-----------------------------------------------------------

Ship it!


+1 Other than small nit/suggestion about iframeUrls param name.

- Henry


On 2012-01-28 01:17:22, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-28 01:17:22)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/#review4687
-----------------------------------------------------------


LGTM other than the comments Stanton already made.  Re: backcompat, if it's an issue could we also return the 'iframeurl' in the metadata response as it always came back?  at that point we'd just be adding stuff for the CC until others catch up.

- Dan


On 2012-01-28 01:17:22, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated 2012-01-28 01:17:22)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.
> 
> 
> This addresses bug SHINDIG-1549.
>     https://issues.apache.org/jira/browse/SHINDIG-1549
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 
> 
> Diff: https://reviews.apache.org/r/3670/diff
> 
> 
> Testing
> -------
> 
> Updated unit tests and tested in the various containers
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: gadgets.metadata request fails for gadgets that don't have a "default" view

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3670/
-----------------------------------------------------------

(Updated 2012-01-28 01:17:22.559949)


Review request for shindig.


Summary
-------

The request for gadgets.metadata fails for a gadget spec that doesn't specify a "default" view.


This addresses bug SHINDIG-1549.
    https://issues.apache.org/jira/browse/SHINDIG-1549


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1236884 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java 1236884 

Diff: https://reviews.apache.org/r/3670/diff


Testing
-------

Updated unit tests and tested in the various containers


Thanks,

Ryan