diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPageIterator.java index 02871a546..376ba456d 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPageIterator.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.AllDocsResult; import com.ibm.cloud.cloudant.v1.model.PostAllDocsOptions; @@ -27,16 +26,6 @@ final class AllDocsPageIterator extends AllDocsBasePageIterator optionsToBuilderFunction() { - return PostAllDocsOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction> nextRequestFunction() { return Cloudant::postAllDocs; @@ -47,9 +36,4 @@ BiFunction nextKeySetter() { return Builder::startKey; } - @Override - Function limitGetter() { - return PostAllDocsOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPartitionPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPartitionPageIterator.java index f48fa8534..cd38455df 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPartitionPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/AllDocsPartitionPageIterator.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.AllDocsResult; import com.ibm.cloud.cloudant.v1.model.PostPartitionAllDocsOptions; @@ -27,16 +26,6 @@ final class AllDocsPartitionPageIterator extends AllDocsBasePageIterator optionsToBuilderFunction() { - return PostPartitionAllDocsOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction> nextRequestFunction() { return Cloudant::postPartitionAllDocs; @@ -48,9 +37,4 @@ BiFunction nextKeySetter() { return Builder::startKey; } - @Override - Function limitGetter() { - return PostPartitionAllDocsOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/BasePageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/BasePageIterator.java index fe3930d53..05d413719 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/BasePageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/BasePageIterator.java @@ -17,7 +17,6 @@ import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; -import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; import java.util.function.Function; @@ -36,9 +35,10 @@ abstract class BasePageIterator implements Iterator> { this.client = client; this.optsHandler = optsHandler; // Set the page size from the supplied options limit - this.pageSize = getPageSizeFromOptionsLimit(options); + this.pageSize = this.optsHandler.getPageSizeFromOptionsLimit(options); // Set the first page options - buildAndSetOptions(optsHandler.builderFromOptions(options)); + buildAndSetOptions( + this.optsHandler.applyLimit(this.optsHandler.builderFromOptions(options), this.pageSize)); } @Override @@ -56,37 +56,31 @@ public List next() { } List nextRequest() { - ServiceCall request = nextRequestFunction().apply(this.client, this.nextPageOptionsRef.get()); + ServiceCall request = + nextRequestFunction().apply(this.client, this.nextPageOptionsRef.get()); R result = request.execute().getResult(); List items = itemsGetter().apply(result); if (items.size() < this.pageSize) { this.hasNext = false; } else { + O options = this.nextPageOptionsRef.get(); B optionsBuilder = optsHandler.builderFromOptions(this.nextPageOptionsRef.get()); setNextPageOptions(optionsBuilder, result); + // Remove any options valid on the user request, but invalid during paging + optionsBuilder = this.optsHandler.removeOptsForSubsequentPage(options, optionsBuilder); buildAndSetOptions(optionsBuilder); } return items; } - private void buildAndSetOptions(B optionsBuilder) { + protected void buildAndSetOptions(B optionsBuilder) { this.nextPageOptionsRef.set(optsHandler.optionsFromBuilder(optionsBuilder)); } - abstract Function optionsToBuilderFunction(); - - abstract Function builderToOptionsFunction(); - abstract Function> itemsGetter(); abstract void setNextPageOptions(B builder, R result); abstract BiFunction> nextRequestFunction(); - abstract Function limitGetter(); - - Long getPageSizeFromOptionsLimit(O opts) { - return Optional.ofNullable(limitGetter().apply(opts)).orElse(200L); - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/DesignDocsPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/DesignDocsPageIterator.java index c593d5a41..5ce7bc496 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/DesignDocsPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/DesignDocsPageIterator.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.AllDocsResult; import com.ibm.cloud.cloudant.v1.model.PostDesignDocsOptions; @@ -32,24 +31,9 @@ BiFunction nextKeySetter() { return Builder::key; } - @Override - Function optionsToBuilderFunction() { - return PostDesignDocsOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction> nextRequestFunction() { return Cloudant::postDesignDocs; } - @Override - Function limitGetter() { - return PostDesignDocsOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPager.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPager.java index d05a462e4..d8ea14b85 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPager.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPager.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.FindResult; import com.ibm.cloud.cloudant.v1.model.PostFindOptions; @@ -27,16 +26,6 @@ final class FindPager extends FindBasePageIterator optionsToBuilderFunction() { - return PostFindOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction bookmarkSetter() { return Builder::bookmark; @@ -47,9 +36,4 @@ BiFunction> nextRequestFuncti return Cloudant::postFind; } - @Override - Function limitGetter() { - return PostFindOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPartitionPager.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPartitionPager.java index ca2467306..d34342d8b 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPartitionPager.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/FindPartitionPager.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.FindResult; import com.ibm.cloud.cloudant.v1.model.PostPartitionFindOptions; @@ -27,16 +26,6 @@ final class FindPartitionPager extends FindBasePageIterator optionsToBuilderFunction() { - return PostPartitionFindOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction bookmarkSetter() { return Builder::bookmark; @@ -47,9 +36,4 @@ BiFunction> nextRequ return Cloudant::postPartitionFind; } - @Override - Function limitGetter() { - return PostPartitionFindOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIterator.java index 8f3e8be7d..503db47af 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIterator.java @@ -63,11 +63,6 @@ final void setNextPageOptions(B builder, R result) { nextKeyIdSetter().ifPresent(f -> f.apply(builder, nextId)); } - @Override - Long getPageSizeFromOptionsLimit(O opts) { - return super.getPageSizeFromOptionsLimit(opts) + 1; - } - abstract Optional checkBoundary(I penultimateItem, I lastItem); } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/OptionsHandler.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/OptionsHandler.java index 13523e438..96ae005e7 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/OptionsHandler.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/OptionsHandler.java @@ -15,10 +15,11 @@ import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.BiFunction; import java.util.function.Function; -import java.util.function.Supplier; import com.ibm.cloud.cloudant.v1.model.PostAllDocsOptions; import com.ibm.cloud.cloudant.v1.model.PostDesignDocsOptions; import com.ibm.cloud.cloudant.v1.model.PostFindOptions; @@ -56,10 +57,15 @@ abstract class OptionsHandler { private final Function builderToOptions; private final Function optionsToBuilder; + private final Function limitGetter; + private final BiFunction limitSetter; - private OptionsHandler(Function builderToOptions, Function optionsToBuilder) { + private OptionsHandler(Function builderToOptions, Function optionsToBuilder, + Function limitGetter, BiFunction limitSetter) { this.builderToOptions = builderToOptions; this.optionsToBuilder = optionsToBuilder; + this.limitGetter = limitGetter; + this.limitSetter = limitSetter; } B builderFromOptions(O options) { @@ -70,53 +76,68 @@ O optionsFromBuilder(B builder) { return this.builderToOptions.apply(builder); } - abstract void validate(O options); + protected void validate(O options) { + validateLimit(options); + } - O clone(O options) { + private O copy(O options) { return this.optionsFromBuilder(this.builderFromOptions(options)); } + B applyLimit(B builder, Long newLimit) { + return this.limitSetter.apply(builder, newLimit); + } + + B removeOptsForSubsequentPage(O options, B builder) { + // Default is a no-op + return builder; + } + + Long getPageSizeFromOptionsLimit(O opts) { + return Optional.ofNullable(this.limitGetter.apply(opts)).orElse(MAX_LIMIT); + } + static final PostAllDocsOptions duplicate(PostAllDocsOptions opts) { - return POST_ALL_DOCS.clone(opts); + return POST_ALL_DOCS.copy(opts); } static final PostDesignDocsOptions duplicate(PostDesignDocsOptions opts) { - return POST_DESIGN_DOCS.clone(opts); + return POST_DESIGN_DOCS.copy(opts); } static final PostFindOptions duplicate(PostFindOptions opts) { - return POST_FIND.clone(opts); + return POST_FIND.copy(opts); } static final PostPartitionAllDocsOptions duplicate(PostPartitionAllDocsOptions opts) { - return POST_PARTITION_ALL_DOCS.clone(opts); + return POST_PARTITION_ALL_DOCS.copy(opts); } static final PostPartitionFindOptions duplicate(PostPartitionFindOptions opts) { - return POST_PARTITION_FIND.clone(opts); + return POST_PARTITION_FIND.copy(opts); } static final PostPartitionSearchOptions duplicate(PostPartitionSearchOptions opts) { - return POST_PARTITION_SEARCH.clone(opts); + return POST_PARTITION_SEARCH.copy(opts); } static final PostPartitionViewOptions duplicate(PostPartitionViewOptions opts) { - return POST_PARTITION_VIEW.clone(opts); + return POST_PARTITION_VIEW.copy(opts); } static final PostSearchOptions duplicate(PostSearchOptions opts) { - return POST_SEARCH.clone(opts); + return POST_SEARCH.copy(opts); } static final PostViewOptions duplicate(PostViewOptions opts) { - return POST_VIEW.clone(opts); + return POST_VIEW.copy(opts); } - private static void validateLimit(Supplier limitSupplier) { + protected void validateLimit(O opts) { // If limit is set check it is within range // Else it is unset and we will set the valid default value later - if (optionIsPresent(limitSupplier)) { - Long limit = limitSupplier.get(); + if (optionIsPresent(opts, this.limitGetter)) { + Long limit = this.limitGetter.apply(opts); if (limit > MAX_LIMIT) { throw new IllegalArgumentException(String.format( "The provided limit %d exceeds the maximum page size value of %d.", limit, MAX_LIMIT)); @@ -129,153 +150,291 @@ private static void validateLimit(Supplier limitSupplier) { } } - private static boolean optionIsPresent(final Supplier optionSupplier) { - return Optional.ofNullable(optionSupplier.get()).isPresent(); + protected Optional getOptionalOption(final O opts, final Function optionGetter) { + return Optional.ofNullable(optionGetter.apply(opts)); + } + + protected boolean optionIsPresent(final O opts, final Function optionGetter) { + return getOptionalOption(opts, optionGetter).isPresent(); } - private static void validateOptionsAbsent(final Map> options) { - for (Map.Entry> option : options.entrySet()) { - if (optionIsPresent(option.getValue())) { + protected void validateOptionsAbsent(final O opts, + final Map> optionGetters, final String messageReason) { + for (Map.Entry> optionGetter : optionGetters.entrySet()) { + if (optionIsPresent(opts, optionGetter.getValue())) { throw new IllegalArgumentException( - String.format("The option '%s' is invalid when using pagination.", option.getKey())); + String.format("The option '%s' is invalid %s", optionGetter.getKey(), messageReason)); } } } + protected void validateOptionsAbsent(final O options, + final Map> optionGetters) { + validateOptionsAbsent(options, optionGetters, "when using pagination."); + } + + private abstract static class KeyOptionsHandler extends OptionsHandler { + + protected final Function keyGetter; + private final Function> keysGetter; + protected final Function skipGetter; + private final String keyErrorMsg = + keyErrorMessage(new StringBuilder("when using pagination. ")); + + protected KeyOptionsHandler(Function builderToOptions, Function optionsToBuilder, + Function limitGetter, BiFunction limitSetter, Function keyGetter, + Function> keysGetter, Function skipGetter) { + super(builderToOptions, optionsToBuilder, limitGetter, limitSetter); + this.keyGetter = keyGetter; + this.keysGetter = keysGetter; + this.skipGetter = skipGetter; + } + + protected String keyErrorMessage(StringBuilder baseMessage) { + // Default for all docs style, override for views + return baseMessage.append("No need to paginate as 'key' returns a single result for an ID.") + .toString(); + } + + @Override + Long getPageSizeFromOptionsLimit(O opts) { + return super.getPageSizeFromOptionsLimit(opts) + 1; + } + + @Override + protected void validate(O options) { + validateOptionsAbsent(options, Collections.singletonMap("keys", this.keysGetter)); + validateOptionsAbsent(options, Collections.singletonMap("key", this.keyGetter), + this.keyErrorMsg); + super.validate(options); + } + + @Override + B removeOptsForSubsequentPage(O options, B builder) { + // Unset the skip option if necessary + if (optionIsPresent(options, this.skipGetter)) { + builder = this.builderFromOptions(replaceOpts(builder)); + } + return super.removeOptsForSubsequentPage(options, builder); + } + + protected abstract O replaceOpts(B builder); + } + + private abstract static class ViewsOptionsHandler extends KeyOptionsHandler { + + protected ViewsOptionsHandler(Function builderToOptions, Function optionsToBuilder, + Function limitGetter, BiFunction limitSetter, + Function keyGetter, Function> keysGetter, + Function skipGetter) { + super(builderToOptions, optionsToBuilder, limitGetter, limitSetter, keyGetter, keysGetter, + skipGetter); + } + + @Override + protected String keyErrorMessage(StringBuilder baseMessage) { + return baseMessage.append("Use startKey and endKey instead.").toString(); + } + + } + + private abstract static class BookmarkOptionsHandler extends OptionsHandler { + + BookmarkOptionsHandler(Function builderToOptions, Function optionsToBuilder, + Function limitGetter, BiFunction limitSetter) { + super(builderToOptions, optionsToBuilder, limitGetter, limitSetter); + } + + } + private static final class AllDocsOptionsHandler - extends OptionsHandler { + extends KeyOptionsHandler { private AllDocsOptionsHandler() { - super(PostAllDocsOptions.Builder::build, PostAllDocsOptions::newBuilder); + super(PostAllDocsOptions.Builder::build, PostAllDocsOptions::newBuilder, + PostAllDocsOptions::limit, PostAllDocsOptions.Builder::limit, PostAllDocsOptions::key, + PostAllDocsOptions::keys, PostAllDocsOptions::skip); } @Override - void validate(PostAllDocsOptions options) { - validateLimit(options::limit); - validateOptionsAbsent(Collections.singletonMap("keys", options::keys)); + protected PostAllDocsOptions replaceOpts(PostAllDocsOptions.Builder builder) { + return new PostAllDocsOptions(builder) { + PostAllDocsOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts(); } } private static final class DesignDocsOptionsHandler - extends OptionsHandler { + extends KeyOptionsHandler { private DesignDocsOptionsHandler() { - super(PostDesignDocsOptions.Builder::build, PostDesignDocsOptions::newBuilder); + super(PostDesignDocsOptions.Builder::build, PostDesignDocsOptions::newBuilder, + PostDesignDocsOptions::limit, PostDesignDocsOptions.Builder::limit, + PostDesignDocsOptions::key, PostDesignDocsOptions::keys, PostDesignDocsOptions::skip); } @Override - void validate(PostDesignDocsOptions options) { - validateLimit(options::limit); - validateOptionsAbsent(Collections.singletonMap("keys", options::keys)); + protected PostDesignDocsOptions replaceOpts(PostDesignDocsOptions.Builder builder) { + return new PostDesignDocsOptions(builder) { + PostDesignDocsOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts(); } } private static final class FindOptionsHandler - extends OptionsHandler { + extends BookmarkOptionsHandler { + + private final Function skipGetter = PostFindOptions::skip; private FindOptionsHandler() { - super(PostFindOptions.Builder::build, PostFindOptions::newBuilder); + super(PostFindOptions.Builder::build, PostFindOptions::newBuilder, PostFindOptions::limit, + PostFindOptions.Builder::limit); } @Override - void validate(PostFindOptions options) { - validateLimit(options::limit); + PostFindOptions.Builder removeOptsForSubsequentPage(PostFindOptions options, + PostFindOptions.Builder builder) { + if (optionIsPresent(options, this.skipGetter)) { + return new PostFindOptions(builder) { + PostFindOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts().newBuilder(); + } + return builder; } } - private static final class PartitionAllDocsOptionsHandler - extends OptionsHandler { + private static final class PartitionAllDocsOptionsHandler extends + KeyOptionsHandler { private PartitionAllDocsOptionsHandler() { - super(PostPartitionAllDocsOptions.Builder::build, PostPartitionAllDocsOptions::newBuilder); + super(PostPartitionAllDocsOptions.Builder::build, PostPartitionAllDocsOptions::newBuilder, + PostPartitionAllDocsOptions::limit, PostPartitionAllDocsOptions.Builder::limit, + PostPartitionAllDocsOptions::key, PostPartitionAllDocsOptions::keys, + PostPartitionAllDocsOptions::skip); } @Override - void validate(PostPartitionAllDocsOptions options) { - validateLimit(options::limit); - validateOptionsAbsent(Collections.singletonMap("keys", options::keys)); + protected PostPartitionAllDocsOptions replaceOpts(PostPartitionAllDocsOptions.Builder builder) { + return new PostPartitionAllDocsOptions(builder) { + PostPartitionAllDocsOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts(); } } private static final class PartitionFindOptionsHandler - extends OptionsHandler { + extends BookmarkOptionsHandler { + + private final Function skipGetter = + PostPartitionFindOptions::skip; private PartitionFindOptionsHandler() { - super(PostPartitionFindOptions.Builder::build, PostPartitionFindOptions::newBuilder); + super(PostPartitionFindOptions.Builder::build, PostPartitionFindOptions::newBuilder, + PostPartitionFindOptions::limit, PostPartitionFindOptions.Builder::limit); } @Override - void validate(PostPartitionFindOptions options) { - validateLimit(options::limit); + PostPartitionFindOptions.Builder removeOptsForSubsequentPage(PostPartitionFindOptions options, + PostPartitionFindOptions.Builder builder) { + if (optionIsPresent(options, this.skipGetter)) { + return new PostPartitionFindOptions(builder) { + PostPartitionFindOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts().newBuilder(); + } + return builder; } } - private static final class PartitionSearchOptionsHandler - extends OptionsHandler { + private static final class PartitionSearchOptionsHandler extends + BookmarkOptionsHandler { private PartitionSearchOptionsHandler() { - super(PostPartitionSearchOptions.Builder::build, PostPartitionSearchOptions::newBuilder); - } - - @Override - void validate(PostPartitionSearchOptions options) { - validateLimit(options::limit); + super(PostPartitionSearchOptions.Builder::build, PostPartitionSearchOptions::newBuilder, + PostPartitionSearchOptions::limit, PostPartitionSearchOptions.Builder::limit); } } private static final class PartitionViewOptionsHandler - extends OptionsHandler { + extends ViewsOptionsHandler { private PartitionViewOptionsHandler() { - super(PostPartitionViewOptions.Builder::build, PostPartitionViewOptions::newBuilder); + super(PostPartitionViewOptions.Builder::build, PostPartitionViewOptions::newBuilder, + PostPartitionViewOptions::limit, PostPartitionViewOptions.Builder::limit, + PostPartitionViewOptions::key, PostPartitionViewOptions::keys, + PostPartitionViewOptions::skip); } @Override - void validate(PostPartitionViewOptions options) { - validateLimit(options::limit); - validateOptionsAbsent(Collections.singletonMap("keys", options::keys)); + protected PostPartitionViewOptions replaceOpts(PostPartitionViewOptions.Builder builder) { + return new PostPartitionViewOptions(builder) { + PostPartitionViewOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts(); } } private static final class SearchOptionsHandler - extends OptionsHandler { + extends BookmarkOptionsHandler { private SearchOptionsHandler() { - super(PostSearchOptions.Builder::build, PostSearchOptions::newBuilder); + super(PostSearchOptions.Builder::build, PostSearchOptions::newBuilder, + PostSearchOptions::limit, PostSearchOptions.Builder::limit); } @Override - void validate(PostSearchOptions options) { - validateLimit(options::limit); - Map> invalidOptions = new HashMap<>(5); - invalidOptions.put("counts", options::counts); - invalidOptions.put("groupField", options::groupField); - invalidOptions.put("groupLimit", options::groupLimit); - invalidOptions.put("groupSort", options::groupSort); - invalidOptions.put("ranges", options::ranges); - validateOptionsAbsent(invalidOptions); + protected void validate(PostSearchOptions options) { + Map> invalidOptions = new HashMap<>(5, 1); + invalidOptions.put("counts", PostSearchOptions::counts); + invalidOptions.put("groupField", PostSearchOptions::groupField); + invalidOptions.put("groupLimit", PostSearchOptions::groupLimit); + invalidOptions.put("groupSort", PostSearchOptions::groupSort); + invalidOptions.put("ranges", PostSearchOptions::ranges); + validateOptionsAbsent(options, invalidOptions); + super.validate(options); } } private static final class ViewOptionsHandler - extends OptionsHandler { + extends ViewsOptionsHandler { private ViewOptionsHandler() { - super(PostViewOptions.Builder::build, PostViewOptions::newBuilder); + super(PostViewOptions.Builder::build, PostViewOptions::newBuilder, PostViewOptions::limit, + PostViewOptions.Builder::limit, PostViewOptions::key, PostViewOptions::keys, + PostViewOptions::skip); } @Override - void validate(PostViewOptions options) { - validateLimit(options::limit); - validateOptionsAbsent(Collections.singletonMap("keys", options::keys)); + protected PostViewOptions replaceOpts(PostViewOptions.Builder builder) { + return new PostViewOptions(builder) { + PostViewOptions unsetOpts() { + this.skip = null; + return this; + } + }.unsetOpts(); } } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPageIterator.java index c7ac7a858..ad41d31fd 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPageIterator.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.PostSearchOptions; import com.ibm.cloud.cloudant.v1.model.PostSearchOptions.Builder; @@ -27,16 +26,6 @@ final class SearchPageIterator extends SearchBasePageIterator optionsToBuilderFunction() { - return PostSearchOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction bookmarkSetter() { return Builder::bookmark; @@ -47,9 +36,4 @@ BiFunction> nextRequestFu return Cloudant::postSearch; } - @Override - Function limitGetter() { - return PostSearchOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPartitionPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPartitionPageIterator.java index 6b599d734..4c77ccfce 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPartitionPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/SearchPartitionPageIterator.java @@ -14,7 +14,6 @@ package com.ibm.cloud.cloudant.features.pagination; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.PostPartitionSearchOptions; import com.ibm.cloud.cloudant.v1.model.PostPartitionSearchOptions.Builder; @@ -27,16 +26,6 @@ final class SearchPartitionPageIterator extends SearchBasePageIterator optionsToBuilderFunction() { - return PostPartitionSearchOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction bookmarkSetter() { return Builder::bookmark; @@ -47,9 +36,4 @@ BiFunction> next return Cloudant::postPartitionSearch; } - @Override - Function limitGetter() { - return PostPartitionSearchOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewBasePageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewBasePageIterator.java index ab15baa2d..be44480de 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewBasePageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewBasePageIterator.java @@ -43,15 +43,13 @@ final Function nextKeyIdGetter() { @Override final Optional checkBoundary(ViewResultRow penultimateItem, ViewResultRow lastItem) { - String pId = penultimateItem.getId(); - String lId = lastItem.getId(); + Optional pId = Optional.ofNullable(penultimateItem.getId()); + Optional lId = Optional.ofNullable(lastItem.getId()); if (pId.equals(lId)) { // ID's are the same, check the keys - Object pKey = penultimateItem.getKey(); - Object lKey = lastItem.getKey(); - // Check reference equality first (e.g. null) - // Then check values - if (pKey == lKey || pKey != null && pKey.equals(lKey)) { + Optional pKey = Optional.ofNullable(penultimateItem.getKey()); + Optional lKey = Optional.ofNullable(lastItem.getKey()); + if (pKey.equals(lKey)) { // Identical keys, set an error message return Optional.of( String.format( diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPageIterator.java index 2bb58f78e..284942165 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPageIterator.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.PostViewOptions; import com.ibm.cloud.cloudant.v1.model.PostViewOptions.Builder; @@ -28,16 +27,6 @@ final class ViewPageIterator extends ViewBasePageIterator optionsToBuilderFunction() { - return PostViewOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction> nextRequestFunction() { return Cloudant::postView; @@ -53,9 +42,4 @@ Optional> nextKeyIdSetter() { return Optional.of(Builder::startKeyDocId); } - @Override - Function limitGetter() { - return PostViewOptions::limit; - } - } diff --git a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPartitionPageIterator.java b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPartitionPageIterator.java index 3663aa957..c434a6687 100644 --- a/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPartitionPageIterator.java +++ b/modules/cloudant/src/main/java/com/ibm/cloud/cloudant/features/pagination/ViewPartitionPageIterator.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.function.BiFunction; -import java.util.function.Function; import com.ibm.cloud.cloudant.v1.Cloudant; import com.ibm.cloud.cloudant.v1.model.PostPartitionViewOptions; import com.ibm.cloud.cloudant.v1.model.PostPartitionViewOptions.Builder; @@ -28,16 +27,6 @@ final class ViewPartitionPageIterator extends ViewBasePageIterator optionsToBuilderFunction() { - return PostPartitionViewOptions::newBuilder; - } - - @Override - Function builderToOptionsFunction() { - return Builder::build; - } - @Override BiFunction> nextRequestFunction() { return Cloudant::postPartitionView; @@ -53,9 +42,4 @@ Optional> nextKeyIdSetter() { return Optional.of(Builder::startKeyDocId); } - @Override - Function limitGetter() { - return PostPartitionViewOptions::limit; - } - } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BasePageIteratorTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BasePageIteratorTest.java index d8bb10fff..fa501385f 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BasePageIteratorTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BasePageIteratorTest.java @@ -29,12 +29,12 @@ import com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.PageSupplier; import com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.TestResult; import com.ibm.cloud.cloudant.v1.Cloudant; -import com.ibm.cloud.cloudant.v1.model.PostViewOptions; -import com.ibm.cloud.cloudant.v1.model.PostViewOptions.Builder; +import com.ibm.cloud.cloudant.v1.model.PostFindOptions; +import com.ibm.cloud.cloudant.v1.model.PostFindOptions.Builder; import com.ibm.cloud.sdk.core.http.ServiceCall; -import static com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.getDefaultTestOptions; -import static com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.getRequiredTestOptionsBuilder; +import static com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.getDefaultTestFindOptions; +import static com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.getRequiredTestFindOptionsBuilder; import static com.ibm.cloud.cloudant.features.pagination.PaginationTestHelpers.newPageSupplierFromList; public class BasePageIteratorTest { @@ -45,10 +45,10 @@ public class BasePageIteratorTest { * This test sub-class of BasePager implicitly tests that various abstract methods are correctly * called by non-abstract methods in the BasePager. */ - private static class TestPager extends BasePageIterator { + private static class TestPager extends BasePageIterator { - protected TestPager(Cloudant client, PostViewOptions options) { - super(client, options, OptionsHandler.POST_VIEW); + protected TestPager(Cloudant client, PostFindOptions options) { + super(client, options, OptionsHandler.POST_FIND); } Cloudant getClient() { @@ -59,15 +59,6 @@ Cloudant getClient() { * Implicitly tests that limit gets applied per page, otherwise the default would be used and * page counts would be wrong. */ - @Override - protected Function optionsToBuilderFunction() { - return PostViewOptions::newBuilder; - } - - @Override - protected Function builderToOptionsFunction() { - return Builder::build; - } @Override protected Function> itemsGetter() { @@ -75,7 +66,7 @@ protected Function> itemsGetter() { } /** - * These tests don't actually use the options, but we set a startKey so we can validate calls to + * These tests don't actually use the options, but we set a bookmark so we can validate calls to * setNextPageOptions. */ @Override @@ -85,7 +76,7 @@ protected void setNextPageOptions(Builder builder, TestResult result) { throw new IllegalStateException("Test failure: tried to setNextPageOptions on empty page."); } else { Integer i = rows.get(rows.size() - 1); - builder.startKey(i); + builder.bookmark(String.valueOf(i)); } } @@ -94,24 +85,19 @@ protected void setNextPageOptions(Builder builder, TestResult result) { * work. */ @Override - protected BiFunction> nextRequestFunction() { + protected BiFunction> nextRequestFunction() { return (c, o) -> { return ((MockPagerClient) c).testCall(); }; } - @Override - protected Function limitGetter() { - return PostViewOptions::limit; - } - } private static class ThrowingTestPager extends TestPager { private int callCounter = 0; - protected ThrowingTestPager(Cloudant client, PostViewOptions options) { + protected ThrowingTestPager(Cloudant client, PostFindOptions options) { super(client, options); } @@ -128,20 +114,20 @@ List nextRequest() { } - private Pagination newPagination(Cloudant client, - PostViewOptions options) { + private Pagination newPagination(Cloudant client, + PostFindOptions options) { return new Pagination<>(client, options, TestPager::new); } - private Pagination newThrowingPagination(Cloudant client, - PostViewOptions options) { + private Pagination newThrowingPagination(Cloudant client, + PostFindOptions options) { return new Pagination<>(client, options, ThrowingTestPager::new); } // test constructor @Test void testConstructor() { - TestPager pager = new TestPager(mockClient, getDefaultTestOptions(42)); + TestPager pager = new TestPager(mockClient, getDefaultTestFindOptions(42)); // Assert the client Assert.assertEquals(((TestPager) pager).getClient(), mockClient, "The client should be the supplied one."); @@ -150,7 +136,7 @@ void testConstructor() { // test page size default @Test void testDefaultPageSize() { - TestPager pager = new TestPager(mockClient, getRequiredTestOptionsBuilder().build()); + TestPager pager = new TestPager(mockClient, getRequiredTestFindOptionsBuilder().build()); // Assert the default page size Assert.assertEquals(pager.pageSize, 200, "The page size should be the default."); } @@ -158,7 +144,7 @@ void testDefaultPageSize() { // test page size limit @Test void testLimitPageSize() { - TestPager pager = new TestPager(mockClient, getDefaultTestOptions(42)); + TestPager pager = new TestPager(mockClient, getDefaultTestFindOptions(42)); // Assert the limit provided as page size Assert.assertEquals(pager.pageSize, 42, "The page size should match the limit."); } @@ -166,7 +152,7 @@ void testLimitPageSize() { // test hasNext @Test void testHasNextIsTrueInitially() { - TestPager pager = new TestPager(mockClient, getDefaultTestOptions(42)); + TestPager pager = new TestPager(mockClient, getDefaultTestFindOptions(42)); Assert.assertEquals(pager.hasNext(), true, "hasNext() should initially return true."); } @@ -176,7 +162,7 @@ void testHasNextIsTrueForResultEqualToLimit() { MockPagerClient c = new MockPagerClient(() -> { return new MockInstruction(new TestResult(Collections.singletonList(1))); }); - TestPager pager = new TestPager(c, getDefaultTestOptions(1)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(1)); // Get the first page (size 1) pager.next(); // hasNext should return true because results size is the same as limit @@ -189,7 +175,7 @@ void testHasNextIsFalseForResultLessThanLimit() { MockPagerClient c = new MockPagerClient(() -> { return new MockInstruction(new TestResult(Collections.emptyList())); }); - TestPager pager = new TestPager(c, getDefaultTestOptions(1)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(1)); // Get the first page (size 0) pager.next(); // hasNext should return false because results size smaller than limit @@ -203,7 +189,7 @@ void testNextFirstPage() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); List actualPage = pager.next(); // Assert first page Assert.assertEquals(actualPage, pageSupplier.pages.get(0), @@ -216,7 +202,7 @@ void testNextTwoPages() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); Assert.assertEquals(pager.hasNext(), true, "hasNext() should return true."); List actualPage1 = pager.next(); // Assert pages @@ -235,7 +221,7 @@ void testNextUntilEmpty() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(3 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); List actualItems = new ArrayList<>(); int pageCount = 0; while (pager.hasNext()) { @@ -259,7 +245,7 @@ void testNextUntilSmaller() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(10, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); List actualItems = new ArrayList<>(); int pageCount = 0; while (pager.hasNext()) { @@ -283,7 +269,7 @@ void testNextException() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(pageSize - 1, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); List actualPage = pager.next(); // Assert first page Assert.assertEquals(actualPage, pageSupplier.pages.get(0), @@ -302,7 +288,7 @@ void testPagesAreImmutable() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); List actualPage = pager.next(); // Assert immutable Assert.assertThrows(UnsupportedOperationException.class, () -> { @@ -317,20 +303,20 @@ void testSetNextPageOptions() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(5 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new TestPager(c, getDefaultTestOptions(pageSize)); - Assert.assertNull(pager.nextPageOptionsRef.get().startKey(), - "startKey should initially be null."); - // Since we use a page size of 1, each next page options key, is the same as the element from + TestPager pager = new TestPager(c, getDefaultTestFindOptions(pageSize)); + Assert.assertNull(pager.nextPageOptionsRef.get().bookmark(), + "bookamrk should initially be null."); + // Since we use a page size of 1, each next page options bookmark, is the same as the element from // the page int page = 0; while (pager.hasNext()) { pager.next(); if (pager.hasNext()) { - Assert.assertEquals(pager.nextPageOptionsRef.get().startKey(), page, - "the startKey should increment per page."); + Assert.assertEquals(pager.nextPageOptionsRef.get().bookmark(), String.valueOf(page), + "the bookmark should increment per page."); } else { // Options don't change for last page - Assert.assertEquals(pager.nextPageOptionsRef.get().startKey(), page - 1, + Assert.assertEquals(pager.nextPageOptionsRef.get().bookmark(), String.valueOf(page - 1), "The options should not be set for the final page."); } page++; @@ -343,20 +329,20 @@ void testNextResumesAfterError() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - TestPager pager = new ThrowingTestPager(c, getDefaultTestOptions(pageSize)); + TestPager pager = new ThrowingTestPager(c, getDefaultTestFindOptions(pageSize)); List actualPage1 = pager.next(); // Assert pages Assert.assertEquals(actualPage1, pageSupplier.pages.get(0), "The actual page should match the expected page."); - // The startKey should point to row 2 (the last row we saw, note this is not doing n+1 paging) - Assert.assertEquals(pager.nextPageOptionsRef.get().startKey(), 2, - "The startKey should be 2 for the second page."); + // The bookmark should point to row 2 (the last row we saw, note this is not doing n+1 paging) + Assert.assertEquals(pager.nextPageOptionsRef.get().bookmark(), "2", + "The bookmark should be 2 for the second page."); Assert.assertThrows(RuntimeException.class, () -> pager.next()); // Assert hasNext Assert.assertEquals(pager.hasNext(), true, "hasNext() should return true."); - // The startKey should still point to the second page - Assert.assertEquals(pager.nextPageOptionsRef.get().startKey(), 2, - "The startKey should be 2 for the second page."); + // The bookmark should still point to the second page + Assert.assertEquals(pager.nextPageOptionsRef.get().bookmark(), "2", + "The bookmark should be 2 for the second page."); List actualPage2 = pager.next(); Assert.assertEquals(actualPage2, pageSupplier.pages.get(1), "The actual page should match the expected page."); @@ -369,7 +355,7 @@ void testAsPagerGetNextFirstPage() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = newPagination(c, getDefaultTestOptions(pageSize)).pager(); + Pager pager = newPagination(c, getDefaultTestFindOptions(pageSize)).pager(); List actualPage = pager.getNext(); // Assert first page Assert.assertEquals(actualPage, pageSupplier.pages.get(0), @@ -383,7 +369,7 @@ void testAsPagerGetNextUntilConsumed() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = newPagination(c, getDefaultTestOptions(pageSize)).pager(); + Pager pager = newPagination(c, getDefaultTestFindOptions(pageSize)).pager(); List actualItems = new ArrayList<>(); Iterator> expectedPages = pageSupplier.pages.iterator(); int pageCount = 0; @@ -412,7 +398,7 @@ void testAsPagerGetAll() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(71, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = newPagination(c, getDefaultTestOptions(pageSize)).pager(); + Pager pager = newPagination(c, getDefaultTestFindOptions(pageSize)).pager(); List actualItems = pager.getAll(); Assert.assertEquals(actualItems, pageSupplier.allItems, "The results should match all the pages."); @@ -432,7 +418,7 @@ void testAsPagerGetNextResumesAfterError() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = newThrowingPagination(c, getDefaultTestOptions(pageSize)).pager(); + Pager pager = newThrowingPagination(c, getDefaultTestFindOptions(pageSize)).pager(); List actualPage1 = pager.getNext(); // Assert pages Assert.assertEquals(actualPage1, pageSupplier.pages.get(0), @@ -457,8 +443,8 @@ void testAsPagerGetAllRestartsAfterError() { Collections.emptyList())); // final empty page MockPagerClient c = new MockPagerClient(pageSupplier); final AtomicInteger constructedOnce = new AtomicInteger(); - Pagination errorOnFirst = new Pagination(c, - getDefaultTestOptions(pageSize), (client, opts) -> { + Pagination errorOnFirst = new Pagination(c, + getDefaultTestFindOptions(pageSize), (client, opts) -> { // Note that Pager automatically makes an iterator for hasNext/getNext so that is call 0 // Call 1 is the first getAll (that will throw) // Call 2 should not throw @@ -484,7 +470,7 @@ void testAsPagerGetNextGetAllThrows() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = newPagination(c, getDefaultTestOptions(pageSize)).pager(); + Pager pager = newPagination(c, getDefaultTestFindOptions(pageSize)).pager(); // Assert first page Assert.assertEquals(pager.getNext(), pageSupplier.pages.get(0), "The actual page should match the expected page."); @@ -505,8 +491,8 @@ void testAsPagerGetAllGetNextThrows() { PageSupplier pageSupplier = PaginationTestHelpers.newBasePageSupplier(2 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); - Pager pager = new Pagination(c, - getDefaultTestOptions(pageSize), ThrowingTestPager::new).pager(); + Pager pager = new Pagination(c, + getDefaultTestFindOptions(pageSize), ThrowingTestPager::new).pager(); // Make sure we stop the getAll in a non-consumed state Assert.assertThrows(RuntimeException.class, () -> pager.getAll() @@ -525,7 +511,7 @@ void testPagesIterable() { PaginationTestHelpers.newBasePageSupplier(3 * pageSize - 1, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); Iterator> expectedPages = pageSupplier.pages.iterator(); - for (List page : newPagination(c, getDefaultTestOptions(pageSize)).pages()) { + for (List page : newPagination(c, getDefaultTestFindOptions(pageSize)).pages()) { Assert.assertEquals(page, expectedPages.next()); } } @@ -538,7 +524,7 @@ void testRowsIterable() { PaginationTestHelpers.newBasePageSupplier(3 * pageSize - 1, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); Iterator expectedRows = pageSupplier.allItems.iterator(); - for (Integer row : newPagination(c, getDefaultTestOptions(pageSize)).rows()) { + for (Integer row : newPagination(c, getDefaultTestFindOptions(pageSize)).rows()) { Assert.assertEquals(row, expectedRows.next()); } } @@ -551,7 +537,7 @@ void testPagesStream() { PaginationTestHelpers.newBasePageSupplier(4 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); Iterator> expectedPages = pageSupplier.pages.iterator(); - newPagination(c, getDefaultTestOptions(pageSize)).pages() + newPagination(c, getDefaultTestFindOptions(pageSize)).pages() .forEach(i -> Assert.assertEquals(i, expectedPages.next())); } @@ -563,7 +549,7 @@ void testRowsStream() { PaginationTestHelpers.newBasePageSupplier(4 * pageSize, pageSize); MockPagerClient c = new MockPagerClient(pageSupplier); Iterator expectedRows = pageSupplier.allItems.iterator(); - newPagination(c, getDefaultTestOptions(pageSize)).rows() + newPagination(c, getDefaultTestFindOptions(pageSize)).rows() .forEach(i -> Assert.assertEquals(i, expectedRows.next())); } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BookmarkPageIteratorTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BookmarkPageIteratorTest.java index 5de76065b..a04740b0f 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BookmarkPageIteratorTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/BookmarkPageIteratorTest.java @@ -38,7 +38,8 @@ public class BookmarkPageIteratorTest { * This test sub-class of BookmarkPager implicitly tests that various abstract methods are * correctly called. */ - class TestBookmarkPager extends BookmarkPageIterator { + class TestBookmarkPager + extends BookmarkPageIterator { protected TestBookmarkPager(Cloudant client, PostFindOptions options) { super(client, options, OptionsHandler.POST_FIND); @@ -59,21 +60,6 @@ protected BiFunction> nextReq }; } - @Override - protected Function limitGetter() { - return PostFindOptions::limit; - } - - @Override - protected Function optionsToBuilderFunction() { - return PostFindOptions::newBuilder; - } - - @Override - protected Function builderToOptionsFunction() { - return Builder::build; - } - @Override protected Function> itemsGetter() { return TestResult::getRows; @@ -202,4 +188,24 @@ void testGetAll() { Assert.assertEquals(actualItems, pageSupplier.allItems, "The results should match all the pages."); } + + @Test + void testSkipRemovedForSubsequentPages() { + int pageSize = 3; + long expectedSkip = 17; + PageSupplier pageSupplier = newBasePageSupplier(pageSize * 3, pageSize); + MockPagerClient c = new MockPagerClient(pageSupplier); + PostFindOptions opts = + getRequiredTestFindOptionsBuilder().limit(pageSize).skip(expectedSkip).build(); + TestBookmarkPager pager = new TestBookmarkPager(c, opts); + // Assert skip set for first request + Assert.assertEquals(pager.nextPageOptionsRef.get().skip(), expectedSkip, + "The skip should equal the user provided skip."); + pager.next(); + // Assert skip not set for next page + Assert.assertNull(pager.nextPageOptionsRef.get().skip(), + "Skip should not be set for the next page."); + pager.next(); + } + } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIteratorTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIteratorTest.java index da3b6b5cc..2c81c0acf 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIteratorTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/KeyPageIteratorTest.java @@ -41,7 +41,8 @@ public class KeyPageIteratorTest { * This test sub-class of KeyPager implicitly tests that various abstract methods are correctly * called. */ - class TestKeyPager extends KeyPageIterator { + class TestKeyPager + extends KeyPageIterator { protected TestKeyPager(Cloudant client, PostViewOptions options) { super(client, options, OptionsHandler.POST_VIEW); @@ -62,11 +63,6 @@ protected BiFunction> nextReq }; } - @Override - protected Function limitGetter() { - return PostViewOptions::limit; - } - @Override protected BiFunction nextKeySetter() { return Builder::startKey; @@ -87,16 +83,6 @@ protected Function nextKeyIdGetter() { return String::valueOf; } - @Override - protected Function optionsToBuilderFunction() { - return PostViewOptions::newBuilder; - } - - @Override - protected Function builderToOptionsFunction() { - return Builder::build; - } - @Override protected Function> itemsGetter() { return TestResult::getRows; @@ -113,18 +99,25 @@ Optional checkBoundary(Integer penultimateItem, Integer lastItem) { @Test void testDefaultPageSize() { TestKeyPager pager = new TestKeyPager(mockClient, getRequiredTestOptionsBuilder().build()); + int expectedPageSize = Math.toIntExact(OptionsHandler.MAX_LIMIT + 1); // Assert the default limit as page size - Assert.assertEquals(pager.pageSize, 201, + Assert.assertEquals(pager.pageSize, expectedPageSize, "The page size should be one more than the default limit."); + Assert.assertEquals(pager.nextPageOptionsRef.get().limit(), expectedPageSize, + "The request limit should equal the page size."); } // Test page size limit (+1) @Test void testLimitPageSize() { - TestKeyPager pager = new TestKeyPager(mockClient, getDefaultTestOptions(42)); + int testPageSize = 42; + TestKeyPager pager = new TestKeyPager(mockClient, getDefaultTestOptions(testPageSize)); + int expectedPageSize = testPageSize + 1; // Assert the limit provided as page size Assert.assertEquals(pager.pageSize, 43, "The page size should be one more than the supplied limit."); + Assert.assertEquals(pager.nextPageOptionsRef.get().limit(), expectedPageSize, + "The request limit should equal the page size."); } // Test all items on page when no more pages @@ -297,4 +290,23 @@ Optional checkBoundary(Integer penultimateItem, Integer lastItem) { Assert.assertFalse(pager.hasNext(), "hasNext() should return false."); } + @Test + void testSkipRemovedForSubsequentPages() { + int pageSize = 3; + long expectedSkip = 17; + PageSupplier pageSupplier = newKeyPageSupplier(pageSize * 3, pageSize); + MockPagerClient c = new MockPagerClient(pageSupplier); + PostViewOptions opts = + getRequiredTestOptionsBuilder().limit(pageSize).skip(expectedSkip).build(); + TestKeyPager pager = new TestKeyPager(c, opts); + // Assert skip set for first request + Assert.assertEquals(pager.nextPageOptionsRef.get().skip(), expectedSkip, + "The skip should equal the user provided skip."); + pager.next(); + // Assert skip not set for next page + Assert.assertNull(pager.nextPageOptionsRef.get().skip(), + "Skip should not be set for the next page."); + pager.next(); + } + } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/OptionsValidationTest.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/OptionsValidationTest.java index 8484721f2..f2ebd1450 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/OptionsValidationTest.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/OptionsValidationTest.java @@ -115,4 +115,37 @@ public void testValidationExceptionForFacetedSearch(String optionName, Object op }); } + @Test(dataProvider = "allDocsOptions", dataProviderClass = PaginationTestHelpers.class) + public void testValidationExceptionForAllDocsKey(OptionsProvider provider) + throws Exception { + provider.setRequiredOpts(); + provider.set("key", "foo"); + Assert.assertThrows("There should be a validation exception", IllegalArgumentException.class, + () -> { + try { + provider.handler.validate(provider.build()); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().endsWith( + "The option 'key' is invalid when using pagination. No need to paginate as 'key' returns a single result for an ID.")); + throw e; + } + }); + } + + @Test(dataProvider = "viewOptions", dataProviderClass = PaginationTestHelpers.class) + public void testValidationExceptionForViewsKey(OptionsProvider provider) + throws Exception { + provider.setRequiredOpts(); + provider.set("key", new Object()); + Assert.assertThrows("There should be a validation exception", IllegalArgumentException.class, + () -> { + try { + provider.handler.validate(provider.build()); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().endsWith( + "The option 'key' is invalid when using pagination. Use startKey and endKey instead.")); + throw e; + } + }); + } } diff --git a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/PaginationTestHelpers.java b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/PaginationTestHelpers.java index fee039864..f1624ff88 100644 --- a/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/PaginationTestHelpers.java +++ b/modules/cloudant/src/test/java/com/ibm/cloud/cloudant/features/pagination/PaginationTestHelpers.java @@ -431,7 +431,7 @@ public Iterator viewLikeOptions() { @DataProvider(name = "viewOptions") public Iterator viewOptions() { - return getIteratorFor(viewLikeOptions); + return getIteratorFor(viewOptions); } static class TestDesignDocsResultRow extends DocsResultRow {