Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

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

(Updated Sept. 25, 2012, 4:40 p.m.)


Review request for shindig, Stanton Sievers and Brian Lillie.


Changes
---

Addresses Brians comments about origination on the AnonymousSecurityToken.


Description
---

When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
should be rewritten (like we do for token operations.)


This addresses bug SHINDIG-1801.
https://issues.apache.org/jira/browse/SHINDIG-1801


Diffs (updated)
-

  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java
 1389848 
  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
 1389848 
  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java
 1389848 
  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java
 1389848 
  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java
 1389848 

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


Testing
---

All unit tests pass


Thanks,

Adam Clarke

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Dan Dumont]

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


Adam, any update or response to address Brian's comments?

- Dan Dumont


On June 22, 2012, 11:18 a.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 22, 2012, 11:18 a.m.)
> 
> 
> Review request for shindig, Stanton Sievers and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1352576 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Brian Lillie]

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


The CodeAuthorizationResponseHandler picks up the gadgetUri from the accessor 
and uses that as the appUrl for the AnonymousSecurityToken associated with the 
subsequent request.   For the shared token state, because we are using that 
value, we have lost the associability to the gadget that is making the request.

- Brian Lillie


On June 22, 2012, 11:18 a.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 22, 2012, 11:18 a.m.)
> 
> 
> Review request for shindig, Stanton Sievers and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1352576 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

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

(Updated June 22, 2012, 11:18 a.m.)


Review request for shindig, Stanton Sievers and Brian Lillie.


Description
---

When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
should be rewritten (like we do for token operations.)


This addresses bug SHINDIG-1801.
https://issues.apache.org/jira/browse/SHINDIG-1801


Diffs
-

  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
 1352576 

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


Testing
---

All unit tests pass


Thanks,

Adam Clarke

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

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

(Updated June 21, 2012, 3:56 p.m.)


Review request for shindig and Brian Lillie.


Description
---

When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
should be rewritten (like we do for token operations.)


This addresses bug SHINDIG-1801.
https://issues.apache.org/jira/browse/SHINDIG-1801


Diffs
-

  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
 1352576 

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


Testing
---

All unit tests pass


Thanks,

Adam Clarke

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

> On June 19, 2012, 4:31 p.m., Ryan Baxter wrote:
> > Adam I noticed we are lacking some unit tests for this method (and most of 
> > the class in general).  This might be a good opportunity to add some unit 
> > tests.

I agree with you that we need some unit tests here, but (as you're probably 
aware) we're enhancing the shared token feature.  It'll include more server and 
client-side updates and I plan on adding the tests once the whole thing is in 
place.  This was just a quick update for an obvious problem.


- Adam


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


On June 21, 2012, 3:53 p.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 21, 2012, 3:53 p.m.)
> 
> 
> Review request for shindig and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1352576 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

> On June 19, 2012, 1:39 p.m., Dan Dumont wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java,
> >  line 182
> > 
> >
> > Could you change this variable name to something more meaningful?
> > 
> > Or perhaps just avoid it completely, and pass the get directly into the 
> > set.

Changed to processedGadgetUri


- Adam


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


On June 21, 2012, 3:53 p.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 21, 2012, 3:53 p.m.)
> 
> 
> Review request for shindig and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1352576 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Adam Clarke]

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

(Updated June 21, 2012, 3:53 p.m.)


Review request for shindig and Brian Lillie.


Changes
---

Updated for Dan's feedback


Description
---

When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
should be rewritten (like we do for token operations.)


This addresses bug SHINDIG-1801.
https://issues.apache.org/jira/browse/SHINDIG-1801


Diffs (updated)
-

  
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
 1352576 

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


Testing
---

All unit tests pass


Thanks,

Adam Clarke

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Ryan Baxter]

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


Adam I noticed we are lacking some unit tests for this method (and most of the 
class in general).  This might be a good opportunity to add some unit tests.

- Ryan Baxter


On June 19, 2012, 1:15 p.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 19, 2012, 1:15 p.m.)
> 
> 
> Review request for shindig, Stanton Sievers and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1351704 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>

Re: Review Request: Code Review for JIRA 1801, shared tokens should share accessors

[Dan Dumont]

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



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java


Could you change this variable name to something more meaningful?

Or perhaps just avoid it completely, and pass the get directly into the set.


- Dan Dumont


On June 19, 2012, 1:15 p.m., Adam Clarke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5420/
> ---
> 
> (Updated June 19, 2012, 1:15 p.m.)
> 
> 
> Review request for shindig, Stanton Sievers and Brian Lillie.
> 
> 
> Description
> ---
> 
> When looking up an OAuth2Accessor in a shared token scenario the gadgetUri 
> should be rewritten (like we do for token operations.)
> 
> 
> This addresses bug SHINDIG-1801.
> https://issues.apache.org/jira/browse/SHINDIG-1801
> 
> 
> Diffs
> -
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
>  1351704 
> 
> Diff: https://reviews.apache.org/r/5420/diff/
> 
> 
> Testing
> ---
> 
> All unit tests pass
> 
> 
> Thanks,
> 
> Adam Clarke
> 
>