From e0294e293ff36db05e5da4ff09bf0807ac356cef Mon Sep 17 00:00:00 2001 From: cecemei Date: Tue, 23 Sep 2025 11:23:33 -0700 Subject: [PATCH 1/6] strict-type-id --- .../data/input/impl/DimensionSchema.java | 22 +++- .../druid/jackson/StrictTypeIdResolver.java | 115 ++++++++++++++++++ .../data/input/impl/DimensionSchemaTest.java | 19 +++ 3 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java b/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java index b5933e722d8c..faf410dfb51d 100644 --- a/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java +++ b/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java @@ -25,9 +25,11 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonValue; +import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver; import com.google.common.base.Strings; import org.apache.druid.guice.BuiltInTypesModule; import org.apache.druid.guice.annotations.PublicApi; +import org.apache.druid.jackson.StrictTypeIdResolver; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.segment.AutoTypeColumnSchema; @@ -43,9 +45,27 @@ import java.util.Objects; /** + * Defines the schema of a single dimension in a dataset. + *

+ * Includes metadata such as the dimension's name, type, and whether it + * can hold multiple values. Supports Jackson serialization/deserialization, + * including polymorphic types via {@code @JsonSubTypes}. + *

+ * + *

+ * Example JSON: + *

{@code
+ * {
+ *     "type": "string",
+ *     "name": "country",
+ *     "multiValue": false
+ * }
+ * }
+ *

*/ @PublicApi -@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = StringDimensionSchema.class) +@JsonTypeIdResolver(StrictTypeIdResolver.class) +@JsonTypeInfo(use = JsonTypeInfo.Id.CUSTOM, property = "type", defaultImpl = StringDimensionSchema.class) @JsonSubTypes(value = { @JsonSubTypes.Type(name = DimensionSchema.STRING_TYPE_NAME, value = StringDimensionSchema.class), @JsonSubTypes.Type(name = DimensionSchema.LONG_TYPE_NAME, value = LongDimensionSchema.class), diff --git a/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java new file mode 100644 index 000000000000..248f522e4582 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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. + */ + +package org.apache.druid.jackson; + +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import com.fasterxml.jackson.databind.DatabindContext; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.jsontype.impl.TypeIdResolverBase; +import org.apache.druid.java.util.common.IAE; + +import java.util.HashMap; +import java.util.Map; + +/** + * A custom {@link com.fasterxml.jackson.databind.jsontype.TypeIdResolver} that enforces strict type id validation. + *

+ * During deserialization, the JSON property used as the type discriminator must match one of the registered subtypes. + *

+ * If the type is missing, the resolver can fall back to a configured default implementation. + */ +public class StrictTypeIdResolver extends TypeIdResolverBase +{ + private Map> typeMap; + private JavaType baseType; + + @Override + public void init(JavaType baseType) + { + this.baseType = baseType; + } + + private void ensureTypeMap() + { + if (typeMap != null) { + return; + } + + typeMap = new HashMap<>(); + Class raw = baseType.getRawClass(); + // walk class hierarchy to collect @JsonSubTypes + while (raw != null) { + JsonSubTypes subTypes = raw.getAnnotation(JsonSubTypes.class); + if (subTypes != null) { + for (JsonSubTypes.Type t : subTypes.value()) { + typeMap.put(t.name(), t.value()); + } + } + raw = raw.getSuperclass(); + } + + if (typeMap.isEmpty()) { + throw new IllegalStateException("No @JsonSubTypes found for " + baseType.getRawClass()); + } + } + + @Override + public String idFromValue(Object value) + { + ensureTypeMap(); + Class clazz = value.getClass(); + // find the name from @JsonSubTypes + for (Map.Entry> e : typeMap.entrySet()) { + if (e.getValue().equals(clazz)) { + return e.getKey(); + } + } + throw new IllegalArgumentException("Unknown class: " + clazz.getName()); + } + + @Override + public String idFromValueAndType(Object value, Class suggestedType) + { + ensureTypeMap(); + return idFromValue(value); + } + + @Override + public JavaType typeFromId(DatabindContext context, String id) + { + ensureTypeMap(); + if (id == null) { + // Missing type, caller decides default + return null; + } + Class clazz = typeMap.get(id); + if (clazz == null) { + throw new IAE("Unknown type[%s]", id); + } + return context.constructType(clazz); + } + + @Override + public Id getMechanism() + { + return Id.CUSTOM; + } +} diff --git a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java index ed13343db4c2..695d6f2f6e31 100644 --- a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java +++ b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java @@ -20,6 +20,7 @@ package org.apache.druid.data.input.impl; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.druid.java.util.common.IAE; import org.junit.Assert; import org.junit.Test; @@ -46,4 +47,22 @@ public void testStringDimensionSchemaSerde() throws Exception OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(schema2), DimensionSchema.class) ); } + + @Test + public void testDeserializeStrictTypeId() throws Exception + { + final String invalidType = "{\"type\":\"invalid\",\"name\":\"foo\",\"multiValueHandling\":\"ARRAY\",\"createBitmapIndex\":false}"; + IAE e = Assert.assertThrows(IAE.class, () -> OBJECT_MAPPER.readValue(invalidType, DimensionSchema.class)); + Assert.assertEquals("Unknown type[invalid]", e.getMessage()); + } + + @Test + public void testDeserializeDefaultAsString() throws Exception + { + final String noType = "{\"name\":\"foo\",\"multiValueHandling\":\"ARRAY\",\"createBitmapIndex\":false}"; + Assert.assertEquals( + new StringDimensionSchema("foo", DimensionSchema.MultiValueHandling.ARRAY, false), + OBJECT_MAPPER.readValue(noType, DimensionSchema.class) + ); + } } From 5ab276aa32feafed12131674ff6a9b06c276f384 Mon Sep 17 00:00:00 2001 From: cecemei Date: Tue, 23 Sep 2025 12:02:48 -0700 Subject: [PATCH 2/6] test --- .../apache/druid/data/input/impl/StringDimensionSchemaTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/processing/src/test/java/org/apache/druid/data/input/impl/StringDimensionSchemaTest.java b/processing/src/test/java/org/apache/druid/data/input/impl/StringDimensionSchemaTest.java index ad2909722d2f..cfc9006fe57e 100644 --- a/processing/src/test/java/org/apache/druid/data/input/impl/StringDimensionSchemaTest.java +++ b/processing/src/test/java/org/apache/druid/data/input/impl/StringDimensionSchemaTest.java @@ -52,7 +52,6 @@ public void testDeserializeFromSimpleString() throws JsonProcessingException public void testDeserializeFromJson() throws JsonProcessingException { final String json = "{\n" - + " \"type\" : \"StringDimensionSchema\",\n" + " \"name\" : \"dim\",\n" + " \"multiValueHandling\" : \"SORTED_SET\",\n" + " \"createBitmapIndex\" : false\n" From baf781321c056ecf47287505135f40ea50ed2875 Mon Sep 17 00:00:00 2001 From: cecemei Date: Tue, 23 Sep 2025 19:41:18 -0700 Subject: [PATCH 3/6] trigger ci / empty commit From a73daebe8a63971f275f540f57fa1de5471c1562 Mon Sep 17 00:00:00 2001 From: cecemei Date: Fri, 3 Oct 2025 13:35:13 -0700 Subject: [PATCH 4/6] use-delegate --- .../data/input/impl/DimensionSchema.java | 4 +- .../druid/jackson/StrictTypeIdResolver.java | 116 ++++++++++-------- .../data/input/impl/DimensionSchemaTest.java | 10 +- 3 files changed, 74 insertions(+), 56 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java b/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java index faf410dfb51d..fd4b527c6469 100644 --- a/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java +++ b/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java @@ -25,7 +25,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonValue; -import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver; +import com.fasterxml.jackson.databind.annotation.JsonTypeResolver; import com.google.common.base.Strings; import org.apache.druid.guice.BuiltInTypesModule; import org.apache.druid.guice.annotations.PublicApi; @@ -64,7 +64,7 @@ *

*/ @PublicApi -@JsonTypeIdResolver(StrictTypeIdResolver.class) +@JsonTypeResolver(StrictTypeIdResolver.Builder.class) @JsonTypeInfo(use = JsonTypeInfo.Id.CUSTOM, property = "type", defaultImpl = StringDimensionSchema.class) @JsonSubTypes(value = { @JsonSubTypes.Type(name = DimensionSchema.STRING_TYPE_NAME, value = StringDimensionSchema.class), diff --git a/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java index 248f522e4582..6895d96782d1 100644 --- a/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java +++ b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java @@ -19,92 +19,106 @@ package org.apache.druid.jackson; -import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DatabindContext; +import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.jsontype.NamedType; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; +import com.fasterxml.jackson.databind.jsontype.TypeIdResolver; +import com.fasterxml.jackson.databind.jsontype.impl.StdTypeResolverBuilder; import com.fasterxml.jackson.databind.jsontype.impl.TypeIdResolverBase; -import org.apache.druid.java.util.common.IAE; +import com.fasterxml.jackson.databind.jsontype.impl.TypeNameIdResolver; -import java.util.HashMap; -import java.util.Map; +import java.util.Collection; /** - * A custom {@link com.fasterxml.jackson.databind.jsontype.TypeIdResolver} that enforces strict type id validation. + * A strict {@link TypeIdResolver} implementation that validates all incoming type ids. *

- * During deserialization, the JSON property used as the type discriminator must match one of the registered subtypes. + * During deserialization, the type discriminator in the JSON must correspond to a registered subtype; + * otherwise this resolver will throw an exception instead of silently accepting or defaulting. *

- * If the type is missing, the resolver can fall back to a configured default implementation. + * An optional default implementation may still be configured and used only when the type id is absent. */ public class StrictTypeIdResolver extends TypeIdResolverBase { - private Map> typeMap; - private JavaType baseType; + public static class Builder extends StdTypeResolverBuilder + { + @Override + protected TypeIdResolver idResolver( + MapperConfig config, + JavaType baseType, + PolymorphicTypeValidator subtypeValidator, + Collection subtypes, + boolean forSer, + boolean forDeser + ) + { + this._customIdResolver = new StrictTypeIdResolver(config, baseType, subtypes, forSer, forDeser); + return this._customIdResolver; + } + } - @Override - public void init(JavaType baseType) + protected final JavaType baseType; + protected final TypeNameIdResolver delegate; + + StrictTypeIdResolver() { - this.baseType = baseType; + // Required default constructor for Jackson, the instance is never used + baseType = null; + delegate = null; } - private void ensureTypeMap() + StrictTypeIdResolver( + MapperConfig config, + JavaType baseType, + Collection subtypes, + boolean forSer, + boolean forDeser + ) { - if (typeMap != null) { - return; - } + this.baseType = baseType; + this.delegate = TypeNameIdResolver.construct(config, baseType, subtypes, forSer, forDeser); + } - typeMap = new HashMap<>(); - Class raw = baseType.getRawClass(); - // walk class hierarchy to collect @JsonSubTypes - while (raw != null) { - JsonSubTypes subTypes = raw.getAnnotation(JsonSubTypes.class); - if (subTypes != null) { - for (JsonSubTypes.Type t : subTypes.value()) { - typeMap.put(t.name(), t.value()); - } - } - raw = raw.getSuperclass(); + @Override + public JavaType typeFromId(DatabindContext context, String id) throws JsonProcessingException + { + if (id == null) { + // fallback to default implementation if configured + return null; } - if (typeMap.isEmpty()) { - throw new IllegalStateException("No @JsonSubTypes found for " + baseType.getRawClass()); + JavaType type = delegate.typeFromId(context, id); + if (type == null) { + // in TypeNameIdResolver, it'd fall back to defaultImpl if configured, but we want to error out instead + throw ((DeserializationContext) context).invalidTypeIdException( + baseType, + id, + "known type ids = " + delegate.getDescForKnownTypeIds() + ); } + return type; } @Override public String idFromValue(Object value) { - ensureTypeMap(); - Class clazz = value.getClass(); - // find the name from @JsonSubTypes - for (Map.Entry> e : typeMap.entrySet()) { - if (e.getValue().equals(clazz)) { - return e.getKey(); - } - } - throw new IllegalArgumentException("Unknown class: " + clazz.getName()); + return delegate.idFromValue(value); } @Override public String idFromValueAndType(Object value, Class suggestedType) { - ensureTypeMap(); - return idFromValue(value); + return delegate.idFromValueAndType(value, suggestedType); } @Override - public JavaType typeFromId(DatabindContext context, String id) + public String getDescForKnownTypeIds() { - ensureTypeMap(); - if (id == null) { - // Missing type, caller decides default - return null; - } - Class clazz = typeMap.get(id); - if (clazz == null) { - throw new IAE("Unknown type[%s]", id); - } - return context.constructType(clazz); + return delegate.getDescForKnownTypeIds(); } @Override diff --git a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java index 695d6f2f6e31..d71b99bdf641 100644 --- a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java +++ b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java @@ -20,7 +20,7 @@ package org.apache.druid.data.input.impl; import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.druid.java.util.common.IAE; +import com.fasterxml.jackson.databind.exc.InvalidTypeIdException; import org.junit.Assert; import org.junit.Test; @@ -52,8 +52,12 @@ public void testStringDimensionSchemaSerde() throws Exception public void testDeserializeStrictTypeId() throws Exception { final String invalidType = "{\"type\":\"invalid\",\"name\":\"foo\",\"multiValueHandling\":\"ARRAY\",\"createBitmapIndex\":false}"; - IAE e = Assert.assertThrows(IAE.class, () -> OBJECT_MAPPER.readValue(invalidType, DimensionSchema.class)); - Assert.assertEquals("Unknown type[invalid]", e.getMessage()); + InvalidTypeIdException e = Assert.assertThrows( + InvalidTypeIdException.class, + () -> OBJECT_MAPPER.readValue(invalidType, DimensionSchema.class) + ); + Assert.assertTrue(e.getMessage().contains( + "Could not resolve type id 'invalid' as a subtype of `org.apache.druid.data.input.impl.DimensionSchema`: known type ids = [auto, double, float, json, long, spatial, string]")); } @Test From f167cd4681d65ae19d07265013583cdfa2809f6a Mon Sep 17 00:00:00 2001 From: cecemei Date: Fri, 3 Oct 2025 14:14:32 -0700 Subject: [PATCH 5/6] unnecessary-null-check --- .../java/org/apache/druid/jackson/StrictTypeIdResolver.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java index 6895d96782d1..e4b4d54ebf0d 100644 --- a/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java +++ b/processing/src/main/java/org/apache/druid/jackson/StrictTypeIdResolver.java @@ -86,11 +86,6 @@ protected TypeIdResolver idResolver( @Override public JavaType typeFromId(DatabindContext context, String id) throws JsonProcessingException { - if (id == null) { - // fallback to default implementation if configured - return null; - } - JavaType type = delegate.typeFromId(context, id); if (type == null) { // in TypeNameIdResolver, it'd fall back to defaultImpl if configured, but we want to error out instead From 71a2897257586da42a046d7a93fbc285890a5942 Mon Sep 17 00:00:00 2001 From: Cece Mei Date: Mon, 6 Oct 2025 16:14:46 -0700 Subject: [PATCH 6/6] Update processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../org/apache/druid/data/input/impl/DimensionSchemaTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java index d71b99bdf641..b7315fe8f3a8 100644 --- a/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java +++ b/processing/src/test/java/org/apache/druid/data/input/impl/DimensionSchemaTest.java @@ -56,8 +56,8 @@ public void testDeserializeStrictTypeId() throws Exception InvalidTypeIdException.class, () -> OBJECT_MAPPER.readValue(invalidType, DimensionSchema.class) ); - Assert.assertTrue(e.getMessage().contains( - "Could not resolve type id 'invalid' as a subtype of `org.apache.druid.data.input.impl.DimensionSchema`: known type ids = [auto, double, float, json, long, spatial, string]")); + Assert.assertTrue(e.getMessage().contains("Could not resolve type id")); + Assert.assertTrue(e.getMessage().contains("invalid")); } @Test