----------------------------------------------------------- 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 > >