You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jason Huynh <jh...@pivotal.io> on 2016/08/17 16:34:56 UTC
Review Request 51184: GEODE-1675: Fix ClassCastException in
ClientServer expiry. Also fix for NPE if manager is null
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51184/
-----------------------------------------------------------
Review request for geode and Dan Smith.
Repository: geode
Description
-------
Fix possible npe with manager being null
Fix ClassCastException with Client/Server expiration
Added new client server tests.
Refactored peer to peer tests to have a common base with client server. This causes the tests to run a bit longer (due to extending JUnit4DistributedTestCase needing @Before and @After. This requires the server and tomcat server to be restarted for each test due to the framework shutting down the server every test)
Diffs
-----
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java aa07b5b
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java 76b301d
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsClientServerJUnitTest.java PRE-CREATION
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java df65690
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java c14f829
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/ClientServerCache.java f8cff16
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/PeerToPeerCache.java b0b3b4a
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/AbstractCacheLifecycleListener.java 1019ddc
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java ff7133d
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java 1d3d1f7
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java 5893c95
geode-junit/src/test/java/com/gemstone/gemfire/test/junit/rules/examples/RuleAndClassRuleTest.java 4e69ec3
Diff: https://reviews.apache.org/r/51184/diff/
Testing
-------
Thanks,
Jason Huynh
Re: Review Request 51184: GEODE-1675: Fix ClassCastException in
ClientServer expiry. Also fix for NPE if manager is null
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51184/#review146000
-----------------------------------------------------------
Ship it!
Other than the test changes mentioned in your review (using a different port, fixing the categories, etc.), it looks good to me! Once that stuff is fixed go ahead and ship it.
- Dan Smith
On Aug. 17, 2016, 4:34 p.m., Jason Huynh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51184/
> -----------------------------------------------------------
>
> (Updated Aug. 17, 2016, 4:34 p.m.)
>
>
> Review request for geode and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Fix possible npe with manager being null
> Fix ClassCastException with Client/Server expiration
> Added new client server tests.
> Refactored peer to peer tests to have a common base with client server. This causes the tests to run a bit longer (due to extending JUnit4DistributedTestCase needing @Before and @After. This requires the server and tomcat server to be restarted for each test due to the framework shutting down the server every test)
>
>
> Diffs
> -----
>
> extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java aa07b5b
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java 76b301d
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsClientServerJUnitTest.java PRE-CREATION
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java df65690
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java c14f829
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/ClientServerCache.java f8cff16
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/PeerToPeerCache.java b0b3b4a
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/AbstractCacheLifecycleListener.java 1019ddc
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java ff7133d
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java 1d3d1f7
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java 5893c95
> geode-junit/src/test/java/com/gemstone/gemfire/test/junit/rules/examples/RuleAndClassRuleTest.java 4e69ec3
>
> Diff: https://reviews.apache.org/r/51184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Huynh
>
>
Re: Review Request 51184: GEODE-1675: Fix ClassCastException in
ClientServer expiry. Also fix for NPE if manager is null
Posted by Jason Huynh <jh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51184/#review145998
-----------------------------------------------------------
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsClientServerJUnitTest.java (line 37)
<https://reviews.apache.org/r/51184/#comment212354>
Change to dunit test and change category -from Dan
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsClientServerJUnitTest.java (line 62)
<https://reviews.apache.org/r/51184/#comment212355>
Investigate how to use a different port
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java (line 40)
<https://reviews.apache.org/r/51184/#comment212356>
Rename and recategorize test
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java (line 94)
<https://reviews.apache.org/r/51184/#comment212357>
Possibly remove commented out code or investigate why closeCache does not work here.
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/AbstractCacheLifecycleListener.java (line 20)
<https://reviews.apache.org/r/51184/#comment212358>
Remove this
geode-junit/src/test/java/com/gemstone/gemfire/test/junit/rules/examples/RuleAndClassRuleTest.java (line 79)
<https://reviews.apache.org/r/51184/#comment212359>
Remove this diff
- Jason Huynh
On Aug. 17, 2016, 4:34 p.m., Jason Huynh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51184/
> -----------------------------------------------------------
>
> (Updated Aug. 17, 2016, 4:34 p.m.)
>
>
> Review request for geode and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Fix possible npe with manager being null
> Fix ClassCastException with Client/Server expiration
> Added new client server tests.
> Refactored peer to peer tests to have a common base with client server. This causes the tests to run a bit longer (due to extending JUnit4DistributedTestCase needing @Before and @After. This requires the server and tomcat server to be restarted for each test due to the framework shutting down the server every test)
>
>
> Diffs
> -----
>
> extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java aa07b5b
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java 76b301d
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsClientServerJUnitTest.java PRE-CREATION
> extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java df65690
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java c14f829
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/ClientServerCache.java f8cff16
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/PeerToPeerCache.java b0b3b4a
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/AbstractCacheLifecycleListener.java 1019ddc
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java ff7133d
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java 1d3d1f7
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java 5893c95
> geode-junit/src/test/java/com/gemstone/gemfire/test/junit/rules/examples/RuleAndClassRuleTest.java 4e69ec3
>
> Diff: https://reviews.apache.org/r/51184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Huynh
>
>