diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesFollower.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesFollower.java index 3a925b792..ec4fe1601 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesFollower.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesFollower.java @@ -257,7 +257,7 @@ private synchronized Stream run(Mode mode) throws IllegalStat // Construct the spliterator using the batch size as the per request limit this.changesResultSpliterator.set(new ChangesResultSpliterator( this.client, - ChangesOptionsHelper.cloneOptionsWithNewLimit(this.options, batchSize.get()), + ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit(this.options, mode, batchSize.get()), mode, this.errorTolerance )); diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelper.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelper.java index 21cacfbf4..a2dc2e2f3 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelper.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelper.java @@ -16,9 +16,13 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; import java.util.stream.Collectors; +import com.ibm.cloud.cloudant.features.ChangesFollower.Mode; import com.ibm.cloud.cloudant.v1.model.PostChangesOptions; +import com.ibm.cloud.cloudant.v1.model.PostChangesOptions.Builder; class ChangesOptionsHelper { @@ -36,51 +40,42 @@ private ChangesOptionsHelper() { } static PostChangesOptions cloneOptions(PostChangesOptions options) { - return cloneOptions(options, options.since(), options.limit()); + return buildOptions(options, null); } - static PostChangesOptions cloneOptionsWithNewLimit(PostChangesOptions options, Long limit) { - return cloneOptions(options, options.since(), limit); + static PostChangesOptions cloneOptionsWithModeAndNewLimit(PostChangesOptions options, Mode mode, Long limit) { + return buildOptions(options, b -> { + switch (mode) { + case FINITE: + b.feed(PostChangesOptions.Feed.NORMAL); + break; + case LISTEN: + b.feed(PostChangesOptions.Feed.LONGPOLL); + b.timeout(LONGPOLL_TIMEOUT); + break; + } + // Handle limit to avoid NPE during unboxing + if (limit != null) { + b.limit(limit); + } + }); } static PostChangesOptions cloneOptionsWithNewSince(PostChangesOptions options, String since) { - return cloneOptions(options, since, options.limit()); + return buildOptions(options, b -> { + b.since(since); + }); } - private static PostChangesOptions cloneOptions(PostChangesOptions options, String since, Long limit) { - // Now merge and set defaults - PostChangesOptions.Builder builder = new PostChangesOptions.Builder(options.db()) - .attEncodingInfo(options.attEncodingInfo()) - .attachments(options.attachments()) - .conflicts(options.conflicts()) - // no descending - .docIds(options.docIds()) - .feed(PostChangesOptions.Feed.LONGPOLL) - .fields(options.fields()) - .filter(options.filter()) - // no heartbeat - .includeDocs(options.includeDocs()) - // no lastEventId - // limit handled after - .selector(options.selector()) - // seq interval handled after - .since(since) - .style(options.style()) - .timeout(LONGPOLL_TIMEOUT) - .view(options.view()); - - // Handle options that might NPE during unboxing - if (limit != null) { - builder.limit(limit); - } - if (options.seqInterval() != null) { - builder.seqInterval(options.seqInterval()); - } - + private static PostChangesOptions buildOptions(PostChangesOptions originalOptions, Consumer extraOpts) { + Builder builder = originalOptions.newBuilder(); + Optional.ofNullable(extraOpts).ifPresent( + builderConsumer -> builderConsumer.accept(builder) + ); return builder.build(); } - private static String throwInvalidOptionsMessageWith(String suffix, List invalidOptions) { + private static String throwInvalidOptionsMessageWith(List invalidOptions) { String errorMsgFormat = (invalidOptions.size() > 1) ? multipleOptionErrorFormat : singleOptionErrorFormat; String errorMsg = String.format(errorMsgFormat, @@ -112,7 +107,7 @@ static void validateOptions(PostChangesOptions options) { invalidOptions.add(String.format("filter=%s", options.filter())); } if (invalidOptions.size() > 0) { - throwInvalidOptionsMessageWith(String.format(" when using %s.", ChangesFollower.class.getSimpleName()), invalidOptions); + throwInvalidOptionsMessageWith(invalidOptions); } } } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelperTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelperTest.java index 7457db3e0..065bdd9b9 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelperTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelperTest.java @@ -13,7 +13,12 @@ package com.ibm.cloud.cloudant.features; +import com.ibm.cloud.cloudant.features.ChangesFollower.Mode; import com.ibm.cloud.cloudant.v1.model.PostChangesOptions; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -29,14 +34,29 @@ Object[][] getValidOptionsDataProvider() { return TestOptions.filteredValuesAsDataProvider(TestOptions::valid); } + @DataProvider(name = "validOptionsBothModes") + Iterator getValidOptionsAndModesDataProvider() { + List options = new ArrayList<>(); + for (Mode mode : Mode.values()) { + for (Object[] opts : getValidOptionsDataProvider()) { + // Add the valid opts + List modeOpts= new ArrayList<>(Arrays.asList(opts)); + // Add the mode + modeOpts.add(mode); + options.add(modeOpts.toArray(new Object[modeOpts.size()])); + } + } + return options.iterator(); + } + @DataProvider(name = "invalidOptions") Object[][] getInvalidOptionsDataProvider() { return TestOptions.filteredValuesAsDataProvider(o -> !o.valid()); } - @Test(dataProvider = "validOptions") - void testCloneOptions(String name, TestOptions opts) { - PostChangesOptions expected = opts.getExpectedOptions(); + @Test(dataProvider = "validOptionsBothModes") + void testCloneOptions(String name, TestOptions opts, Mode mode) { + PostChangesOptions expected = opts.getExpectedOptions(mode); Assert.assertNotNull(expected); PostChangesOptions cloned = ChangesOptionsHelper.cloneOptions(expected); // Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486 @@ -44,12 +64,12 @@ void testCloneOptions(String name, TestOptions opts) { Assert.assertEquals(cloned, expected, "The clone should equal the original options object."); } - @Test(dataProvider = "validOptions") - void testCloneOptionsWithNewLimit(String name, TestOptions opts) { + @Test(dataProvider = "validOptionsBothModes") + void testCloneOptionsWithNewLimit(String name, TestOptions opts, Mode mode) { Long newLimit = 50L; PostChangesOptions original = opts.getOptions(); - PostChangesOptions expected = opts.getExpectedOptionsBuilder().limit(newLimit).build(); - PostChangesOptions newLimitOpts = ChangesOptionsHelper.cloneOptionsWithNewLimit(original, newLimit); + PostChangesOptions expected = opts.getExpectedOptionsBuilder(mode).limit(newLimit).build(); + PostChangesOptions newLimitOpts = ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit(original, mode, newLimit); // Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486 Assert.assertFalse(newLimitOpts == original, "The clone should not be the original options object."); Assert.assertNotEquals(newLimitOpts, original, "The clone should not equal the original options object."); @@ -61,7 +81,7 @@ void testCloneOptionsWithNewLimit(String name, TestOptions opts) { void testCloneOptionsWithNewSince(String name, TestOptions opts) { String newSince = "9876-alotofcharactersthatarenotreallyrandom"; PostChangesOptions original = opts.getOptions(); - PostChangesOptions expected = opts.getExpectedOptionsBuilder().since(newSince).build(); + PostChangesOptions expected = opts.getExpectedOptionsBuilder(null).since(newSince).build(); PostChangesOptions newSinceOpts = ChangesOptionsHelper.cloneOptionsWithNewSince(original, newSince); // Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486 Assert.assertFalse(newSinceOpts == original, "The clone should not be the original options object."); diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesResultSpliteratorTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesResultSpliteratorTest.java index 31b616ee6..2d1f7a24a 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesResultSpliteratorTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesResultSpliteratorTest.java @@ -40,8 +40,9 @@ public class ChangesResultSpliteratorTest { // A default set of options to run with - static final PostChangesOptions DEFAULT_OPTIONS = ChangesOptionsHelper.cloneOptionsWithNewLimit( + static final PostChangesOptions DEFAULT_OPTIONS = ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit( TestOptions.MINIMUM.getOptions(), + ChangesFollower.Mode.LISTEN, ChangesFollower.BATCH_SIZE); // A duration to use for testing error suppression diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/TestOptions.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/TestOptions.java index 3e0ddf67d..26803c270 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/TestOptions.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/TestOptions.java @@ -19,6 +19,7 @@ import java.util.function.Supplier; import java.util.stream.Stream; import com.ibm.cloud.cloudant.v1.model.PostChangesOptions; +import com.ibm.cloud.cloudant.features.ChangesFollower.Mode; import com.ibm.cloud.cloudant.v1.model.GetDbUpdatesOptions.Feed; /** @@ -129,14 +130,24 @@ PostChangesOptions getOptions() { } // Adds in the implementation expected changes - PostChangesOptions.Builder getExpectedOptionsBuilder() { - return this.getBuilder() - .feed(Feed.LONGPOLL) - .timeout(ChangesOptionsHelper.LONGPOLL_TIMEOUT); + PostChangesOptions.Builder getExpectedOptionsBuilder(Mode mode) { + PostChangesOptions.Builder b = this.getBuilder(); + if (mode != null) { + switch (mode) { + case FINITE: + b.feed(Feed.NORMAL); + break; + case LISTEN: + b.feed(Feed.LONGPOLL); + b.timeout(ChangesOptionsHelper.LONGPOLL_TIMEOUT); + break; + } + } + return b; } // Adds in the implementation expected changes - PostChangesOptions getExpectedOptions() { - return this.getExpectedOptionsBuilder().build(); + PostChangesOptions getExpectedOptions(Mode mode) { + return this.getExpectedOptionsBuilder(mode).build(); } }