remove S3 specific parser and update implementation and tests

botocore/data/s3/2006-03-01/service-2.sdk-extras.json

@@ -1,10 +1,6 @@
 {
   "version": 1.0,
   "merge": {
-    "metadata":{
-      "protocol":"s3",
-      "protocols":["s3"]
-    },
     "shapes": {
       "Expires":{"type":"timestamp"}
     }

botocore/handlers.py

@@ -615,17 +615,22 @@ def document_s3_expires_shape(section, event_name, **kwargs):
         if not section.has_section('Expires'):
             return
         param_section = section.get_section('Expires')
+        # Add a deprecation notice for the "Expires" param
         doc_section = param_section.get_section('param-documentation')
+        doc_section.style.start_note()
         doc_section.write(
-            '*This member has been deprecated*. Please use ``ExpiresString`` instead.'
+            'This member has been deprecated. Please use ``ExpiresString`` instead.'
+        )
+        doc_section.style.end_note()
+        # Document the "ExpiresString" param
+        new_param_section = param_section.add_new_section('ExpiresString')
+        new_param_section.style.new_paragraph()
+        new_param_section.write('- **ExpiresString** *(string) --*')
+        new_param_section.style.indent()
+        new_param_section.style.new_paragraph()
+        new_param_section.write(
+            'The raw, unparsed value of the ``Expires`` field.'
         )
-        param_section.add_new_section('ExpiresString')
-        new_param = param_section.get_section('ExpiresString')
-        new_param.style.start_li()
-        new_param.write('**ExpiresString** *(string) --*')
-        new_param.style.end_li()
-        new_param.style.new_line()
-        new_param.write('\tThe raw, unparsed value of the ``Expires`` field.')
 
 
 def base64_encode_user_data(params, **kwargs):

botocore/parsers.py

@@ -536,6 +536,17 @@ def _handle_float(self, shape, text):
     def _handle_timestamp(self, shape, text):
         return self._timestamp_parser(text)
 
+    @_text_content
+    def _handle_expires_timestamp(self, shape, text):
+        try:
+            return self._handle_timestamp(shape, text)
+        except (ValueError, RuntimeError):
+            LOG.warning(
+                f'Failed to parse the "Expires" member as a timestamp: {text}. '
+                f'The unparsed value is available in the response under "ExpiresString".'
+            )
+            return None
+
     @_text_content
     def _handle_integer(self, shape, text):
         return int(text)
@@ -964,14 +975,19 @@ def _parse_non_payload_attrs(
                     member_shape, headers
                 )
             elif location == 'header':
-                self._parse_header(final_parsed, headers, member_shape, name)
-
-    def _parse_header(self, final_parsed, headers, member_shape, name):
-        header_name = member_shape.serialization.get('name', name)
-        if header_name in headers:
-            final_parsed[name] = self._parse_shape(
-                member_shape, headers[header_name]
-            )
+                header_name = member_shape.serialization.get('name', name)
+                if header_name in headers:
+                    if header_name == 'Expires':
+                        final_parsed[name] = self._handle_expires_timestamp(
+                            member_shape, headers[header_name]
+                        )
+                        if final_parsed[name] is None:
+                            del final_parsed[name]
+                        final_parsed['ExpiresString'] = headers[header_name]
+                    else:
+                        final_parsed[name] = self._parse_shape(
+                            member_shape, headers[header_name]
+                        )
 
     def _parse_header_map(self, shape, headers):
         # Note that headers are case insensitive, so we .lower()
@@ -1116,36 +1132,8 @@ def _handle_string(self, shape, text):
         return text
 
 
-class S3Parser(RestXMLParser):
-    def _parse_header(self, final_parsed, headers, member_shape, name):
-        header_name = member_shape.serialization.get('name', name)
-        if header_name in headers:
-            if header_name == 'Expires':
-                final_parsed['ExpiresString'] = headers[header_name]
-                final_parsed[name] = self._handle_expires_timestamp(
-                    member_shape, headers[header_name]
-                )
-                if final_parsed[name] is None:
-                    del final_parsed[name]
-            else:
-                super()._parse_header(
-                    final_parsed, headers, member_shape, name
-                )
-
-    def _handle_expires_timestamp(self, shape, timestamp):
-        try:
-            return self._handle_timestamp(shape, timestamp)
-        except (ValueError, RuntimeError):
-            LOG.warning(
-                f'Failed to parse the "Expires" member as a timestamp: {timestamp}. '
-                f'The unparsed value is available in the response under "ExpiresString".'
-            )
-            return None
-
-
 PROTOCOL_PARSERS = {
     'ec2': EC2QueryParser,
-    's3': S3Parser,
     'query': QueryParser,
     'json': JSONParser,
     'rest-json': RestJSONParser,

botocore/serialize.py

@@ -804,7 +804,6 @@ def _default_serialize(self, xmlnode, params, shape, name):
 
 SERIALIZERS = {
     'ec2': EC2Serializer,
-    's3': RestXMLSerializer,
     'query': QuerySerializer,
     'json': JSONSerializer,
     'rest-json': RestJSONSerializer,

tests/functional/test_s3.py

@@ -12,15 +12,13 @@
 # language governing permissions and limitations under the License.
 import base64
 import datetime
-import json
-import os
 import re
 
 import pytest
 from dateutil.tz import tzutc
 
 import botocore.session
-from botocore import BOTOCORE_ROOT, UNSIGNED
+from botocore import UNSIGNED
 from botocore.compat import get_md5, parse_qs, urlsplit
 from botocore.config import Config
 from botocore.exceptions import (
@@ -1343,7 +1341,7 @@ def test_500_error_with_non_xml_body(self):
             self.assertEqual(len(http_stubber.requests), 2)
 
 
-class TestS3Parser(BaseS3OperationTest):
+class TestS3ExpiresHeaderResponse(BaseS3OperationTest):
     def test_valid_expires_value_in_response(self):
         expires_value = "Thu, 01 Jan 1970 00:00:00 GMT"
         mock_headers = {'expires': expires_value}
@@ -2113,23 +2111,6 @@ def test_presign_uses_v2_for_us_east_1_with_us_east_1_regional(self):
         self.assert_is_v2_presigned_url(url)
 
 
-@pytest.mark.validates_models
-def test_s3_protocols_unchanged():
-    # Verify that the 'protocols' metadata key remains unchanged in the S3 model.
-    # If support for another protocol is added, we want to be alerted instead of
-    # silently overwriting the value in the service-2.sdk-extras.json file.
-    file_path = os.path.join(
-        BOTOCORE_ROOT, 'data/s3/2006-03-01/service-2.json'
-    )
-    with open(file_path) as file:
-        s3_model = json.load(file)
-    protocols = s3_model.get('metadata', {}).get('protocols', [])
-    expected_protocols = ['rest-xml']
-    assert (
-        protocols == expected_protocols
-    ), f"Expected protocols list: {expected_protocols}, but got: {protocols}"
-
-
 CHECKSUM_TEST_CASES = [
     ("put_bucket_tagging", {"Bucket": "foo", "Tagging": {"TagSet": []}}),
     (

tests/functional/test_service_headers.py

@@ -0,0 +1,50 @@
+# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+import pytest
+
+from botocore.session import get_session
+
+# This test verifies that S3 is the only service that uses the 'expires' header
+# in their response. If any other service is found to target this header in any
+# of their output shapes, the test will fail and alert us of the change.
+
+
+@pytest.mark.validates_models
+def test_only_s3_targets_expires_header():
+    session = get_session()
+    loader = session.get_component('data_loader')
+    services = loader.list_available_services('service-2')
+    services_that_target_expires_header = set()
+
+    for service in services:
+        service_model = session.get_service_model(service)
+        for operation in service_model.operation_names:
+            operation_model = service_model.operation_model(operation)
+            if output_shape := operation_model.output_shape:
+                if _shape_targets_expires_header(output_shape):
+                    services_that_target_expires_header.add(service)
+    assert services_that_target_expires_header == {'s3'}
+
+
+def _shape_targets_expires_header(shape):
+    for member_shape in shape.members.values():
+        location = member_shape.serialization.get('location')
+        location_name = member_shape.serialization.get('name')
+        if (
+            location
+            and location.lower() == 'header'
+            and location_name
+            and location_name.lower() == 'expires'
+        ):
+            return True
+    return False

tests/unit/test_handlers.py

@@ -1790,7 +1790,7 @@ def document_s3_expires_mocks():
     param_section = mock.Mock()
     doc_section = mock.Mock()
     new_param_line = mock.Mock()
-    new_param = mock.Mock()
+    new_param_section = mock.Mock()
     response_example_event = (
         'docs.response-example.s3.TestOperation.complete-section'
     )
@@ -1804,7 +1804,7 @@ def document_s3_expires_mocks():
         'param_section': param_section,
         'doc_section': doc_section,
         'new_param_line': new_param_line,
-        'new_param': new_param,
+        'new_param_section': new_param_section,
         'response_example_event': response_example_event,
         'response_params_event': response_params_event,
     }
@@ -1849,26 +1849,31 @@ def test_document_response_params_with_expires(document_s3_expires_mocks):
     mocks['section'].get_section.return_value = mocks['param_section']
     mocks['param_section'].get_section.side_effect = [
         mocks['doc_section'],
-        mocks['new_param'],
     ]
-    mocks['new_param'].style = mock.Mock()
+    mocks['param_section'].add_new_section.side_effect = [
+        mocks['new_param_section'],
+    ]
+    mocks['doc_section'].style = mock.Mock()
+    mocks['new_param_section'].style = mock.Mock()
     handlers.document_s3_expires_shape(
         mocks['section'], mocks['response_params_event']
     )
     mocks['param_section'].get_section.assert_any_call('param-documentation')
-    mocks['param_section'].get_section.assert_any_call('ExpiresString')
+    mocks['doc_section'].style.start_note.assert_called_once()
     mocks['doc_section'].write.assert_called_once_with(
-        '*This member has been deprecated*. Please use ``ExpiresString`` instead.'
+        'This member has been deprecated. Please use ``ExpiresString`` instead.'
     )
+    mocks['doc_section'].style.end_note.assert_called_once()
     mocks['param_section'].add_new_section.assert_called_once_with(
         'ExpiresString'
     )
-    mocks['new_param'].style.start_li.assert_called_once()
-    mocks['new_param'].write.assert_any_call('**ExpiresString** *(string) --*')
-    mocks['new_param'].style.end_li.assert_called_once()
-    mocks['new_param'].style.new_line.assert_called_once()
-    mocks['new_param'].write.assert_any_call(
-        '\tThe raw, unparsed value of the ``Expires`` field.'
+    mocks['new_param_section'].style.new_paragraph.assert_any_call()
+    mocks['new_param_section'].write.assert_any_call(
+        '- **ExpiresString** *(string) --*'
+    )
+    mocks['new_param_section'].style.indent.assert_called_once()
+    mocks['new_param_section'].write.assert_any_call(
+        'The raw, unparsed value of the ``Expires`` field.'
     )
 
 

tests/unit/test_parsers.py

@@ -1648,11 +1648,6 @@ def test_can_handle_generic_error_message(parser, body):
     assert parsed['ResponseMetadata']['HTTPStatusCode'] == 503
 
 
-@pytest.fixture
-def s3_parser():
-    return parsers.S3Parser()
-
-
 @pytest.fixture
 def expires_output_shape():
     output_shape = model.StructureShape(
@@ -1681,10 +1676,10 @@ def expires_output_shape():
         ),
     ],
 )
-def test_s3_valid_expires_response_parsed(
-    s3_parser, expires_output_shape, expires, expected_expires
+def test_valid_expires_header_response_parsed(
+    expires_output_shape, expires, expected_expires
 ):
-    parser = s3_parser
+    parser = parsers.RestXMLParser()
     output_shape = expires_output_shape
     parsed = parser.parse(
         {
@@ -1715,10 +1710,8 @@ def test_s3_valid_expires_response_parsed(
         -33702800404003370280040400,
     ],
 )
-def test_s3_invalid_expires_response_parsed(
-    s3_parser, expires_output_shape, expires
-):
-    parser = s3_parser
+def test_invalid_expires_header_response_parsed(expires_output_shape, expires):
+    parser = parsers.RestXMLParser()
     output_shape = expires_output_shape
     parsed = parser.parse(
         {