----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1632/ -----------------------------------------------------------
(Updated 2011-12-14 21:13:40.309607) Review request for shindig. Changes ------- I've attached an updated patch which has been realigned with the current shindig trunk. More details can be found in the review description and previous comments. I think the sticking point we had on this last time was a concern that we might not want to be using siteId for the moduleId -- but I'm thinking maybe we could start there and then update later to introduce a specific moduleId property somewhere if we find that it does actually need to be different. Summary ------- Common container currently doesn't include the siteId (moduleId) in any of it's security token processing/handling (all security tokens in common container currently get minted with a moduleId of 0). This patch is a first (rough) cut at getting moduleId's into security tokens. I am posting it somewhat prematurely to solicit feedback before I invest any more time in finishing up the last bits. Here are the things that I know are still left to do: -- Update unit tests on both the JS and Java side -- currently I've been building and deploying with skipTests=true... -- Figure out a strategy for dealing with preloaded gadgets. The current auth-refresh process maintains tokens for preloaded gadgets, however the preoad JS functions just take a gadgetUrl so there is no concept of a siteId (moduleId) for them at this time. -- Figure out how to get the token that is included in the original iframe to include a moduleId. I think the token in the iframe likely comes back in the metadata request (although I haven't looked yet to verify), which means that the call to the metadata service would likely need to include the moduleId as well. I'd greatly appreciate any comments people have on the patch and strategies for dealing with the outstanding issues noted above. Thanks! This addresses bugs RAVE-198 and SHINDIG-1611. https://issues.apache.org/jira/browse/RAVE-198 https://issues.apache.org/jira/browse/SHINDIG-1611 Diffs (updated) ----- http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/service_test.js 1214246 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1214246 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1214246 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1214246 Diff: https://reviews.apache.org/r/1632/diff Testing ------- Thanks, Jesse