Hi Jesse, Do you want to update this patch to something that ready for review for commit?
I would love to have this patch and Dan's about the common container token to in before next beta or final 3.0 release. - Henry On Wed, Aug 24, 2011 at 7:47 AM, Jesse Ciancetta <[email protected]> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1632/ > ----------------------------------------------------------- > > 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! > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js > 1148651 > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js > 1151535 > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1150288 > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1157893 > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js > 1158700 > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java > 1098824 > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java > 1098824 > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java > 1125606 > > Diff: https://reviews.apache.org/r/1632/diff > > > Testing > ------- > > > Thanks, > > Jesse >
