Skip to content

Commit cb25a1f

Browse files
zoewanggzoewangg
committed
Fix paginated response iterable hasNextPage to check if the output token is non null or non empty if it is a collection or map type. Fixes #677 
1 parent 791811c commit cb25a1f

File tree

10 files changed

+180
-38
lines changed

10 files changed

+180
-38
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-532f520.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "bugfix",
4+
"description": "Fix Response Fetcher hasNextPage to check if the output token is non null or non empty if it is a collection or map type. Related to [#677](https://github.com/aws/aws-sdk-java-v2/issues/677)"
5+
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/paginators/PaginatorsClassSpec.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@
3434
import software.amazon.awssdk.codegen.poet.ClassSpec;
3535
import software.amazon.awssdk.codegen.poet.PoetExtensions;
3636
import software.amazon.awssdk.codegen.poet.model.TypeProvider;
37-
import software.amazon.awssdk.core.util.SdkAutoConstructList;
38-
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
37+
import software.amazon.awssdk.core.util.PaginatorUtils;
3938

4039
public abstract class PaginatorsClassSpec implements ClassSpec {
4140

@@ -178,14 +177,11 @@ protected CodeBlock hasNextPageMethodBody() {
178177
}
179178
// If there is no more_results token, then output_token will be a single value
180179
return CodeBlock.builder()
181-
// Auto construct containers are treated as null so terminate
182-
// pagination for those cases as well.
183-
.add("return $1N.$2L != null && !$3T.class.isInstance($1N.$2L) && !$4T.class.isInstance($1N.$2L)",
184-
PREVIOUS_PAGE_METHOD_ARGUMENT,
185-
fluentGetterMethodsForOutputToken().get(0),
186-
SdkAutoConstructList.class,
187-
SdkAutoConstructMap.class)
188-
.build();
180+
.add("return $3T.isOutputTokenAvailable($1N.$2L)",
181+
PREVIOUS_PAGE_METHOD_ARGUMENT,
182+
fluentGetterMethodsForOutputToken().get(0),
183+
PaginatorUtils.class)
184+
.build();
189185

190186
}
191187

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/paginators/PaginatedOperationWithResultKeyIterable.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
import software.amazon.awssdk.core.pagination.sync.PaginatedResponsesIterator;
99
import software.amazon.awssdk.core.pagination.sync.SdkIterable;
1010
import software.amazon.awssdk.core.pagination.sync.SyncPageFetcher;
11-
import software.amazon.awssdk.core.util.SdkAutoConstructList;
12-
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
11+
import software.amazon.awssdk.core.util.PaginatorUtils;
1312
import software.amazon.awssdk.services.jsonprotocoltests.JsonProtocolTestsClient;
1413
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithResultKeyRequest;
1514
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithResultKeyResponse;
@@ -132,8 +131,7 @@ private class PaginatedOperationWithResultKeyResponseFetcher implements
132131
SyncPageFetcher<PaginatedOperationWithResultKeyResponse> {
133132
@Override
134133
public boolean hasNextPage(PaginatedOperationWithResultKeyResponse previousPage) {
135-
return previousPage.nextToken() != null && !SdkAutoConstructList.class.isInstance(previousPage.nextToken())
136-
&& !SdkAutoConstructMap.class.isInstance(previousPage.nextToken());
134+
return PaginatorUtils.isOutputTokenAvailable(previousPage.nextToken());
137135
}
138136

139137
@Override

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/paginators/PaginatedOperationWithResultKeyPublisher.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@
1111
import software.amazon.awssdk.core.pagination.async.EmptySubscription;
1212
import software.amazon.awssdk.core.pagination.async.PaginatedItemsPublisher;
1313
import software.amazon.awssdk.core.pagination.async.ResponsesSubscription;
14-
import software.amazon.awssdk.core.util.SdkAutoConstructList;
15-
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
14+
import software.amazon.awssdk.core.util.PaginatorUtils;
1615
import software.amazon.awssdk.services.jsonprotocoltests.JsonProtocolTestsAsyncClient;
1716
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithResultKeyRequest;
1817
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithResultKeyResponse;
@@ -138,8 +137,7 @@ private class PaginatedOperationWithResultKeyResponseFetcher implements
138137
AsyncPageFetcher<PaginatedOperationWithResultKeyResponse> {
139138
@Override
140139
public boolean hasNextPage(final PaginatedOperationWithResultKeyResponse previousPage) {
141-
return previousPage.nextToken() != null && !SdkAutoConstructList.class.isInstance(previousPage.nextToken())
142-
&& !SdkAutoConstructMap.class.isInstance(previousPage.nextToken());
140+
return PaginatorUtils.isOutputTokenAvailable(previousPage.nextToken());
143141
}
144142

145143
@Override

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/paginators/PaginatedOperationWithoutResultKeyIterable.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
import software.amazon.awssdk.core.pagination.sync.PaginatedResponsesIterator;
77
import software.amazon.awssdk.core.pagination.sync.SdkIterable;
88
import software.amazon.awssdk.core.pagination.sync.SyncPageFetcher;
9-
import software.amazon.awssdk.core.util.SdkAutoConstructList;
10-
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
9+
import software.amazon.awssdk.core.util.PaginatorUtils;
1110
import software.amazon.awssdk.services.jsonprotocoltests.JsonProtocolTestsClient;
1211
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithoutResultKeyRequest;
1312
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithoutResultKeyResponse;
@@ -110,8 +109,7 @@ private class PaginatedOperationWithoutResultKeyResponseFetcher implements
110109
SyncPageFetcher<PaginatedOperationWithoutResultKeyResponse> {
111110
@Override
112111
public boolean hasNextPage(PaginatedOperationWithoutResultKeyResponse previousPage) {
113-
return previousPage.nextToken() != null && !SdkAutoConstructList.class.isInstance(previousPage.nextToken())
114-
&& !SdkAutoConstructMap.class.isInstance(previousPage.nextToken());
112+
return PaginatorUtils.isOutputTokenAvailable(previousPage.nextToken());
115113
}
116114

117115
@Override

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/paginators/PaginatedOperationWithoutResultKeyPublisher.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
import software.amazon.awssdk.core.pagination.async.AsyncPageFetcher;
88
import software.amazon.awssdk.core.pagination.async.EmptySubscription;
99
import software.amazon.awssdk.core.pagination.async.ResponsesSubscription;
10-
import software.amazon.awssdk.core.util.SdkAutoConstructList;
11-
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
10+
import software.amazon.awssdk.core.util.PaginatorUtils;
1211
import software.amazon.awssdk.services.jsonprotocoltests.JsonProtocolTestsAsyncClient;
1312
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithoutResultKeyRequest;
1413
import software.amazon.awssdk.services.jsonprotocoltests.model.PaginatedOperationWithoutResultKeyResponse;
@@ -117,8 +116,7 @@ private class PaginatedOperationWithoutResultKeyResponseFetcher implements
117116
AsyncPageFetcher<PaginatedOperationWithoutResultKeyResponse> {
118117
@Override
119118
public boolean hasNextPage(final PaginatedOperationWithoutResultKeyResponse previousPage) {
120-
return previousPage.nextToken() != null && !SdkAutoConstructList.class.isInstance(previousPage.nextToken())
121-
&& !SdkAutoConstructMap.class.isInstance(previousPage.nextToken());
119+
return PaginatorUtils.isOutputTokenAvailable(previousPage.nextToken());
122120
}
123121

124122
@Override

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/util/PaginatorUtils.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright 2010-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.core.util;
17+
18+
import java.util.Collection;
19+
import java.util.Map;
20+
import software.amazon.awssdk.annotations.SdkProtectedApi;
21+
22+
@SdkProtectedApi
23+
public final class PaginatorUtils {
24+
25+
private PaginatorUtils() {
26+
}
27+
28+
29+
/**
30+
* Checks if the output token is available.
31+
*
32+
* @param outputToken the output token to check
33+
* @param <T> the type of the output token
34+
* @return true if the output token is non-null or non-empty if the output token is a map or Collection type
35+
*/
36+
public static <T> boolean isOutputTokenAvailable(T outputToken) {
37+
if (outputToken == null) {
38+
return false;
39+
}
40+
41+
if (outputToken instanceof Map) {
42+
return !((Map) outputToken).isEmpty();
43+
}
44+
45+
if (outputToken instanceof Collection) {
46+
return !((Collection) outputToken).isEmpty();
47+
}
48+
49+
return true;
50+
}
51+
}

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/util/PaginatorUtilsTest.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright 2010-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.core.util;
17+
18+
import static org.junit.Assert.assertFalse;
19+
import static org.junit.Assert.assertTrue;
20+
21+
import java.util.ArrayList;
22+
import java.util.Arrays;
23+
import java.util.HashMap;
24+
import org.junit.Test;
25+
26+
public class PaginatorUtilsTest {
27+
28+
@Test
29+
public void nullOutputToken_shouldReturnFalse() {
30+
assertFalse(PaginatorUtils.isOutputTokenAvailable(null));
31+
}
32+
33+
@Test
34+
public void nonNullString_shouldReturnTrue() {
35+
assertTrue(PaginatorUtils.isOutputTokenAvailable("next"));
36+
}
37+
38+
@Test
39+
public void nonNullInteger_shouldReturnTrue() {
40+
assertTrue(PaginatorUtils.isOutputTokenAvailable(12));
41+
}
42+
43+
@Test
44+
public void emptyCollection_shouldReturnFalse() {
45+
assertFalse(PaginatorUtils.isOutputTokenAvailable(new ArrayList<>()));
46+
}
47+
48+
@Test
49+
public void nonEmptyCollection_shouldReturnTrue() {
50+
assertTrue(PaginatorUtils.isOutputTokenAvailable(Arrays.asList("foo", "bar")));
51+
}
52+
53+
@Test
54+
public void emptyMap_shouldReturnFalse() {
55+
assertFalse(PaginatorUtils.isOutputTokenAvailable(new HashMap<>()));
56+
}
57+
58+
@Test
59+
public void nonEmptyMap_shouldReturnTrue() {
60+
HashMap<String, String> outputTokens = new HashMap<>();
61+
outputTokens.put("foo", "bar");
62+
assertTrue(PaginatorUtils.isOutputTokenAvailable(outputTokens));
63+
}
64+
65+
@Test
66+
public void sdkAutoConstructList_shouldReturnFalse() {
67+
assertFalse(PaginatorUtils.isOutputTokenAvailable(DefaultSdkAutoConstructList.getInstance()));
68+
}
69+
70+
@Test
71+
public void sdkAutoConstructMap_shouldReturnFalse() {
72+
assertFalse(PaginatorUtils.isOutputTokenAvailable(DefaultSdkAutoConstructMap.getInstance()));
73+
}
74+
}

‎services/dynamodb/src/it/java/software/amazon/awssdk/services/dynamodb/ScanPaginatorIntegrationTest.java renamed to ‎services/dynamodb/src/it/java/software/amazon/awssdk/services/dynamodb/PaginatorIntegrationTest.java

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@
1515

1616
package software.amazon.awssdk.services.dynamodb;
1717

18+
import static org.assertj.core.api.Assertions.assertThat;
1819
import static org.junit.Assert.assertEquals;
1920

21+
import java.util.ArrayList;
2022
import java.util.HashMap;
23+
import java.util.Iterator;
24+
import java.util.List;
2125
import java.util.Map;
2226
import java.util.Random;
2327
import java.util.stream.Stream;
@@ -27,10 +31,12 @@
2731
import software.amazon.awssdk.core.pagination.sync.SdkIterable;
2832
import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition;
2933
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
34+
import software.amazon.awssdk.services.dynamodb.model.BatchGetItemResponse;
3035
import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest;
3136
import software.amazon.awssdk.services.dynamodb.model.DeleteTableRequest;
3237
import software.amazon.awssdk.services.dynamodb.model.KeySchemaElement;
3338
import software.amazon.awssdk.services.dynamodb.model.KeyType;
39+
import software.amazon.awssdk.services.dynamodb.model.KeysAndAttributes;
3440
import software.amazon.awssdk.services.dynamodb.model.ProvisionedThroughput;
3541
import software.amazon.awssdk.services.dynamodb.model.PutItemRequest;
3642
import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType;
@@ -41,7 +47,7 @@
4147
import utils.test.util.DynamoDBTestBase;
4248
import utils.test.util.TableUtils;
4349

44-
public class ScanPaginatorIntegrationTest extends DynamoDBTestBase {
50+
public class PaginatorIntegrationTest extends DynamoDBTestBase {
4551

4652
private static final String TABLE_NAME = "java-sdk-scan-paginator-test" + System.currentTimeMillis();
4753
private static final String HASH_KEY_NAME = BasicTempTable.HASH_KEY_NAME;
@@ -76,6 +82,29 @@ public static void cleanUpFixture() {
7682
dynamo.deleteTable(DeleteTableRequest.builder().tableName(TABLE_NAME).build());
7783
}
7884

85+
@Test
86+
public void batchGetItem_allProcessed_shouldNotHaveNextPage() {
87+
Map<String, KeysAndAttributes> attributes = new HashMap<>();
88+
Map<String, AttributeValue> keys;
89+
KeysAndAttributes keysAndAttributes;
90+
List<Map<String, AttributeValue>> keysList = new ArrayList<>();
91+
92+
for (int i = 0; i < ITEM_COUNT; i++) {
93+
keys = new HashMap<>();
94+
keys.put(HASH_KEY_NAME, AttributeValue.builder().n(String.valueOf(i)).build());
95+
keysList.add(keys);
96+
}
97+
98+
keysAndAttributes = KeysAndAttributes.builder().keys(keysList).build();
99+
100+
attributes.put(TABLE_NAME, keysAndAttributes);
101+
102+
Iterator<BatchGetItemResponse> iterator =
103+
dynamo.batchGetItemPaginator(b -> b.requestItems(attributes)).iterator();
104+
assertThat(iterator.next().unprocessedKeys().isEmpty()).isTrue();
105+
assertThat(iterator.hasNext()).isFalse();
106+
}
107+
79108
@Test
80109
public void test_MultipleIteration_On_Responses_Iterable() {
81110
ScanRequest request = ScanRequest.builder().tableName(TABLE_NAME).limit(2).build();
@@ -126,16 +155,16 @@ public void test_MultipleIteration_On_Responses_Stream() {
126155

127156
// Iterate once
128157
assertEquals(ITEM_COUNT, scanResponses.stream()
129-
.mapToInt(response -> response.count())
130-
.sum());
158+
.mapToInt(response -> response.count())
159+
.sum());
131160

132161
// Iterate second time
133162
assertEquals(ITEM_COUNT, scanResponses.stream()
134-
.mapToInt(response -> response.count())
135-
.sum());
163+
.mapToInt(response -> response.count())
164+
.sum());
136165

137166
// Number of pages
138-
assertEquals(Math.ceil((double) ITEM_COUNT/results_per_page), scanResponses.stream().count(), 0);
167+
assertEquals(Math.ceil((double) ITEM_COUNT / results_per_page), scanResponses.stream().count(), 0);
139168
}
140169

141170
@Test
@@ -151,7 +180,7 @@ public void test_MultipleIteration_On_PaginatedMember_Stream() {
151180
assertEquals(ITEM_COUNT, items.stream().distinct().count());
152181
}
153182

154-
@Test (expected = IllegalStateException.class)
183+
@Test(expected = IllegalStateException.class)
155184
public void iteration_On_SameStream_ThrowsError() {
156185
int results_per_page = 2;
157186
ScanRequest request = ScanRequest.builder().tableName(TABLE_NAME).limit(results_per_page).build();

‎utils/src/test/java/software/amazon/awssdk/utils/CollectionUtilsTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,12 @@
1515

1616
package software.amazon.awssdk.utils;
1717

18-
import static org.hamcrest.MatcherAssert.assertThat;
19-
import static org.hamcrest.Matchers.isEmptyString;
20-
import static org.junit.Assert.assertEquals;
2118
import static org.junit.Assert.assertFalse;
2219
import static org.junit.Assert.assertTrue;
2320

24-
import java.util.ArrayList;
2521
import java.util.Arrays;
2622
import java.util.Collection;
2723
import java.util.Collections;
28-
import java.util.List;
2924
import org.junit.Test;
3025

3126
public class CollectionUtilsTest {

0 commit comments

Comments
 (0)