shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stanton Sievers" <siever...@gmail.com>
Subject Re: Review Request: CommonContainer token refresh changes for a better UX when tokens expire.
Date Tue, 10 Jan 2012 13:00:45 GMT

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


Overall this looks good to me.  I have some questions about setting the module id as the fragment,
since I think there are cases where the fragment gets used for an rpctoken or an ifpctok...
you can see it in the gadget_holder feature code.  I'm not sure how/why this is used but it's
certainly worth looking into, as I think your code right now would kill off that piece of
the fragment.


http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
<https://reviews.apache.org/r/3180/#comment9667>

    I'm having doubts about setting the fragment to be the module id.  I thought we already
used the fragment for the rpctoken in some cases (for like IFPC??).  Will this overwrite that
piece if it's set on the url?  Can you use the "setFP" API instead of "setFragment"?



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js
<https://reviews.apache.org/r/3180/#comment9668>

    Whether for this review or not, I think the ServiceConfig constants, like API_HOST and
API_PATH should be added here as well.


- Stanton


On 2012-01-09 21:18:06, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3180/
> -----------------------------------------------------------
> 
> (Updated 2012-01-09 21:18:06)
> 
> 
> Review request for shindig, Henry Saputra, Ryan Baxter, li xu, Jesse Ciancetta, and Stanton
Sievers.
> 
> 
> 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
> -----
> 
>   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