Add support for disabling typed_keys serialization

Signed-off-by: Thomas Farr <[email protected]>
opensearch-project · Nov 15, 2024 · bdafb4c · bdafb4c
1 parent c7103d8
commit bdafb4c
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ This section is for maintaining a changelog for all breaking changes for the cli
 ## [Unreleased 2.x]
 
 ### Added
+- Added support for disabling `typed_keys` serialization ([#]())
 
 ### Dependencies
 

diff --git a/java-client/src/main/java/org/opensearch/client/json/AttributedJsonpMapper.java b/java-client/src/main/java/org/opensearch/client/json/AttributedJsonpMapper.java
@@ -36,6 +36,7 @@
 import jakarta.json.stream.JsonGenerator;
 import jakarta.json.stream.JsonParser;
 
+@Deprecated
 class AttributedJsonpMapper implements JsonpMapper {
 
     private final JsonpMapper mapper;

diff --git a/java-client/src/main/java/org/opensearch/client/json/ExternallyTaggedUnion.java b/java-client/src/main/java/org/opensearch/client/json/ExternallyTaggedUnion.java
@@ -188,10 +188,17 @@ public void deserializeEntry(String key, JsonParser parser, JsonpMapper mapper,
         JsonGenerator generator,
         JsonpMapper mapper
     ) {
-        for (Map.Entry<String, T> entry : map.entrySet()) {
-            T value = entry.getValue();
-            generator.writeKey(value._kind().jsonValue() + "#" + entry.getKey());
-            value.serialize(generator, mapper);
+        if (mapper.attribute(JsonpMapperAttributes.SERIALIZE_TYPED_KEYS, true)) {
+            for (Map.Entry<String, T> entry : map.entrySet()) {
+                T value = entry.getValue();
+                generator.writeKey(value._kind().jsonValue() + "#" + entry.getKey());
+                value.serialize(generator, mapper);
+            }
+        } else {
+            for (Map.Entry<String, T> entry : map.entrySet()) {
+                generator.writeKey(entry.getKey());
+                entry.getValue().serialize(generator, mapper);
+            }
         }
     }
 }
diff --git a/java-client/src/main/java/org/opensearch/client/json/JsonpMapper.java b/java-client/src/main/java/org/opensearch/client/json/JsonpMapper.java
@@ -88,8 +88,12 @@ default <T> T attribute(String name, T defaultValue) {
 
     /**
      * Create a new mapper with a named attribute that delegates to this one.
+     *
+     * @implNote The default implementation will be removed in a future release, and inheritors will be required to implement it themselves.
+     *           See {@link org.opensearch.client.json.jsonb.JsonbJsonpMapper#withAttribute(String, Object)} and {@link org.opensearch.client.json.jackson.JacksonJsonpMapper#withAttribute(String, Object)} for examples.
      */
     default <T> JsonpMapper withAttribute(String name, T value) {
+        //noinspection deprecation
         return new AttributedJsonpMapper(this, name, value);
     }
 }
diff --git a/java-client/src/main/java/org/opensearch/client/json/JsonpMapperAttributes.java b/java-client/src/main/java/org/opensearch/client/json/JsonpMapperAttributes.java
@@ -0,0 +1,8 @@
+package org.opensearch.client.json;
+
+public final class JsonpMapperAttributes {
+    private JsonpMapperAttributes() {
+    }
+
+    public static final String SERIALIZE_TYPED_KEYS = JsonpMapperAttributes.class.getName() + ":SERIALIZE_TYPED_KEYS";
+}
diff --git a/java-client/src/main/java/org/opensearch/client/json/JsonpMapperBase.java b/java-client/src/main/java/org/opensearch/client/json/JsonpMapperBase.java
@@ -36,9 +36,46 @@
 import jakarta.json.stream.JsonGenerator;
 import jakarta.json.stream.JsonParser;
 import java.lang.reflect.Field;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import javax.annotation.Nullable;
 
 public abstract class JsonpMapperBase implements JsonpMapper {
+    @Nullable
+    private Map<String, Object> attributes;
+
+    protected JsonpMapperBase() {
+    }
+
+    protected JsonpMapperBase(JsonpMapperBase o) {
+        this.attributes = o.attributes; // We always copy in `setAttribute` so no need to copy here.
+    }
+
+    @Override
+    public <T> T attribute(String name) {
+        //noinspection unchecked
+        return attributes == null ? null : (T) attributes.get(name);
+    }
+
+    /**
+     * Updates attributes to a copy of the current ones with an additional key/value pair.
+     * Mutates the current mapper, intended to be used in implementations of {@link #withAttribute(String, Object)}
+     */
+    protected JsonpMapperBase addAttribute(String name, Object value) {
+        if (attributes == null) {
+            this.attributes = Collections.singletonMap(name, value);
+        } else {
+            // Copy the map to avoid modifying the original in case it was shared.
+            // We're generally only ever called from implementations' `withAttribute` methods which are intended
+            // to construct new instances rather than modify existing ones.
+            Map<String, Object> attributes = new HashMap<>(this.attributes.size() + 1);
+            attributes.putAll(this.attributes);
+            attributes.put(name, value);
+            this.attributes = attributes;
+        }
+        return this;
+    }
 
     /** Get a serializer when none of the builtin ones are applicable */
     protected abstract <T> JsonpDeserializer<T> getDefaultDeserializer(Class<T> clazz);

diff --git a/java-client/src/main/java/org/opensearch/client/json/jackson/JacksonJsonpMapper.java b/java-client/src/main/java/org/opensearch/client/json/jackson/JacksonJsonpMapper.java
@@ -48,7 +48,6 @@
 import org.opensearch.client.json.JsonpSerializer;
 
 public class JacksonJsonpMapper extends JsonpMapperBase {
-
     private final JacksonJsonProvider provider;
     private final ObjectMapper objectMapper;
 
@@ -68,6 +67,17 @@ public JacksonJsonpMapper() {
         );
     }
 
+    private JacksonJsonpMapper(JacksonJsonpMapper o) {
+        super(o);
+        this.provider = o.provider;
+        this.objectMapper = o.objectMapper;
+    }
+
+    @Override
+    public <T> JsonpMapper withAttribute(String name, T value) {
+        return new JacksonJsonpMapper(this).addAttribute(name, value);
+    }
+
     /**
      * Returns the underlying Jackson mapper.
      */

diff --git a/java-client/src/main/java/org/opensearch/client/json/jsonb/JsonbJsonpMapper.java b/java-client/src/main/java/org/opensearch/client/json/jsonb/JsonbJsonpMapper.java
@@ -65,6 +65,17 @@ public JsonbJsonpMapper() {
         this(JsonProvider.provider(), JsonbProvider.provider());
     }
 
+    private JsonbJsonpMapper(JsonbJsonpMapper o) {
+        super(o);
+        this.jsonProvider = o.jsonProvider;
+        this.jsonb = o.jsonb;
+    }
+
+    @Override
+    public <T> JsonpMapper withAttribute(String name, T value) {
+        return new JsonbJsonpMapper(this).addAttribute(name, value);
+    }
+
     @Override
     protected <T> JsonpDeserializer<T> getDefaultDeserializer(Class<T> clazz) {
         return new Deserializer<>(clazz);

diff --git a/...t/src/test/java/org/opensearch/client/opensearch/json/jackson/JacksonJsonpParserTest.java b/...t/src/test/java/org/opensearch/client/opensearch/json/jackson/JacksonJsonpParserTest.java
@@ -32,18 +32,12 @@
 
 package org.opensearch.client.opensearch.json.jackson;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import jakarta.json.stream.JsonParser;
 import jakarta.json.stream.JsonParser.Event;
 import java.io.StringReader;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import javax.annotation.Nullable;
 import org.junit.Test;
 import org.opensearch.client.json.JsonpDeserializer;
 import org.opensearch.client.json.JsonpMapper;
-import org.opensearch.client.json.JsonpMapperBase;
 import org.opensearch.client.json.jackson.JacksonJsonProvider;
 import org.opensearch.client.json.jackson.JacksonJsonpMapper;
 import org.opensearch.client.opensearch.core.MsearchResponse;
@@ -176,7 +170,7 @@ public void testMultiSearchResponse() {
             + "  ]\n"
             + "}\n";
 
-        JsonpMapper mapper = new AttributedJacksonJsonpMapper().withAttribute(
+        JsonpMapper mapper = new JacksonJsonpMapper().withAttribute(
             "org.opensearch.client:Deserializer:_global.msearch.TDocument",
             JsonpDeserializer.of(Foo.class)
         );
@@ -189,46 +183,6 @@ public void testMultiSearchResponse() {
         assertEquals((Integer) 200, response.responses().get(1).result().status());
     }
 
-    public static class AttributedJacksonJsonpMapper extends JacksonJsonpMapper {
-        private Map<String, Object> attributes;
-
-        public AttributedJacksonJsonpMapper() {
-            super();
-        }
-
-        public AttributedJacksonJsonpMapper(ObjectMapper objectMapper) {
-            super(objectMapper);
-        }
-
-        @Nullable
-        @Override
-        @SuppressWarnings("unchecked")
-        public <T> T attribute(String name) {
-            return attributes == null ? null : (T) attributes.get(name);
-        }
-
-        /**
-         * Updates attributes to a copy of the current ones with an additional key/value pair.
-         * Mutates the current mapper, intended to be used in implementations of {@link #withAttribute(String, Object)}
-         */
-        protected JsonpMapperBase addAttribute(String name, Object value) {
-            if (attributes == null) {
-                this.attributes = Collections.singletonMap(name, value);
-            } else {
-                Map<String, Object> newAttrs = new HashMap<>(attributes.size() + 1);
-                newAttrs.putAll(attributes);
-                newAttrs.put(name, value);
-                this.attributes = newAttrs;
-            }
-            return this;
-        }
-
-        @Override
-        public <T> JsonpMapper withAttribute(String name, T value) {
-            return new AttributedJacksonJsonpMapper(objectMapper()).addAttribute(name, value);
-        }
-    }
-
     public static class Foo {
         private int x;
         private boolean y;

diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/model/TypedKeysTest.java b/java-client/src/test/java/org/opensearch/client/opensearch/model/TypedKeysTest.java
@@ -32,9 +32,13 @@
 
 package org.opensearch.client.opensearch.model;
 
+import java.util.ArrayList;
 import java.util.Collections;
+
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.junit.Test;
 import org.opensearch.client.json.JsonpDeserializer;
+import org.opensearch.client.json.JsonpMapperAttributes;
 import org.opensearch.client.opensearch._types.aggregations.Aggregate;
 import org.opensearch.client.opensearch._types.aggregations.AvgAggregate;
 import org.opensearch.client.opensearch._types.aggregations.StringTermsAggregate;
@@ -113,4 +117,19 @@ public void testAdditionalProperties() {
         assertEquals("key_2", foo.buckets().array().get(1).key());
         assertEquals(2.0, foo.buckets().array().get(1).aggregations().get("bar").avg().value(), 0.01);
     }
+
+    @Test
+    public void testDisablingSerializeTypedKeys() {
+        SearchResponse<ObjectNode> resp = new SearchResponse.Builder<ObjectNode>()
+                .aggregations("aggKey", v -> v.lterms(lv -> lv.buckets(b -> b.array(new ArrayList<>())).sumOtherDocCount(0)))
+                .took(0)
+                .timedOut(false)
+                .shards(s -> s.failed(0).successful(1).total(1))
+                .hits(h -> h.hits(new ArrayList<>()))
+                .build();
+
+        String json = "{\"took\":0,\"timed_out\":false,\"_shards\":{\"failed\":0,\"successful\":1,\"total\":1},\"hits\":{\"hits\":[]},\"aggregations\":{\"aggKey\":{\"buckets\":[],\"sum_other_doc_count\":0}}}";
+
+        assertEquals(json, toJson(resp, mapper.withAttribute(JsonpMapperAttributes.SERIALIZE_TYPED_KEYS, false)));
+    }
 }