From 0da2ba901514447089f8a290f0ddba4f18cf53f8 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Fri, 28 Jan 2022 11:07:02 -0600 Subject: [PATCH] Set json/x-ndjson serializer to compatibility mode mimetype too --- elasticsearch/_async/client/__init__.py | 15 +- elasticsearch/_async/client/_base.py | 23 ++- elasticsearch/_sync/client/__init__.py | 15 +- elasticsearch/_sync/client/_base.py | 23 ++- elasticsearch/serializer.py | 28 +--- .../test_async/test_transport.py | 14 +- .../test_client/test_deprecated_options.py | 2 + .../test_client/test_options.py | 26 +-- .../test_client/test_serializers.py | 149 ++++++++++++++++++ test_elasticsearch/test_serializer.py | 6 + test_elasticsearch/test_transport.py | 14 +- 11 files changed, 267 insertions(+), 48 deletions(-) create mode 100644 test_elasticsearch/test_client/test_serializers.py diff --git a/elasticsearch/_async/client/__init__.py b/elasticsearch/_async/client/__init__.py index 825a81bdd..47a8b1177 100644 --- a/elasticsearch/_async/client/__init__.py +++ b/elasticsearch/_async/client/__init__.py @@ -352,9 +352,22 @@ def __init__( if meta_header is not DEFAULT: transport_kwargs["meta_header"] = meta_header - transport_serializers = DEFAULT_SERIALIZERS + transport_serializers = DEFAULT_SERIALIZERS.copy() if serializers is not DEFAULT: transport_serializers.update(serializers) + + # Override compatibility serializers from their non-compat mimetypes too. + # So we use the same serializer for requests and responses. + for mime_subtype in ("json", "x-ndjson"): + if f"application/{mime_subtype}" in serializers: + compat_mimetype = ( + f"application/vnd.elasticsearch+{mime_subtype}" + ) + if compat_mimetype not in serializers: + transport_serializers[compat_mimetype] = serializers[ + f"application/{mime_subtype}" + ] + transport_kwargs["serializers"] = transport_serializers transport_kwargs["default_mimetype"] = default_mimetype diff --git a/elasticsearch/_async/client/_base.py b/elasticsearch/_async/client/_base.py index c8cc4f97a..a994534de 100644 --- a/elasticsearch/_async/client/_base.py +++ b/elasticsearch/_async/client/_base.py @@ -44,6 +44,7 @@ ) from elastic_transport.client_utils import DEFAULT, DefaultType +from ..._version import __versionstr__ from ...compat import warn_stacklevel from ...exceptions import ( HTTP_EXCEPTIONS, @@ -56,6 +57,11 @@ from .utils import _TYPE_ASYNC_SNIFF_CALLBACK, _base64_auth_header, _quote_query _WARNING_RE = re.compile(r"\"([^\"]*)\"") +_COMPAT_MIMETYPE_TEMPLATE = "application/vnd.elasticsearch+%s; compatible-with=" + str( + __versionstr__.partition(".")[0] +) +_COMPAT_MIMETYPE_RE = re.compile(r"application/(json|x-ndjson|vnd\.mapbox-vector-tile)") +_COMPAT_MIMETYPE_SUB = _COMPAT_MIMETYPE_TEMPLATE % (r"\g<1>",) def resolve_auth_headers( @@ -166,7 +172,9 @@ async def sniff_callback( meta, node_infos = await transport.perform_request( "GET", "/_nodes/_all/http", - headers={"accept": "application/json"}, + headers={ + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, request_timeout=( sniff_options.sniff_timeout if not sniff_options.is_initial_sniff @@ -257,6 +265,19 @@ async def perform_request( else: request_headers = self._headers + def mimetype_header_to_compat(header: str) -> None: + # Converts all parts of a Accept/Content-Type headers + # from application/X -> application/vnd.elasticsearch+X + nonlocal request_headers + mimetype = request_headers.get(header, None) + if mimetype: + request_headers[header] = _COMPAT_MIMETYPE_RE.sub( + _COMPAT_MIMETYPE_SUB, mimetype + ) + + mimetype_header_to_compat("Accept") + mimetype_header_to_compat("Content-Type") + if params: target = f"{path}?{_quote_query(params)}" else: diff --git a/elasticsearch/_sync/client/__init__.py b/elasticsearch/_sync/client/__init__.py index b7a8e59a2..3e448e886 100644 --- a/elasticsearch/_sync/client/__init__.py +++ b/elasticsearch/_sync/client/__init__.py @@ -352,9 +352,22 @@ def __init__( if meta_header is not DEFAULT: transport_kwargs["meta_header"] = meta_header - transport_serializers = DEFAULT_SERIALIZERS + transport_serializers = DEFAULT_SERIALIZERS.copy() if serializers is not DEFAULT: transport_serializers.update(serializers) + + # Override compatibility serializers from their non-compat mimetypes too. + # So we use the same serializer for requests and responses. + for mime_subtype in ("json", "x-ndjson"): + if f"application/{mime_subtype}" in serializers: + compat_mimetype = ( + f"application/vnd.elasticsearch+{mime_subtype}" + ) + if compat_mimetype not in serializers: + transport_serializers[compat_mimetype] = serializers[ + f"application/{mime_subtype}" + ] + transport_kwargs["serializers"] = transport_serializers transport_kwargs["default_mimetype"] = default_mimetype diff --git a/elasticsearch/_sync/client/_base.py b/elasticsearch/_sync/client/_base.py index 7d3af12bc..ae869db84 100644 --- a/elasticsearch/_sync/client/_base.py +++ b/elasticsearch/_sync/client/_base.py @@ -44,6 +44,7 @@ ) from elastic_transport.client_utils import DEFAULT, DefaultType +from ..._version import __versionstr__ from ...compat import warn_stacklevel from ...exceptions import ( HTTP_EXCEPTIONS, @@ -56,6 +57,11 @@ from .utils import _TYPE_SYNC_SNIFF_CALLBACK, _base64_auth_header, _quote_query _WARNING_RE = re.compile(r"\"([^\"]*)\"") +_COMPAT_MIMETYPE_TEMPLATE = "application/vnd.elasticsearch+%s; compatible-with=" + str( + __versionstr__.partition(".")[0] +) +_COMPAT_MIMETYPE_RE = re.compile(r"application/(json|x-ndjson|vnd\.mapbox-vector-tile)") +_COMPAT_MIMETYPE_SUB = _COMPAT_MIMETYPE_TEMPLATE % (r"\g<1>",) def resolve_auth_headers( @@ -166,7 +172,9 @@ def sniff_callback( meta, node_infos = transport.perform_request( "GET", "/_nodes/_all/http", - headers={"accept": "application/json"}, + headers={ + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, request_timeout=( sniff_options.sniff_timeout if not sniff_options.is_initial_sniff @@ -257,6 +265,19 @@ def perform_request( else: request_headers = self._headers + def mimetype_header_to_compat(header: str) -> None: + # Converts all parts of a Accept/Content-Type headers + # from application/X -> application/vnd.elasticsearch+X + nonlocal request_headers + mimetype = request_headers.get(header, None) + if mimetype: + request_headers[header] = _COMPAT_MIMETYPE_RE.sub( + _COMPAT_MIMETYPE_SUB, mimetype + ) + + mimetype_header_to_compat("Accept") + mimetype_header_to_compat("Content-Type") + if params: target = f"{path}?{_quote_query(params)}" else: diff --git a/elasticsearch/serializer.py b/elasticsearch/serializer.py index d5d27ec04..758c6b730 100644 --- a/elasticsearch/serializer.py +++ b/elasticsearch/serializer.py @@ -25,7 +25,6 @@ from elastic_transport import Serializer as Serializer from elastic_transport import TextSerializer as TextSerializer -from .compat import to_bytes from .exceptions import SerializationError INTEGER_TYPES = () @@ -37,7 +36,8 @@ "JsonSerializer", "TextSerializer", "NdjsonSerializer", - "CompatibilityModeSerializer", + "CompatibilityModeJsonSerializer", + "CompatibilityModeNdjsonSerializer", "MapboxVectorTileSerializer", ] @@ -80,27 +80,12 @@ def default(self, data: Any) -> Any: return JsonSerializer.default(self, data) -class CompatibilityModeSerializer(JsonSerializer): +class CompatibilityModeJsonSerializer(JsonSerializer): mimetype: ClassVar[str] = "application/vnd.elasticsearch+json" - def dumps(self, data: Any) -> bytes: - if isinstance(data, str): - data = data.encode("utf-8", "surrogatepass") - if isinstance(data, bytes): - return data - if isinstance(data, (tuple, list)): - return NdjsonSerializer.dumps(self, data) # type: ignore - return JsonSerializer.dumps(self, data) - def loads(self, data: bytes) -> Any: - if isinstance(data, str): - data = to_bytes(data, "utf-8") - if isinstance(data, bytes) and data.endswith(b"\n"): - return NdjsonSerializer.loads(self, data) # type: ignore - try: # Try as JSON first but if that fails then try NDJSON. - return JsonSerializer.loads(self, data) - except SerializationError: - return NdjsonSerializer.loads(self, data) # type: ignore +class CompatibilityModeNdjsonSerializer(NdjsonSerializer): + mimetype: ClassVar[str] = "application/vnd.elasticsearch+x-ndjson" class MapboxVectorTileSerializer(Serializer): @@ -119,7 +104,8 @@ def dumps(self, data: bytes) -> bytes: JsonSerializer.mimetype: JsonSerializer(), MapboxVectorTileSerializer.mimetype: MapboxVectorTileSerializer(), NdjsonSerializer.mimetype: NdjsonSerializer(), - CompatibilityModeSerializer.mimetype: CompatibilityModeSerializer(), + CompatibilityModeJsonSerializer.mimetype: CompatibilityModeJsonSerializer(), + CompatibilityModeNdjsonSerializer.mimetype: CompatibilityModeNdjsonSerializer(), } # Alias for backwards compatibility diff --git a/test_elasticsearch/test_async/test_transport.py b/test_elasticsearch/test_async/test_transport.py index 1bf67d65a..17f50f02a 100644 --- a/test_elasticsearch/test_async/test_transport.py +++ b/test_elasticsearch/test_async/test_transport.py @@ -243,7 +243,7 @@ async def test_client_meta_header_not_sent(self): calls = client.transport.node_pool.get().calls assert 1 == len(calls) assert calls[0][1]["headers"] == { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } async def test_body_surrogates_replaced_encoded_into_bytes(self): @@ -391,7 +391,9 @@ async def test_sniff_on_start_ignores_sniff_timeout(self): ("GET", "/_nodes/_all/http"), { "body": None, - "headers": {"accept": "application/json"}, + "headers": { + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, "request_timeout": None, # <-- Should be None instead of 12 }, ) @@ -418,7 +420,7 @@ async def test_sniff_uses_sniff_timeout(self): { "body": None, "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "request_timeout": DEFAULT, }, @@ -427,7 +429,9 @@ async def test_sniff_uses_sniff_timeout(self): ("GET", "/_nodes/_all/http"), { "body": None, - "headers": {"accept": "application/json"}, + "headers": { + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, "request_timeout": 12, }, ) @@ -681,7 +685,7 @@ async def test_unsupported_product_error(headers): { "body": None, "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "request_timeout": DEFAULT, }, diff --git a/test_elasticsearch/test_client/test_deprecated_options.py b/test_elasticsearch/test_client/test_deprecated_options.py index 37977ce82..dd1016bb9 100644 --- a/test_elasticsearch/test_client/test_deprecated_options.py +++ b/test_elasticsearch/test_client/test_deprecated_options.py @@ -135,6 +135,7 @@ class CustomSerializer(JsonSerializer): "application/json", "text/*", "application/vnd.elasticsearch+json", + "application/vnd.elasticsearch+x-ndjson", } client = Elasticsearch( @@ -154,5 +155,6 @@ class CustomSerializer(JsonSerializer): "application/json", "text/*", "application/vnd.elasticsearch+json", + "application/vnd.elasticsearch+x-ndjson", "application/cbor", } diff --git a/test_elasticsearch/test_client/test_options.py b/test_elasticsearch/test_client/test_options.py index 5984a7464..16e89af53 100644 --- a/test_elasticsearch/test_client/test_options.py +++ b/test_elasticsearch/test_client/test_options.py @@ -139,7 +139,7 @@ def test_options_passed_to_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, } @@ -157,7 +157,7 @@ def test_options_passed_to_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, "request_timeout": 1, @@ -182,7 +182,7 @@ def test_options_passed_to_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, "request_timeout": 1, @@ -209,7 +209,7 @@ async def test_options_passed_to_async_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, } @@ -227,7 +227,7 @@ async def test_options_passed_to_async_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, "request_timeout": 1, @@ -252,7 +252,7 @@ async def test_options_passed_to_async_perform_request(self): assert call.pop("client_meta") is DEFAULT assert call == { "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "body": None, "request_timeout": 1, @@ -294,7 +294,7 @@ def test_http_headers_overrides(self): assert call["headers"] == { "key": "val", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client.options(headers={"key1": "val"}).indices.get(index="2") @@ -303,7 +303,7 @@ def test_http_headers_overrides(self): assert call["headers"] == { "key": "val", "key1": "val", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client.options(headers={"key": "val2"}).indices.get(index="3") @@ -311,7 +311,7 @@ def test_http_headers_overrides(self): assert call["headers"] == { "key": "val2", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client = Elasticsearch( @@ -338,14 +338,14 @@ def test_user_agent_override(self): call = calls[("GET", "/1")][0] assert call["headers"] == { "user-agent": "custom1", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client.indices.get(index="2", headers={"user-agent": "custom2"}) call = calls[("GET", "/2")][0] assert call["headers"] == { "user-agent": "custom2", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client = Elasticsearch( @@ -359,12 +359,12 @@ def test_user_agent_override(self): call = calls[("GET", "/1")][0] assert call["headers"] == { "user-agent": "custom3", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } client.indices.get(index="2", headers={"user-agent": "custom4"}) call = calls[("GET", "/2")][0] assert call["headers"] == { "user-agent": "custom4", - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } diff --git a/test_elasticsearch/test_client/test_serializers.py b/test_elasticsearch/test_client/test_serializers.py new file mode 100644 index 000000000..fa1ea362c --- /dev/null +++ b/test_elasticsearch/test_client/test_serializers.py @@ -0,0 +1,149 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License 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 elasticsearch import Elasticsearch +from test_elasticsearch.test_cases import DummyTransportTestCase + + +class TestSerializers(DummyTransportTestCase): + def test_compat_mode_on_by_default(self): + calls = self.client.transport.calls + + # Get, never uses a body + self.client.get(index="test0", id="1") + assert len(calls) == 1 + assert calls[("GET", "/test0/_doc/1")][0]["headers"] == { + "Accept": "application/vnd.elasticsearch+json; compatible-with=8" + } + + # Search with body + self.client.search(index="test1", query={"match_all": {}}) + assert len(calls) == 2 + assert calls[("POST", "/test1/_search")][0]["headers"] == { + "Accept": "application/vnd.elasticsearch+json; compatible-with=8", + "Content-Type": "application/vnd.elasticsearch+json; compatible-with=8", + } + + # Search without body + self.client.search(index="test2") + assert len(calls) == 3 + assert calls[("POST", "/test2/_search")][0]["headers"] == { + "Accept": "application/vnd.elasticsearch+json; compatible-with=8", + } + + # Multiple mimetypes in Accept + self.client.cat.nodes() + assert len(calls) == 4 + assert calls[("GET", "/_cat/nodes")][0]["headers"] == { + # text/plain isn't modified. + "Accept": "text/plain,application/vnd.elasticsearch+json; compatible-with=8", + } + + # Bulk uses x-ndjson + self.client.bulk(operations=[]) + assert len(calls) == 5 + assert calls[("PUT", "/_bulk")][0]["headers"] == { + "Accept": "application/vnd.elasticsearch+json; compatible-with=8", + "Content-Type": "application/vnd.elasticsearch+x-ndjson; compatible-with=8", + } + + # Mapbox vector tiles + self.client.search_mvt( + index="test3", + field="field", + zoom="z", + y="y", + x="x", + query={"match_all": {}}, + ) + assert len(calls) == 6 + assert calls[("POST", "/test3/_mvt/field/z/x/y")][0]["headers"] == { + "Accept": "application/vnd.elasticsearch+vnd.mapbox-vector-tile; compatible-with=8", + "Content-Type": "application/vnd.elasticsearch+json; compatible-with=8", + } + + @pytest.mark.parametrize("mime_subtype", ["json", "x-ndjson"]) + def test_compat_serializers_used_when_given_non_compat( + self, mime_subtype: str + ) -> None: + class CustomSerializer: + pass + + ser = CustomSerializer() + client = Elasticsearch( + "https://localhost:9200", serializers={f"application/{mime_subtype}": ser} + ) + serializers = client.transport.serializers.serializers + assert set(serializers.keys()) == { + "application/json", + "text/*", + "application/x-ndjson", + "application/vnd.mapbox-vector-tile", + "application/vnd.elasticsearch+json", + "application/vnd.elasticsearch+x-ndjson", + } + + assert serializers[f"application/{mime_subtype}"] is ser + assert serializers[f"application/vnd.elasticsearch+{mime_subtype}"] is ser + + @pytest.mark.parametrize("mime_subtype", ["json", "x-ndjson"]) + def test_compat_serializers_used_when_given_compat(self, mime_subtype: str) -> None: + class CustomSerializer: + pass + + ser1 = CustomSerializer() + ser2 = CustomSerializer() + client = Elasticsearch( + "https://localhost:9200", + serializers={ + f"application/{mime_subtype}": ser1, + f"application/vnd.elasticsearch+{mime_subtype}": ser2, + }, + ) + serializers = client.transport.serializers.serializers + assert set(serializers.keys()) == { + "application/json", + "text/*", + "application/x-ndjson", + "application/vnd.mapbox-vector-tile", + "application/vnd.elasticsearch+json", + "application/vnd.elasticsearch+x-ndjson", + } + + assert serializers[f"application/{mime_subtype}"] is ser1 + assert serializers[f"application/vnd.elasticsearch+{mime_subtype}"] is ser2 + + def test_compat_serializer_used_when_given_non_compat(self) -> None: + class CustomSerializer: + mimetype: str = "application/json" + + ser = CustomSerializer() + client = Elasticsearch("https://localhost:9200", serializer=ser) + serializers = client.transport.serializers.serializers + assert set(serializers.keys()) == { + "application/json", + "text/*", + "application/x-ndjson", + "application/vnd.mapbox-vector-tile", + "application/vnd.elasticsearch+json", + "application/vnd.elasticsearch+x-ndjson", + } + + assert serializers["application/json"] is ser + assert serializers["application/vnd.elasticsearch+json"] is ser diff --git a/test_elasticsearch/test_serializer.py b/test_elasticsearch/test_serializer.py index 20fe61b99..65f13cf3a 100644 --- a/test_elasticsearch/test_serializer.py +++ b/test_elasticsearch/test_serializer.py @@ -207,6 +207,12 @@ def test_deserialize_compatibility_header(self): '{"some":"data"}', content_type ) + for content_type in ( + "application/vnd.elasticsearch+x-ndjson;compatible-with=7", + "application/vnd.elasticsearch+x-ndjson; compatible-with=7", + "application/vnd.elasticsearch+x-ndjson;compatible-with=8", + "application/vnd.elasticsearch+x-ndjson; compatible-with=8", + ): assert b'{"some":"data"}\n{"some":"data"}\n' == self.serializers.dumps( ['{"some":"data"}', {"some": "data"}], content_type ) diff --git a/test_elasticsearch/test_transport.py b/test_elasticsearch/test_transport.py index 340b2b5b4..9e78bdf64 100644 --- a/test_elasticsearch/test_transport.py +++ b/test_elasticsearch/test_transport.py @@ -257,7 +257,7 @@ def test_client_meta_header_not_sent(self): calls = client.transport.node_pool.get().calls assert 1 == len(calls) assert calls[0][1]["headers"] == { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", } def test_meta_header_type_error(self): @@ -396,7 +396,9 @@ def test_sniff_on_start_ignores_sniff_timeout(self): ("GET", "/_nodes/_all/http"), { "body": None, - "headers": {"accept": "application/json"}, + "headers": { + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, "request_timeout": None, # <-- Should be None instead of 12 }, ) @@ -418,7 +420,9 @@ def test_sniff_uses_sniff_timeout(self): ("GET", "/_nodes/_all/http"), { "body": None, - "headers": {"accept": "application/json"}, + "headers": { + "accept": "application/vnd.elasticsearch+json; compatible-with=8" + }, "request_timeout": 12, }, ) @@ -427,7 +431,7 @@ def test_sniff_uses_sniff_timeout(self): { "body": None, "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "request_timeout": DEFAULT, }, @@ -594,7 +598,7 @@ def test_unsupported_product_error(headers): { "body": None, "headers": { - "accept": "application/json", + "accept": "application/vnd.elasticsearch+json; compatible-with=8", }, "request_timeout": DEFAULT, },