> On June 15, 2016, 9:34 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java, > > line 815 > > <https://reviews.apache.org/r/48738/diff/1/?file=1420204#file1420204line815> > > > > You could probably get rid of the nested if conditions here: > > > > // Check if its AEQ and is configured to forward expiration > > destroy events. > > if (event.getOperation().isExpiration() && this.isAsyncEventQueue() > > && this.isForwardExpirationDestroy()) { > > return true; > > } > > > > > > if (event.getOperation().isLocal() || > > event.getOperation().isExpiration()) { > > return false; > > }
The reason I added is to have eviction/expiration in the same context...I can break it down... > On June 15, 2016, 9:34 p.m., Dan Smith wrote: > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java, > > line 283 > > <https://reviews.apache.org/r/48738/diff/1/?file=1420216#file1420216line283> > > > > If you use an assertion in the below awaility clause, you woudln't need > > a System.out.println. This is for debugging...Forgot to remove... > On June 15, 2016, 9:34 p.m., Dan Smith wrote: > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java, > > line 240 > > <https://reviews.apache.org/r/48738/diff/1/?file=1420216#file1420216line240> > > > > This is one big test method with a lot of boolean flags to check > > different conditions. Maybe this should be broken into smaller, simpler > > test methods? For example checkForInvalidOp could be it's own method, > > rather than a flag. I am breaking it down...All the validations are not required in all methods...I am moving validations within test methods. > On June 15, 2016, 9:34 p.m., Dan Smith wrote: > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java, > > line 262 > > <https://reviews.apache.org/r/48738/diff/1/?file=1420216#file1420216line262> > > > > I think it's better to put assertions into awaitility, like this > > > > Awaitility...until(() -> assertEquals(1, r.size()) > > > > Although I guess in this case you are using >=, rather than ==. But why > > is that? Can't you make an assertion about exactly what size the region > > should be? Good point...Done. - anilkumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48738/#review137852 ----------------------------------------------------------- On June 15, 2016, 5:42 p.m., anilkumar gingade wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48738/ > ----------------------------------------------------------- > > (Updated June 15, 2016, 5:42 p.m.) > > > Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, > Jason Huynh, Jens Deppe, William Markito, nabarun nag, Dan Smith, and > xiaojian zhou. > > > Repository: geode > > > Description > ------- > > GEODE-1209 was proposed to add new attribute ignoreEvictionAndExpiration to > AsyncEventQueue...As mentioned in the ticket due to product issue a new > proposal was made to change the functionality, to only forward > expiration-destroy operation. > > The changes made here replaces ignoreEvictionAndExpiration attribute to > forwardExiprationDestroy. > > > Diffs > ----- > > > geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationEndToEndDUnitTest.java > 93cb31a > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueue.java > edf887b > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueFactory.java > 6294dfe > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/AsyncEventQueueFactoryImpl.java > 1ec3ba0 > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/AsyncEventQueueImpl.java > 5a0b370 > geode-core/src/main/java/com/gemstone/gemfire/cache/wan/GatewaySender.java > d559a1a > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java > 7e2a0af > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderAttributes.java > 163943f > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/AsyncEventQueueCreation.java > e55ec3f > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXml.java > 4ee3585 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlGenerator.java > 17076db > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlParser.java > 76ab0f9 > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommands.java > d84959f > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/AsyncEventQueueFunctionArgs.java > 2066628 > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/CreateAsyncEventQueueFunction.java > 32e8f83 > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/i18n/CliStrings.java > cc80de8 > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/QueueCommandsController.java > 9367612 > > geode-core/src/main/resources/META-INF/schemas/geode.apache.org/schema/cache/cache-1.0.xsd > 688ff1f > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java > 4c5944b > > geode-core/src/test/java/com/gemstone/gemfire/cache30/CacheXmlGeode10DUnitTest.java > be4c657 > > geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java > c1877fe > > geode-core/src/test/resources/com/gemstone/gemfire/management/internal/cli/commands/golden-help-offline.properties > 3c0d388 > > Diff: https://reviews.apache.org/r/48738/diff/ > > > Testing > ------- > > - Test for ignoreEvictionAndExpiration was changeg to test > forwardExpirationDestroy > - SharedConfigurationEndToEndDUnitTest > - CacheXmlGeode10DUnitTest > - QueueCommandsDUnitTest > > > Thanks, > > anilkumar gingade > >
