You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Paul Lindner <li...@inuus.com> on 2011/10/20 19:30:46 UTC

Review Request: Java hygiene for shindig

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

Review request for shindig.


Summary
-------

Java hygiene for shindig

* Fix javadoc @param references to renamed variables.
* Remove useless initializers for some variables and restructure some usages.
* Convert several idioms to use isEmpty().
* Use ternary expressions where they make sense.
* use chars for single-character strings.
* use StringUtils#split or Splitter#on instead of String#split regexes.
* use IOUtils#closeQuietly in finally blocks.
* other stylistic cleanups.


Diffs
-----

  /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
  /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 

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


Testing
-------

mvn  build/test, samplecontainer deployment.


Thanks,

Paul


Re: Review Request: Java hygiene for shindig

Posted by Paul Lindner <li...@inuus.com>.

> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java, line 89
> > <https://reviews.apache.org/r/2475/diff/1/?file=51556#file51556line89>
> >
> >     Is there a performance implication here?   I actually find the double quotes around a single quote easier on the eyes.

it's a pico-optimization to be sure.   For me it's more readable for anything except an escaped quote.


> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java, line 58
> > <https://reviews.apache.org/r/2475/diff/1/?file=51590#file51590line58>
> >
> >     StringBuilder?

nah.


> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java, line 200
> > <https://reviews.apache.org/r/2475/diff/1/?file=51588#file51588line200>
> >
> >     Should we use a StringBuilder here?

fixed.

yes, perfect use of stringbuilder.


> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java, line 106
> > <https://reviews.apache.org/r/2475/diff/1/?file=51577#file51577line106>
> >
> >     and here?

fixed.

another Objects.hashCode....


> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java, line 86
> > <https://reviews.apache.org/r/2475/diff/1/?file=51577#file51577line86>
> >
> >     Should we use a StringBuilder here?

fixed.

only if it makes it more readable.

Another use-case for Objects.hashCode()


> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java, line 174
> > <https://reviews.apache.org/r/2475/diff/1/?file=51576#file51576line174>
> >
> >     Should we use a StringBuilder here?

fixed.

Should use Joiner.on(":").join(...)

However here we can just use Objects.hashCode(...)


- Paul


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


On 2011-10-20 17:30:46, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 17:30:46)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Dan Dumont <dd...@us.ibm.com>.

> On 2011-10-20 18:09:06, Dan Dumont wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java, line 58
> > <https://reviews.apache.org/r/2475/diff/1/?file=51590#file51590line58>
> >
> >     StringBuilder?
> 
> Paul Lindner wrote:
>     nah.

Why not?  the review here doesn't show it, but it's a 3 line concat of 5 strings and 3 chars.


- Dan


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


On 2011-10-20 18:54:27, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 18:54:27)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2475/#review2708
-----------------------------------------------------------


I've also been meaning to ask you about the whitespace nits.  Is there an official policy on them?   I try to remove line ending white space any time I see them, I posted a regex to use with eclipse find/replace so I can remove that low hanging fruit from the review before I send it out.   Should I not be changing the white space if it already exists?  Though you didn't add any, there are some that show up close to the changes you've made.  Either way, just trying to understand how this *should* be handled for my future reviews.


/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java
<https://reviews.apache.org/r/2475/#comment6104>

    Is there a performance implication here?   I actually find the double quotes around a single quote easier on the eyes.



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java
<https://reviews.apache.org/r/2475/#comment6105>

    Should we use a StringBuilder here?



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java
<https://reviews.apache.org/r/2475/#comment6106>

    Should we use a StringBuilder here?



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java
<https://reviews.apache.org/r/2475/#comment6107>

    and here?



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java
<https://reviews.apache.org/r/2475/#comment6108>

    Should we use a StringBuilder here?



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java
<https://reviews.apache.org/r/2475/#comment6109>

    StringBuilder?


- Dan


On 2011-10-20 17:30:46, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 17:30:46)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Paul Lindner <li...@inuus.com>.

> On 2011-10-20 20:45:34, Jesse Ciancetta wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java, line 24
> > <https://reviews.apache.org/r/2475/diff/2/?file=51720#file51720line24>
> >
> >     Unused import

fixed


> On 2011-10-20 20:45:34, Jesse Ciancetta wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java, line 200
> > <https://reviews.apache.org/r/2475/diff/2/?file=51732#file51732line200>
> >
> >     Extra carriage return

fixed.


> On 2011-10-20 20:45:34, Jesse Ciancetta wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java, line 71
> > <https://reviews.apache.org/r/2475/diff/2/?file=51735#file51735line71>
> >
> >     Is there any reason this shouldn't use Objects.hashCode(...) like the others?

indeed.  done.


> On 2011-10-20 20:45:34, Jesse Ciancetta wrote:
> > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java, line 123
> > <https://reviews.apache.org/r/2475/diff/2/?file=51743#file51743line123>
> >
> >     Empty else block

fixed


- Paul


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


On 2011-10-20 18:54:27, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 18:54:27)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Jesse Ciancetta <jc...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2475/#review2722
-----------------------------------------------------------

Ship it!


Found a few small items which I noted -- other than those LGTM


/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java
<https://reviews.apache.org/r/2475/#comment6127>

    Unused import



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java
<https://reviews.apache.org/r/2475/#comment6130>

    Extra carriage return



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java
<https://reviews.apache.org/r/2475/#comment6126>

    Is there any reason this shouldn't use Objects.hashCode(...) like the others?



/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java
<https://reviews.apache.org/r/2475/#comment6129>

    Empty else block


- Jesse


On 2011-10-20 18:54:27, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 18:54:27)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2475/#review2724
-----------------------------------------------------------

Ship it!


- Henry


On 2011-10-20 18:54:27, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 18:54:27)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2475/#review2718
-----------------------------------------------------------

Ship it!


LGTM

- Ryan


On 2011-10-20 18:54:27, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2475/
> -----------------------------------------------------------
> 
> (Updated 2011-10-20 18:54:27)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Java hygiene for shindig
> 
> * Fix javadoc @param references to renamed variables.
> * Remove useless initializers for some variables and restructure some usages.
> * Convert several idioms to use isEmpty().
> * Use ternary expressions where they make sense.
> * use chars for single-character strings.
> * use StringUtils#split or Splitter#on instead of String#split regexes.
> * use IOUtils#closeQuietly in finally blocks.
> * other stylistic cleanups.
> 
> 
> Diffs
> -----
> 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
>   /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
>   /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
>   /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
>   /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 
> 
> Diff: https://reviews.apache.org/r/2475/diff
> 
> 
> Testing
> -------
> 
> mvn  build/test, samplecontainer deployment.
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Java hygiene for shindig

Posted by Paul Lindner <li...@inuus.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2475/
-----------------------------------------------------------

(Updated 2011-10-20 18:54:27.879369)


Review request for shindig.


Summary
-------

Java hygiene for shindig

* Fix javadoc @param references to renamed variables.
* Remove useless initializers for some variables and restructure some usages.
* Convert several idioms to use isEmpty().
* Use ternary expressions where they make sense.
* use chars for single-character strings.
* use StringUtils#split or Splitter#on instead of String#split regexes.
* use IOUtils#closeQuietly in finally blocks.
* other stylistic cleanups.


Diffs (updated)
-----

  /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java 1186576 
  /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java 1186576 
  /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java 1186576 
  /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java 1186576 
  /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1186576 

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


Testing
-------

mvn  build/test, samplecontainer deployment.


Thanks,

Paul