[ 
https://issues.apache.org/jira/browse/SHINDIG-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13196172#comment-13196172
 ] 

[email protected] commented on SHINDIG-1549:
--------------------------------------------------------



bq.  On 2012-01-28 15:17:28, Stanton Sievers wrote:
bq.  > 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.
bq.  > 
bq.  > 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.


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


                
> gadgets.metadata request fails for gadgets that don't have a "default" view
> ---------------------------------------------------------------------------
>
>                 Key: SHINDIG-1549
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1549
>             Project: Shindig
>          Issue Type: Bug
>          Components: Java
>    Affects Versions: 3.0.0
>         Environment: Mac OSX
>            Reporter: Doug Davies
>             Fix For: 3.0.0
>
>
> The request for gadgets.metadata fails for a gadget spec that doesn't specify 
> a "default" view.  For example 
> http://www.google.com/ig/modules/wikipedia.xml.  The reason is in 
> GadgetsHandlerService.java.
>     if (needIfrUrl && gadget.getCurrentView() == null) {
>       throw new ProcessingException("View " + request.getView() + " does not 
> exist",
>           HttpResponse.SC_BAD_REQUEST);
>     }
> The gadget.getCurrentView returns null if no "default" view is defined.
> You can see this behavior by testing in the commoncontainer sample and trying 
> to load the wikipedia gadget.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to