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 2012/01/04 23:03:24 UTC

Review Request: Large refactoring of Shindig caching functionality

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

Review request for shindig.


Summary
-------

Lots going on here.  Looking for an initial sanity check, plus interested in people who can verify that EhCache stuff is decent, plus maybe help with the background reloading impl...

Specifically:
* Upgrade to guava 11.0
* MapMaker caches are being replaced with com.google.common.cache.* classes.
- convert over many small usages to use CacheBuilder and/or a CacheLoader where it makes sense.

* Bigger change is the removal of the Shindig Cache interface in deference to the Guava one.  It's semantically similar and just better.
* Modified CacheProvider to provide a way to create a Cache with a CacheLoader
* Removed LruCache and put the functionality into LruCacheProvider
* Removed SoftExpiringCache, it's ugly and the implementation was really awful, especially if you were using ehcache or memcache.
* AbstractSpecFactory changed a lot.  It now uses a separate negative cache for exceptions and uses CacheLoaders's reload() method to do stale reloads in the background.
* AbstractSpecFactory doesn't blow away the cache when using nocache=1
* Fixed a bunch of brittle tests.
* Removed a mapmaker in the gadgetspec that was only used for storing the gadgetspecification value.


Diffs
-----

  trunk/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/Cache.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/CacheProvider.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCache.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCacheProvider.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/NullCache.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/SoftExpiringCache.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhConfiguredCache.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/expressions/juel/JuelProvider.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java 1226951 
  trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/GuiceBeanProvider.java 1226951 
  trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheProviderTest.java 1226951 
  trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheTest.java 1226951 
  trunk/java/common/src/test/java/org/apache/shindig/common/cache/SoftExpiringCacheTest.java 1226951 
  trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/DefaultConfigProcessor.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/OsapiServicesConfigContributor.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/TestFeatureRegistry.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultHttpCache.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultInvalidationService.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssLexerParser.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssParser.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultRpcServiceLookup.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCache.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/ContainerTagLibraryFactory.java 1226951 
  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java 1226951 
  trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultHttpCacheTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultInvalidationServiceTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/testing/FakeOAuthServiceProvider.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/ProxyRendererTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitorTest.java 1226951 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/GadgetSpecTest.java 1226951 
  trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java 1226951 
  trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/oauth/SampleOAuthDataStore.java 1226951 
  trunk/pom.xml 1226951 

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


Testing
-------

unit only so far..


Thanks,

Paul


Re: Review Request: Large refactoring of Shindig caching functionality

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


I'd like to give something more meaningful than whitespace nits, but I haven't spent any time in the cache impl in Shindig yet. :(

- Dan


On 2012-01-04 22:03:24, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3396/
> -----------------------------------------------------------
> 
> (Updated 2012-01-04 22:03:24)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Lots going on here.  Looking for an initial sanity check, plus interested in people who can verify that EhCache stuff is decent, plus maybe help with the background reloading impl...
> 
> Specifically:
> * Upgrade to guava 11.0
> * MapMaker caches are being replaced with com.google.common.cache.* classes.
> - convert over many small usages to use CacheBuilder and/or a CacheLoader where it makes sense.
> 
> * Bigger change is the removal of the Shindig Cache interface in deference to the Guava one.  It's semantically similar and just better.
> * Modified CacheProvider to provide a way to create a Cache with a CacheLoader
> * Removed LruCache and put the functionality into LruCacheProvider
> * Removed SoftExpiringCache, it's ugly and the implementation was really awful, especially if you were using ehcache or memcache.
> * AbstractSpecFactory changed a lot.  It now uses a separate negative cache for exceptions and uses CacheLoaders's reload() method to do stale reloads in the background.
> * AbstractSpecFactory doesn't blow away the cache when using nocache=1
> * Fixed a bunch of brittle tests.
> * Removed a mapmaker in the gadgetspec that was only used for storing the gadgetspecification value.
> 
> 
> Diffs
> -----
> 
>   trunk/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/Cache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/CacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/NullCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/SoftExpiringCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhConfiguredCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/expressions/juel/JuelProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/GuiceBeanProvider.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheProviderTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/SoftExpiringCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/DefaultConfigProcessor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/OsapiServicesConfigContributor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/TestFeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultInvalidationService.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssLexerParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultRpcServiceLookup.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/ContainerTagLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultInvalidationServiceTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/testing/FakeOAuthServiceProvider.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/ProxyRendererTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitorTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/GadgetSpecTest.java 1226951 
>   trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java 1226951 
>   trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/oauth/SampleOAuthDataStore.java 1226951 
>   trunk/pom.xml 1226951 
> 
> Diff: https://reviews.apache.org/r/3396/diff
> 
> 
> Testing
> -------
> 
> unit only so far..
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Large refactoring of Shindig caching functionality

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


Paul would it make sense to break this one review up into smaller reviews?  Like you said there is a lot going on in these changes, maybe breaking them up into smaller reviews may make things easier to digest.

- Ryan


On 2012-01-04 22:03:24, Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3396/
> -----------------------------------------------------------
> 
> (Updated 2012-01-04 22:03:24)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Lots going on here.  Looking for an initial sanity check, plus interested in people who can verify that EhCache stuff is decent, plus maybe help with the background reloading impl...
> 
> Specifically:
> * Upgrade to guava 11.0
> * MapMaker caches are being replaced with com.google.common.cache.* classes.
> - convert over many small usages to use CacheBuilder and/or a CacheLoader where it makes sense.
> 
> * Bigger change is the removal of the Shindig Cache interface in deference to the Guava one.  It's semantically similar and just better.
> * Modified CacheProvider to provide a way to create a Cache with a CacheLoader
> * Removed LruCache and put the functionality into LruCacheProvider
> * Removed SoftExpiringCache, it's ugly and the implementation was really awful, especially if you were using ehcache or memcache.
> * AbstractSpecFactory changed a lot.  It now uses a separate negative cache for exceptions and uses CacheLoaders's reload() method to do stale reloads in the background.
> * AbstractSpecFactory doesn't blow away the cache when using nocache=1
> * Fixed a bunch of brittle tests.
> * Removed a mapmaker in the gadgetspec that was only used for storing the gadgetspecification value.
> 
> 
> Diffs
> -----
> 
>   trunk/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/Cache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/CacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/NullCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/SoftExpiringCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhConfiguredCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/expressions/juel/JuelProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/GuiceBeanProvider.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheProviderTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/SoftExpiringCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/DefaultConfigProcessor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/OsapiServicesConfigContributor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/TestFeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultInvalidationService.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssLexerParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultRpcServiceLookup.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/ContainerTagLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultInvalidationServiceTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/testing/FakeOAuthServiceProvider.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/ProxyRendererTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitorTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/GadgetSpecTest.java 1226951 
>   trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java 1226951 
>   trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/oauth/SampleOAuthDataStore.java 1226951 
>   trunk/pom.xml 1226951 
> 
> Diff: https://reviews.apache.org/r/3396/diff
> 
> 
> Testing
> -------
> 
> unit only so far..
> 
> 
> Thanks,
> 
> Paul
> 
>


Re: Review Request: Large refactoring of Shindig caching functionality

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


Hi Paul.   Any chance we could break this up into smaller reviews?

It looks like we've already moved past 11.0 for guava (pom shows us at 11.0.2
Do you want to wait to put this in till post 2.5?

- Dan Dumont


On Jan. 4, 2012, 10:03 p.m., Paul Lindner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3396/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2012, 10:03 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Description
> -------
> 
> Lots going on here.  Looking for an initial sanity check, plus interested in people who can verify that EhCache stuff is decent, plus maybe help with the background reloading impl...
> 
> Specifically:
> * Upgrade to guava 11.0
> * MapMaker caches are being replaced with com.google.common.cache.* classes.
> - convert over many small usages to use CacheBuilder and/or a CacheLoader where it makes sense.
> 
> * Bigger change is the removal of the Shindig Cache interface in deference to the Guava one.  It's semantically similar and just better.
> * Modified CacheProvider to provide a way to create a Cache with a CacheLoader
> * Removed LruCache and put the functionality into LruCacheProvider
> * Removed SoftExpiringCache, it's ugly and the implementation was really awful, especially if you were using ehcache or memcache.
> * AbstractSpecFactory changed a lot.  It now uses a separate negative cache for exceptions and uses CacheLoaders's reload() method to do stale reloads in the background.
> * AbstractSpecFactory doesn't blow away the cache when using nocache=1
> * Fixed a bunch of brittle tests.
> * Removed a mapmaker in the gadgetspec that was only used for storing the gadgetspecification value.
> 
> 
> Diffs
> -----
> 
>   trunk/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/Cache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/CacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/LruCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/NullCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/SoftExpiringCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhConfiguredCache.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/expressions/juel/JuelProvider.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java 1226951 
>   trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/GuiceBeanProvider.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheProviderTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/LruCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/SoftExpiringCacheTest.java 1226951 
>   trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/DefaultConfigProcessor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/OsapiServicesConfigContributor.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/TestFeatureRegistry.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultHttpCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultInvalidationService.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssLexerParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssParser.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultRpcServiceLookup.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCache.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/ContainerTagLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java 1226951 
>   trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultHttpCacheTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultInvalidationServiceTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/testing/FakeOAuthServiceProvider.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/ProxyRendererTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitorTest.java 1226951 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/GadgetSpecTest.java 1226951 
>   trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java 1226951 
>   trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/oauth/SampleOAuthDataStore.java 1226951 
>   trunk/pom.xml 1226951 
> 
> Diff: https://reviews.apache.org/r/3396/diff/
> 
> 
> Testing
> -------
> 
> unit only so far..
> 
> 
> Thanks,
> 
> Paul Lindner
> 
>