shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Dumont" <ddum...@us.ibm.com>
Subject Re: Review Request: CommonContainer token refresh changes for a better UX when tokens expire.
Date Mon, 09 Jan 2012 21:18:06 GMT

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

(Updated 2012-01-09 21:18:06.698353)


Review request for shindig, Henry Saputra, Ryan Baxter, li xu, Jesse Ciancetta, and Stanton
Sievers.


Changes
-------

Hello again, 

In today's installment of "ModuleIds - Things I wish I never knew" (version 6 -> 7) we
have some constants moving to the constants cleanup, and some changes to the navigate chain
to (hopefully) account for a moduleId being set in the RENDER_PARAMS bag during navigate.

ModuleId < 0: Please generate me a moduleId with a new token and let me know what they
are
ModuleId == 0: No token extra token request.
ModuleId > 0: Get a token for a moduleId I've previously persisted.

If all goes according to plan, the moduleId would be parsed out, and if non-zero (default:
0) it would result in another request to get a security token for the gadget before the render
is complete.
Jesse, I'd really appreciate it if you could go over all of this...  Also, I think I might
let you make the update to batch the metadata and token requests(if you don't mind)... because
I think I'm starting to go cross-eyed looking at this code...

So for now it results in at worst 1 extra transaction for gadgets that specify a moduleId.
 But since that code path never existed before, I guess it shouldn't hurt shindig performance
as-is.
This hasn't really been tested (the moduleId path which shindig doesn't use yet) because shindig
doesn't have an impl for persisting moduleIds, so until I get that infra locally for our versions
of shindig, I don't know if I'll really be able to pound on this.  I hope that's ok.


Summary
-------

Initial review of 1st change.  Allowing common container to manage container token refreshes.
 Also, refresh of gadget security tokens will now wait for valid container security token
before trying to refresh.


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleIdManager.java
PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleIdManagerImpl.java
PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/service_test.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AbstractSecurityToken.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/feature.xml
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js
1222407 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js
1222407 

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


Testing
-------

Tested code in a private container with some examples of setting no refresh (ttl = 0) and
setting an initial token (if it was written by jsp page to avoid transaction) etc..


Thanks,

Dan


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message