You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stanton Sievers <si...@gmail.com> on 2013/09/01 15:13:40 UTC

Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

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

Review request for shindig.


Bugs: SHINDIG-1931
    https://issues.apache.org/jira/browse/SHINDIG-1931


Repository: shindig


Description
-------

Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.

[1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 

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


Testing
-------

Unit tests continue to pass.  Shindig starts and samples load in the sample container.


Thanks,

Stanton Sievers


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

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

Ship it!


Ship It!

- Ryan Baxter


On Sept. 1, 2013, 1:13 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13923/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2013, 1:13 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: SHINDIG-1931
>     https://issues.apache.org/jira/browse/SHINDIG-1931
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.
> 
> [1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
> 
> Diff: https://reviews.apache.org/r/13923/diff/
> 
> 
> Testing
> -------
> 
> Unit tests continue to pass.  Shindig starts and samples load in the sample container.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

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

Ship it!


Ship It!

- Ryan Baxter


On Sept. 4, 2013, 11:49 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13923/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2013, 11:49 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: SHINDIG-1931
>     https://issues.apache.org/jira/browse/SHINDIG-1931
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.
> 
> [1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1519267 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/resources/testEhCacheConfig.xml PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1519267 
> 
> Diff: https://reviews.apache.org/r/13923/diff/
> 
> 
> Testing
> -------
> 
> Unit tests continue to pass.  Shindig starts and samples load in the sample container.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13923/
-----------------------------------------------------------

(Updated Sept. 4, 2013, 11:49 p.m.)


Review request for shindig.


Changes
-------

Added a unit test that reproduces the issue.  I decided to add Powermock as a test-scoped dependency to help get at the CacheManager.  I figure we can make use of it in other unit tests moving forward as well.


Bugs: SHINDIG-1931
    https://issues.apache.org/jira/browse/SHINDIG-1931


Repository: shindig


Description
-------

Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.

[1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
  http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java 1519267 
  http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/resources/testEhCacheConfig.xml PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1519267 

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


Testing
-------

Unit tests continue to pass.  Shindig starts and samples load in the sample container.


Thanks,

Stanton Sievers


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

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

Ship it!


Ship It!

- Henry Saputra


On Sept. 1, 2013, 1:13 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13923/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2013, 1:13 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: SHINDIG-1931
>     https://issues.apache.org/jira/browse/SHINDIG-1931
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.
> 
> [1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
> 
> Diff: https://reviews.apache.org/r/13923/diff/
> 
> 
> Testing
> -------
> 
> Unit tests continue to pass.  Shindig starts and samples load in the sample container.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

Posted by Stanton Sievers <si...@gmail.com>.

> On Sept. 3, 2013, 1:19 a.m., Doug Davies wrote:
> > Thanks for looking at this so quickly. My only comment would be should there be a unit test that shows the original problem and that this resolves it? Not sure how easy it would be to write a unit test that would cause the conflict. At any rate... looks good... until we upgrade again I will continue to use our patched EhCacheCacheProvider.

I'll think about how I can write some unit tests and get back to you.  The easiest way would be to get the CacheManager object itself from the provider, but that's not supported today by the EhCacheCacheProvider.  It's something I'll look into though.


- Stanton


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


On Sept. 1, 2013, 1:13 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13923/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2013, 1:13 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: SHINDIG-1931
>     https://issues.apache.org/jira/browse/SHINDIG-1931
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.
> 
> [1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
> 
> Diff: https://reviews.apache.org/r/13923/diff/
> 
> 
> Testing
> -------
> 
> Unit tests continue to pass.  Shindig starts and samples load in the sample container.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>


Re: Review Request 13923: EhCache provider always uses a singleton cache manager, even if it is the wrong one

Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13923/#review25842
-----------------------------------------------------------

Ship it!


Thanks for looking at this so quickly. My only comment would be should there be a unit test that shows the original problem and that this resolves it? Not sure how easy it would be to write a unit test that would cause the conflict. At any rate... looks good... until we upgrade again I will continue to use our patched EhCacheCacheProvider.

- Doug Davies


On Sept. 1, 2013, 1:13 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13923/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2013, 1:13 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: SHINDIG-1931
>     https://issues.apache.org/jira/browse/SHINDIG-1931
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Reported on the dev list[1].  The issue is that the CacheManager.create(Configuration) APIs will always use the same singleton, regardless of whether the provided Configuration defines a cache manager of a different name then the existing singleton.  We should use the CacheManager.newInstance(Configuration) APIs, as this will use a singleton per cache manager name.  This will keep us from stomping on other cache managers that exist in the environment.
> 
> [1] http://markmail.org/message/4adopzvvi3ltv7yq?q=Shindig+list:org.apache.shindig.dev+order:date-backward&page=1
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java 1519267 
> 
> Diff: https://reviews.apache.org/r/13923/diff/
> 
> 
> Testing
> -------
> 
> Unit tests continue to pass.  Shindig starts and samples load in the sample container.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>