----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1632/#review2310 -----------------------------------------------------------
Overall this looks good. It does lead me to wonder what your use cases are for the module ID. In the Shindig Java code I only ever see org.apache.shindig.auth.SecurityToken.getModuleId() used in org.apache.shindig.gadgets.http.AbstractHttpCache.getInstanceId(HttpRequest). What are the intentions of the module ID and do you have new use cases for its use? http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js <https://reviews.apache.org/r/1632/#comment5314> I'd rather we not set this by default and if we do, I'd rather it be a bit bigger, e.g., 60 seconds or more. - Stanton On 2011-09-28 19:36:42, Jesse Ciancetta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1632/ > ----------------------------------------------------------- > > (Updated 2011-09-28 19:36:42) > > > Review request for shindig. > > > 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 > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/service_test.js > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java > 1176946 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java > 1176946 > > Diff: https://reviews.apache.org/r/1632/diff > > > Testing > ------- > > > Thanks, > > Jesse > >
