Skip to content

Commit 60db3b2

Browse files
ricellisricellis
committed
perf: use normal feed for one-off changes
1 parent fa26103 commit 60db3b2

5 files changed

Lines changed: 79 additions & 52 deletions

File tree

‎modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesFollower.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ private synchronized Stream<ChangesResultItem> run(Mode mode) throws IllegalStat
257257
// Construct the spliterator using the batch size as the per request limit
258258
this.changesResultSpliterator.set(new ChangesResultSpliterator(
259259
this.client,
260-
ChangesOptionsHelper.cloneOptionsWithNewLimit(this.options, batchSize.get()),
260+
ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit(this.options, mode, batchSize.get()),
261261
mode,
262262
this.errorTolerance
263263
));

‎modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelper.java‎

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616
import java.time.Duration;
1717
import java.util.ArrayList;
1818
import java.util.List;
19+
import java.util.Optional;
1920
import java.util.concurrent.TimeUnit;
21+
import java.util.function.Consumer;
2022
import java.util.stream.Collectors;
23+
import com.ibm.cloud.cloudant.features.ChangesFollower.Mode;
2124
import com.ibm.cloud.cloudant.v1.model.PostChangesOptions;
25+
import com.ibm.cloud.cloudant.v1.model.PostChangesOptions.Builder;
2226

2327
class ChangesOptionsHelper {
2428

@@ -36,51 +40,42 @@ private ChangesOptionsHelper() {
3640
}
3741

3842
static PostChangesOptions cloneOptions(PostChangesOptions options) {
39-
return cloneOptions(options, options.since(), options.limit());
43+
return buildOptions(options, null);
4044
}
4145

42-
static PostChangesOptions cloneOptionsWithNewLimit(PostChangesOptions options, Long limit) {
43-
return cloneOptions(options, options.since(), limit);
46+
static PostChangesOptions cloneOptionsWithModeAndNewLimit(PostChangesOptions options, Mode mode, Long limit) {
47+
return buildOptions(options, b -> {
48+
switch (mode) {
49+
case FINITE:
50+
b.feed(PostChangesOptions.Feed.NORMAL);
51+
break;
52+
case LISTEN:
53+
b.feed(PostChangesOptions.Feed.LONGPOLL);
54+
b.timeout(LONGPOLL_TIMEOUT);
55+
break;
56+
}
57+
// Handle limit to avoid NPE during unboxing
58+
if (limit != null) {
59+
b.limit(limit);
60+
}
61+
});
4462
}
4563

4664
static PostChangesOptions cloneOptionsWithNewSince(PostChangesOptions options, String since) {
47-
return cloneOptions(options, since, options.limit());
65+
return buildOptions(options, b -> {
66+
b.since(since);
67+
});
4868
}
4969

50-
private static PostChangesOptions cloneOptions(PostChangesOptions options, String since, Long limit) {
51-
// Now merge and set defaults
52-
PostChangesOptions.Builder builder = new PostChangesOptions.Builder(options.db())
53-
.attEncodingInfo(options.attEncodingInfo())
54-
.attachments(options.attachments())
55-
.conflicts(options.conflicts())
56-
// no descending
57-
.docIds(options.docIds())
58-
.feed(PostChangesOptions.Feed.LONGPOLL)
59-
.fields(options.fields())
60-
.filter(options.filter())
61-
// no heartbeat
62-
.includeDocs(options.includeDocs())
63-
// no lastEventId
64-
// limit handled after
65-
.selector(options.selector())
66-
// seq interval handled after
67-
.since(since)
68-
.style(options.style())
69-
.timeout(LONGPOLL_TIMEOUT)
70-
.view(options.view());
71-
72-
// Handle options that might NPE during unboxing
73-
if (limit != null) {
74-
builder.limit(limit);
75-
}
76-
if (options.seqInterval() != null) {
77-
builder.seqInterval(options.seqInterval());
78-
}
79-
70+
private static PostChangesOptions buildOptions(PostChangesOptions originalOptions, Consumer<Builder> extraOpts) {
71+
Builder builder = originalOptions.newBuilder();
72+
Optional.ofNullable(extraOpts).ifPresent(
73+
builderConsumer -> builderConsumer.accept(builder)
74+
);
8075
return builder.build();
8176
}
8277

83-
private static String throwInvalidOptionsMessageWith(String suffix, List<String> invalidOptions) {
78+
private static String throwInvalidOptionsMessageWith(List<String> invalidOptions) {
8479

8580
String errorMsgFormat = (invalidOptions.size() > 1) ? multipleOptionErrorFormat : singleOptionErrorFormat;
8681
String errorMsg = String.format(errorMsgFormat,
@@ -112,7 +107,7 @@ static void validateOptions(PostChangesOptions options) {
112107
invalidOptions.add(String.format("filter=%s", options.filter()));
113108
}
114109
if (invalidOptions.size() > 0) {
115-
throwInvalidOptionsMessageWith(String.format(" when using %s.", ChangesFollower.class.getSimpleName()), invalidOptions);
110+
throwInvalidOptionsMessageWith(invalidOptions);
116111
}
117112
}
118113
}

‎modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesOptionsHelperTest.java‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@
1313

1414
package com.ibm.cloud.cloudant.features;
1515

16+
import com.ibm.cloud.cloudant.features.ChangesFollower.Mode;
1617
import com.ibm.cloud.cloudant.v1.model.PostChangesOptions;
18+
import java.util.ArrayList;
19+
import java.util.Arrays;
20+
import java.util.Iterator;
21+
import java.util.List;
1722
import org.testng.Assert;
1823
import org.testng.annotations.DataProvider;
1924
import org.testng.annotations.Test;
@@ -29,27 +34,42 @@ Object[][] getValidOptionsDataProvider() {
2934
return TestOptions.filteredValuesAsDataProvider(TestOptions::valid);
3035
}
3136

37+
@DataProvider(name = "validOptionsBothModes")
38+
Iterator<Object[]> getValidOptionsAndModesDataProvider() {
39+
List<Object[]> options = new ArrayList<>();
40+
for (Mode mode : Mode.values()) {
41+
for (Object[] opts : getValidOptionsDataProvider()) {
42+
// Add the valid opts
43+
List<Object> modeOpts= new ArrayList<>(Arrays.asList(opts));
44+
// Add the mode
45+
modeOpts.add(mode);
46+
options.add(modeOpts.toArray(new Object[modeOpts.size()]));
47+
}
48+
}
49+
return options.iterator();
50+
}
51+
3252
@DataProvider(name = "invalidOptions")
3353
Object[][] getInvalidOptionsDataProvider() {
3454
return TestOptions.filteredValuesAsDataProvider(o -> !o.valid());
3555
}
3656

37-
@Test(dataProvider = "validOptions")
38-
void testCloneOptions(String name, TestOptions opts) {
39-
PostChangesOptions expected = opts.getExpectedOptions();
57+
@Test(dataProvider = "validOptionsBothModes")
58+
void testCloneOptions(String name, TestOptions opts, Mode mode) {
59+
PostChangesOptions expected = opts.getExpectedOptions(mode);
4060
Assert.assertNotNull(expected);
4161
PostChangesOptions cloned = ChangesOptionsHelper.cloneOptions(expected);
4262
// Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486
4363
Assert.assertFalse(cloned == expected, "The clone should not be the original options object.");
4464
Assert.assertEquals(cloned, expected, "The clone should equal the original options object.");
4565
}
4666

47-
@Test(dataProvider = "validOptions")
48-
void testCloneOptionsWithNewLimit(String name, TestOptions opts) {
67+
@Test(dataProvider = "validOptionsBothModes")
68+
void testCloneOptionsWithNewLimit(String name, TestOptions opts, Mode mode) {
4969
Long newLimit = 50L;
5070
PostChangesOptions original = opts.getOptions();
51-
PostChangesOptions expected = opts.getExpectedOptionsBuilder().limit(newLimit).build();
52-
PostChangesOptions newLimitOpts = ChangesOptionsHelper.cloneOptionsWithNewLimit(original, newLimit);
71+
PostChangesOptions expected = opts.getExpectedOptionsBuilder(mode).limit(newLimit).build();
72+
PostChangesOptions newLimitOpts = ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit(original, mode, newLimit);
5373
// Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486
5474
Assert.assertFalse(newLimitOpts == original, "The clone should not be the original options object.");
5575
Assert.assertNotEquals(newLimitOpts, original, "The clone should not equal the original options object.");
@@ -61,7 +81,7 @@ void testCloneOptionsWithNewLimit(String name, TestOptions opts) {
6181
void testCloneOptionsWithNewSince(String name, TestOptions opts) {
6282
String newSince = "9876-alotofcharactersthatarenotreallyrandom";
6383
PostChangesOptions original = opts.getOptions();
64-
PostChangesOptions expected = opts.getExpectedOptionsBuilder().since(newSince).build();
84+
PostChangesOptions expected = opts.getExpectedOptionsBuilder(null).since(newSince).build();
6585
PostChangesOptions newSinceOpts = ChangesOptionsHelper.cloneOptionsWithNewSince(original, newSince);
6686
// Can't use assertNotSame in testng 7.4 because https://github.com/cbeust/testng/issues/2486
6787
Assert.assertFalse(newSinceOpts == original, "The clone should not be the original options object.");

‎modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/ChangesResultSpliteratorTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@
4040
public class ChangesResultSpliteratorTest {
4141

4242
// A default set of options to run with
43-
static final PostChangesOptions DEFAULT_OPTIONS = ChangesOptionsHelper.cloneOptionsWithNewLimit(
43+
static final PostChangesOptions DEFAULT_OPTIONS = ChangesOptionsHelper.cloneOptionsWithModeAndNewLimit(
4444
TestOptions.MINIMUM.getOptions(),
45+
ChangesFollower.Mode.LISTEN,
4546
ChangesFollower.BATCH_SIZE);
4647

4748
// A duration to use for testing error suppression

‎modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/TestOptions.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.function.Supplier;
2020
import java.util.stream.Stream;
2121
import com.ibm.cloud.cloudant.v1.model.PostChangesOptions;
22+
import com.ibm.cloud.cloudant.features.ChangesFollower.Mode;
2223
import com.ibm.cloud.cloudant.v1.model.GetDbUpdatesOptions.Feed;
2324

2425
/**
@@ -129,14 +130,24 @@ PostChangesOptions getOptions() {
129130
}
130131

131132
// Adds in the implementation expected changes
132-
PostChangesOptions.Builder getExpectedOptionsBuilder() {
133-
return this.getBuilder()
134-
.feed(Feed.LONGPOLL)
135-
.timeout(ChangesOptionsHelper.LONGPOLL_TIMEOUT);
133+
PostChangesOptions.Builder getExpectedOptionsBuilder(Mode mode) {
134+
PostChangesOptions.Builder b = this.getBuilder();
135+
if (mode != null) {
136+
switch (mode) {
137+
case FINITE:
138+
b.feed(Feed.NORMAL);
139+
break;
140+
case LISTEN:
141+
b.feed(Feed.LONGPOLL);
142+
b.timeout(ChangesOptionsHelper.LONGPOLL_TIMEOUT);
143+
break;
144+
}
145+
}
146+
return b;
136147
}
137148

138149
// Adds in the implementation expected changes
139-
PostChangesOptions getExpectedOptions() {
140-
return this.getExpectedOptionsBuilder().build();
150+
PostChangesOptions getExpectedOptions(Mode mode) {
151+
return this.getExpectedOptionsBuilder(mode).build();
141152
}
142153
}

0 commit comments

Comments
 (0)