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

Ship it!


LGTM. only one small nit. Thanks!


trunk/java/common/src/test/java/org/apache/shindig/common/servlet/BasicAuthorityTest.java
<https://reviews.apache.org/r/1676/#comment3847>

    Please replace "Provider" with "Authority". 


- li


On 2011-08-29 23:14:02, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1676/
> -----------------------------------------------------------
> 
> (Updated 2011-08-29 23:14:02)
> 
> 
> Review request for shindig and li xu.
> 
> 
> Summary
> -------
> 
> Currently the code inject BasicAuthorityProvider to allow Shindig Java to be 
> deploy in non root environment. 
> 
> However this is not necessary since the BasicAuthority is bind to Authority 
> in DefaultGuiceModule: 
> 
> bind(Authority.class).toProvider(BasicAuthorityProvider.class); 
> 
> we just need to inject Authority instance itself because we dont really need 
> multiple instances of Authority which is the main purpose of the Provider 
> injection. 
> 
> Implementation of Authority such as BasicAuthority has to be immutable and 
> thread safe.
> 
> 
> This addresses bug SHINDIG-1604.
>     https://issues.apache.org/jira/browse/SHINDIG-1604
> 
> 
> Diffs
> -----
> 
>   
> trunk/java/common/src/main/java/org/apache/shindig/common/servlet/BasicAuthority.java
>  1162967 
>   
> trunk/java/common/src/main/java/org/apache/shindig/common/servlet/BasicAuthorityProvider.java
>  1162967 
>   
> trunk/java/common/src/test/java/org/apache/shindig/common/servlet/BasicAuthorityProviderTest.java
>  1162967 
>   
> trunk/java/common/src/test/java/org/apache/shindig/common/servlet/BasicAuthorityTest.java
>  PRE-CREATION 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java
>  1162967 
>   
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
>  1162967 
>   
> trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
>  1162967 
> 
> Diff: https://reviews.apache.org/r/1676/diff
> 
> 
> Testing
> -------
> 
> Updated the JUnit test case to reflect the change
> 
> 
> Thanks,
> 
> Henry
> 
>

Reply via email to