diff --git a/.changes/next-release/bugfix-dynamodb-60935.json b/.changes/next-release/bugfix-dynamodb-60935.json new file mode 100644 index 000000000000..e500bece5d64 --- /dev/null +++ b/.changes/next-release/bugfix-dynamodb-60935.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "``dynamodb``", + "description": "Correct Scan and Scanned Count values when resuming a scan or query with a ``--starting-token``" +} diff --git a/awscli/botocore/paginate.py b/awscli/botocore/paginate.py index 49cafe91adf2..24889c9600cd 100644 --- a/awscli/botocore/paginate.py +++ b/awscli/botocore/paginate.py @@ -411,7 +411,12 @@ def _handle_first_request( elif isinstance(sample, str): empty_value = '' elif isinstance(sample, (int, float)): - empty_value = 0 + # Even though we may be resuming from a truncated page, we + # still start from the actual numeric secondary result. For + # DynamoDB's Count/ScannedCount, this will still show how many + # items the server evaluated, even if the client is truncating + # due to a StartingToken. + empty_value = sample else: empty_value = None set_value_from_jmespath(parsed, token.expression, empty_value) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 5d6f44b63f00..353753c1441f 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -131,6 +131,10 @@ 'xray.GetTraceSummaries.TracesProcessedCount', 'xray.GetTraceSummaries.ApproximateTime', ] +KNOWN_PAGINATORS_WITH_INTEGER_OUTPUTS = ( + ('dynamodb', 'Query'), + ('dynamodb', 'Scan'), +) def _pagination_configs(): @@ -150,11 +154,12 @@ def _pagination_configs(): ) def test_lint_pagination_configs(operation_name, page_config, service_model): _validate_known_pagination_keys(page_config) - _valiate_result_key_exists(page_config) + _validate_result_key_exists(page_config) _validate_referenced_operation_exists(operation_name, service_model) _validate_operation_has_output(operation_name, service_model) _validate_input_keys_match(operation_name, page_config, service_model) _validate_output_keys_match(operation_name, page_config, service_model) + _validate_new_numeric_keys(operation_name, page_config, service_model) def _validate_known_pagination_keys(page_config): @@ -165,7 +170,7 @@ def _validate_known_pagination_keys(page_config): ) -def _valiate_result_key_exists(page_config): +def _validate_result_key_exists(page_config): if 'result_key' not in page_config: raise AssertionError( "Required key 'result_key' is missing " @@ -260,6 +265,30 @@ def _validate_output_keys_match(operation_name, page_config, service_model): ) +def _validate_new_numeric_keys(operation_name, page_config, service_model): + output_shape = service_model.operation_model(operation_name).output_shape + for key in _get_list_value(page_config, 'result_key'): + current_shape = output_shape + if '.' in key: # result_key is a JMESPath expression + for part in key.split('.'): + current_shape = current_shape.members[part] + elif key in output_shape.members: + current_shape = output_shape.members[key] + + if ( + getattr(current_shape, 'type_name', None) == 'integer' + and (service_model.service_name, operation_name) + not in KNOWN_PAGINATORS_WITH_INTEGER_OUTPUTS + ): + raise AssertionError( + f'There is a new operation {operation_name} for service ' + f'{service_model.service_name} that is configured to sum ' + 'integer outputs across pages. Verify that this behavior is ' + 'correct before allow-listing, since whether or not it is ' + 'appropriate to sum depends on the subject matter.' + ) + + def _looks_like_jmespath(expression): if all(ch in MEMBER_NAME_CHARS for ch in expression): return False diff --git a/tests/functional/ddb/test_select.py b/tests/functional/ddb/test_select.py index 670611f4b5cb..8ac60255e380 100644 --- a/tests/functional/ddb/test_select.py +++ b/tests/functional/ddb/test_select.py @@ -27,7 +27,7 @@ def assert_yaml_response_equal(self, response, expected): class TestSelect(BaseSelectTest): def setUp(self): - super(TestSelect, self).setUp() + super().setUp() self.parsed_response = { "Count": 1, "Items": [{"foo": {"S": "spam"}}], @@ -509,7 +509,7 @@ def test_select_parsing_error_rc(self): class TestSelectPagination(BaseSelectTest): def setUp(self): - super(TestSelectPagination, self).setUp() + super().setUp() self.parsed_responses = [ { "Count": 1, @@ -617,8 +617,8 @@ def test_starting_token(self): command, expected_params, expected_rc=0 ) expected_response = { - 'Count': 0, + 'Count': 1, 'Items': [{'foo': 2}], - 'ScannedCount': 0, + 'ScannedCount': 1, } self.assert_yaml_response_equal(stdout, expected_response) diff --git a/tests/unit/botocore/test_paginate.py b/tests/unit/botocore/test_paginate.py index d5450a787122..8c20681e618a 100644 --- a/tests/unit/botocore/test_paginate.py +++ b/tests/unit/botocore/test_paginate.py @@ -1089,7 +1089,7 @@ def test_resume_with_secondary_result_as_string(self): # they were in the original (first) response. self.assertEqual(complete, {"Users": ["User2"], "Groups": ""}) - def test_resume_with_secondary_result_as_integer(self): + def test_resume_with_secondary_result_does_not_reset_integer(self): self.method.return_value = {"Users": ["User1", "User2"], "Groups": 123} starting_token = encode_token( {"Marker": None, "boto_truncate_amount": 1} @@ -1098,9 +1098,9 @@ def test_resume_with_secondary_result_as_integer(self): PaginationConfig={'MaxItems': 1, 'StartingToken': starting_token} ) complete = pages.build_full_result() - # Note that the secondary keys ("Groups") becomes zero because - # they were in the original (first) response. - self.assertEqual(complete, {"Users": ["User2"], "Groups": 0}) + # Note that we don't reset numeric secondary keys, for DynamoDB's + # Count/ScannedCount since the server still evaluates those items + self.assertEqual(complete, {"Users": ["User2"], "Groups": 123}) class TestMultipleInputKeys(unittest.TestCase):