You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Henry Saputra <hs...@apache.org> on 2012/03/02 00:38:41 UTC
Review Request: Remove throwing UnsupportedOperationException with call to
BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
Simplify the SecurityToken.getActiveUrl implementation and definition.
If not set simply return null should be suffice.
This addresses bug SHINDIG-1719.
https://issues.apache.org/jira/browse/SHINDIG-1719
Diffs
-----
trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
Diff: https://reviews.apache.org/r/4135/diff
Testing
-------
Thanks,
Henry
Re: Review Request: Remove throwing UnsupportedOperationException with call
to BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
Posted by Henry Saputra <hs...@apache.org>.
> On 2012-03-02 12:52:49, Stanton Sievers wrote:
> > Hi Henry,
> >
> > You should also remove the logic in org.apache.shindig.auth.BlobCrypterSecurityToken.fromToken(SecurityToken) where we eat the UnsupportedOperationException when calling getActiveUrl().
> >
> > I'm also looking at org.apache.shindig.gadgets.oauth.GadgetOAuthCallbackGenerator.checkGadgetCanRender(SecurityToken, OAuthArguments, OAuthResponseParams) and it seems like a value of "null" here is going to throw other exceptions when create a Uri.parse. I'm wondering if we can handle "null" more gracefully here... although we really weren't handling UnsupportedOperationException gracefully here either.
> >
> > Do you know why the exception was being thrown in the first place? Looking at the rest of the APIs in SecurityToken that one strikes me as odd.
Thanks for the BlobCrypterSecurityToken.fromToken change.
Yeah, I am aware of the checkGadgetCanRender call. I wouldnt worry about it tho bc most of times the UrlParameterAuthenticationHandler should do its job to inject the right activeUrl to the token.
- Henry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/#review5556
-----------------------------------------------------------
On 2012-03-01 23:38:41, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4135/
> -----------------------------------------------------------
>
> (Updated 2012-03-01 23:38:41)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Simplify the SecurityToken.getActiveUrl implementation and definition.
>
> If not set simply return null should be suffice.
>
>
> This addresses bug SHINDIG-1719.
> https://issues.apache.org/jira/browse/SHINDIG-1719
>
>
> Diffs
> -----
>
> trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
>
> Diff: https://reviews.apache.org/r/4135/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: Remove throwing UnsupportedOperationException with call
to BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/#review5556
-----------------------------------------------------------
Hi Henry,
You should also remove the logic in org.apache.shindig.auth.BlobCrypterSecurityToken.fromToken(SecurityToken) where we eat the UnsupportedOperationException when calling getActiveUrl().
I'm also looking at org.apache.shindig.gadgets.oauth.GadgetOAuthCallbackGenerator.checkGadgetCanRender(SecurityToken, OAuthArguments, OAuthResponseParams) and it seems like a value of "null" here is going to throw other exceptions when create a Uri.parse. I'm wondering if we can handle "null" more gracefully here... although we really weren't handling UnsupportedOperationException gracefully here either.
Do you know why the exception was being thrown in the first place? Looking at the rest of the APIs in SecurityToken that one strikes me as odd.
- Stanton
On 2012-03-01 23:38:41, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4135/
> -----------------------------------------------------------
>
> (Updated 2012-03-01 23:38:41)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Simplify the SecurityToken.getActiveUrl implementation and definition.
>
> If not set simply return null should be suffice.
>
>
> This addresses bug SHINDIG-1719.
> https://issues.apache.org/jira/browse/SHINDIG-1719
>
>
> Diffs
> -----
>
> trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
>
> Diff: https://reviews.apache.org/r/4135/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: Remove throwing UnsupportedOperationException with call
to BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/#review5604
-----------------------------------------------------------
Ship it!
LGTM
- Stanton
On 2012-03-02 19:56:52, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4135/
> -----------------------------------------------------------
>
> (Updated 2012-03-02 19:56:52)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Simplify the SecurityToken.getActiveUrl implementation and definition.
>
> If not set simply return null should be suffice.
>
>
> This addresses bug SHINDIG-1719.
> https://issues.apache.org/jira/browse/SHINDIG-1719
>
>
> Diffs
> -----
>
> trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
>
> Diff: https://reviews.apache.org/r/4135/diff
>
>
> Testing
> -------
>
> Updated unit test and run apps in common container
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: Remove throwing UnsupportedOperationException with call
to BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/#review5615
-----------------------------------------------------------
Committed as 1297157
- Henry
On 2012-03-02 19:56:52, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4135/
> -----------------------------------------------------------
>
> (Updated 2012-03-02 19:56:52)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Simplify the SecurityToken.getActiveUrl implementation and definition.
>
> If not set simply return null should be suffice.
>
>
> This addresses bug SHINDIG-1719.
> https://issues.apache.org/jira/browse/SHINDIG-1719
>
>
> Diffs
> -----
>
> trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
> trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
>
> Diff: https://reviews.apache.org/r/4135/diff
>
>
> Testing
> -------
>
> Updated unit test and run apps in common container
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: Remove throwing UnsupportedOperationException with call
to BlobCrypterSecurityToken.getActiveUrl and BasicSecurityToken.getActiveUrl
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/
-----------------------------------------------------------
(Updated 2012-03-02 19:56:52.007316)
Review request for shindig.
Changes
-------
Updated diff per review from Stanton
Summary
-------
Simplify the SecurityToken.getActiveUrl implementation and definition.
If not set simply return null should be suffice.
This addresses bug SHINDIG-1719.
https://issues.apache.org/jira/browse/SHINDIG-1719
Diffs (updated)
-----
trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java 1295758
trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 1295758
trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java 1295758
trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1295758
Diff: https://reviews.apache.org/r/4135/diff
Testing (updated)
-------
Updated unit test and run apps in common container
Thanks,
Henry