shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jesse Ciancetta" <jc...@mitre.org>
Subject Re: Review Request: CommonContainer token refresh changes for a better UX when tokens expire.
Date Tue, 10 Jan 2012 18:48:26 GMT

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



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

    This code (or something very similar) ends up repeated in a bunch of different places
-- it might be worth making a utility function somewhere which takes the gadget URL and moduleId
and returns the properly formatted string.



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

    You may be able to get away from using this function for gadget token refresh and maybe
even eliminate it entirely -- see my next comment around osapi.container.Container.prototype.refreshTokens_()



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

    In the patch I was working on for moduleId's I took a different approach here which I
think you may be able to do as well.  I think in my patch -- any all the gadget tokens we
had already fetched either by pre-load or navigating a gadget (which would make them eligible
for refresh) were cached in the Service object -- so it seemed to me that knowing what tokens
needed refresh was a simple matter of getting the keys from the gadget token cache map in
the Service object.
    
    It looks like your keying the gadget token cache in the Service object with the gadget
spec URL + moduleId as well (or just a raw gadget spec URL in the case of no moduleId) so
it *seems* like that strategy might work for you here too.



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

    This looks like it is supposed to be:
    
    url.setFragment(moduleId)



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

    Is this supposed to be calling url.toString() instead of keying the cache with the actual
shindig.uri object?


- Jesse


On 2012-01-10 14:59:57, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3180/
> -----------------------------------------------------------
> 
> (Updated 2012-01-10 14:59:57)
> 
> 
> 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.
> 
> 
> This addresses bug SHINDIG-1681.
>     https://issues.apache.org/jira/browse/SHINDIG-1681
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.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/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.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.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/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/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/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/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/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java
1222407 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java
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