This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 59a0c10c47 Add remedial information in error message when type is
unknown (#12612)
59a0c10c47 is described below
commit 59a0c10c47af27b0186e4134b9f5461e9516f399
Author: Abhishek Agarwal <[email protected]>
AuthorDate: Tue Jun 7 20:22:45 2022 +0530
Add remedial information in error message when type is unknown (#12612)
Often users are submitting queries, and ingestion specs that work only if
the relevant extension is not loaded. However, the error is too technical for
the users and doesn't suggest them to check for missing extensions. This PR
modifies the error message so users can at least check their settings before
assuming that the error is because of a bug.
---
.../druid/benchmark/query/GroupByBenchmark.java | 2 +-
.../materializedview/DatasourceOptimizerTest.java | 2 +-
.../parallel/distribution/StringSketchTest.java | 3 +-
.../apache/druid/jackson/DefaultObjectMapper.java | 60 ++++++++++++++++-
.../org/apache/druid/jackson/JacksonModule.java | 25 ++++++--
.../druid/jackson/DefaultObjectMapperTest.java | 35 ++++++++++
.../apache/druid/jackson/JacksonModuleTest.java | 75 ++++++++++++++++++++++
.../apache/druid/client/BrokerServerViewTest.java | 2 +-
.../client/CachingClusteredClientTestUtils.java | 2 +-
9 files changed, 193 insertions(+), 13 deletions(-)
diff --git
a/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
b/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
index 6310dd499d..0fdffa8b87 100644
---
a/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
+++
b/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
@@ -784,7 +784,7 @@ public class GroupByBenchmark
QueryRunner<ResultRow> theRunner = new FinalizeResultsQueryRunner<>(
toolChest.mergeResults(
new SerializingQueryRunner<>(
- new DefaultObjectMapper(new SmileFactory()),
+ new DefaultObjectMapper(new SmileFactory(), null),
ResultRow.class,
toolChest.mergeResults(
factory.mergeRunners(state.executorService,
makeMultiRunners(state))
diff --git
a/extensions-contrib/materialized-view-selection/src/test/java/org/apache/druid/query/materializedview/DatasourceOptimizerTest.java
b/extensions-contrib/materialized-view-selection/src/test/java/org/apache/druid/query/materializedview/DatasourceOptimizerTest.java
index 4f0d387c1a..520aa3193b 100644
---
a/extensions-contrib/materialized-view-selection/src/test/java/org/apache/druid/query/materializedview/DatasourceOptimizerTest.java
+++
b/extensions-contrib/materialized-view-selection/src/test/java/org/apache/druid/query/materializedview/DatasourceOptimizerTest.java
@@ -308,7 +308,7 @@ public class DatasourceOptimizerTest extends CuratorTestBase
final SmileFactory smileFactory = new SmileFactory();
smileFactory.configure(SmileGenerator.Feature.ENCODE_BINARY_AS_7BIT,
false);
smileFactory.delegateToTextual(true);
- final ObjectMapper retVal = new DefaultObjectMapper(smileFactory);
+ final ObjectMapper retVal = new DefaultObjectMapper(smileFactory,
"broker");
retVal.getFactory().setCodec(retVal);
return retVal;
}
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/distribution/StringSketchTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/distribution/StringSketchTest.java
index 60d7b543ec..c00dede460 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/distribution/StringSketchTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/distribution/StringSketchTest.java
@@ -40,6 +40,7 @@ import org.junit.runner.RunWith;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Properties;
import java.util.StringJoiner;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
@@ -62,7 +63,7 @@ public class StringSketchTest
public static class SerializationDeserializationTest
{
- private static final ObjectMapper OBJECT_MAPPER = new
JacksonModule().smileMapper();
+ private static final ObjectMapper OBJECT_MAPPER = new
JacksonModule().smileMapper(new Properties());
@Test
public void serializesDeserializes()
diff --git
a/processing/src/main/java/org/apache/druid/jackson/DefaultObjectMapper.java
b/processing/src/main/java/org/apache/druid/jackson/DefaultObjectMapper.java
index 71cf30ff73..ac9387b11e 100644
--- a/processing/src/main/java/org/apache/druid/jackson/DefaultObjectMapper.java
+++ b/processing/src/main/java/org/apache/druid/jackson/DefaultObjectMapper.java
@@ -20,11 +20,23 @@
package org.apache.druid.jackson;
import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler;
+import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
+import com.fasterxml.jackson.databind.jsontype.TypeIdResolver;
+import com.fasterxml.jackson.databind.util.ClassUtil;
import com.fasterxml.jackson.datatype.guava.GuavaModule;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.druid.java.util.common.StringUtils;
+
+import javax.annotation.Nullable;
+
+import java.io.IOException;
/**
*/
@@ -32,7 +44,12 @@ public class DefaultObjectMapper extends ObjectMapper
{
public DefaultObjectMapper()
{
- this((JsonFactory) null);
+ this((JsonFactory) null, null);
+ }
+
+ public DefaultObjectMapper(String serviceName)
+ {
+ this((JsonFactory) null, serviceName);
}
public DefaultObjectMapper(DefaultObjectMapper mapper)
@@ -40,7 +57,7 @@ public class DefaultObjectMapper extends ObjectMapper
super(mapper);
}
- public DefaultObjectMapper(JsonFactory factory)
+ public DefaultObjectMapper(JsonFactory factory, @Nullable String serviceName)
{
super(factory);
registerModule(new DruidDefaultSerializersModule());
@@ -61,6 +78,8 @@ public class DefaultObjectMapper extends ObjectMapper
configure(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false);
configure(SerializationFeature.INDENT_OUTPUT, false);
configure(SerializationFeature.FLUSH_AFTER_WRITE_VALUE, false);
+
+ addHandler(new DefaultDeserializationProblemHandler(serviceName));
}
@Override
@@ -68,4 +87,41 @@ public class DefaultObjectMapper extends ObjectMapper
{
return new DefaultObjectMapper(this);
}
+
+ /**
+ * A custom implementation of {@link #DeserializationProblemHandler} to add
custom error message so
+ * that users know how to troubleshoot unknown type ids.
+ */
+ static class DefaultDeserializationProblemHandler extends
DeserializationProblemHandler
+ {
+ @Nullable
+ private final String serviceName;
+
+ public DefaultDeserializationProblemHandler(@Nullable String serviceName)
+ {
+ this.serviceName = serviceName;
+ }
+
+ @Nullable
+ @VisibleForTesting
+ String getServiceName()
+ {
+ return serviceName;
+ }
+
+ @Override
+ public JavaType handleUnknownTypeId(DeserializationContext ctxt,
+ JavaType baseType, String subTypeId,
TypeIdResolver idResolver,
+ String failureMsg)
+ throws IOException
+ {
+ String serviceMsg = (serviceName == null) ? "" : StringUtils.format(" on
'%s' service", serviceName);
+ String msg = StringUtils.format("Please make sure to load all the
necessary extensions and jars " +
+ "with type '%s'%s. " +
+ "Could not resolve type id '%s' as a subtype of %s",
+ subTypeId, serviceMsg, subTypeId,
ClassUtil.getTypeDescription(baseType));
+ String extraFailureMsg = (failureMsg == null) ? msg : msg + " " +
failureMsg;
+ throw InvalidTypeIdException.from(ctxt.getParser(), extraFailureMsg,
baseType, subTypeId);
+ }
+ }
}
diff --git
a/processing/src/main/java/org/apache/druid/jackson/JacksonModule.java
b/processing/src/main/java/org/apache/druid/jackson/JacksonModule.java
index a7c947d365..fa4ec61e22 100644
--- a/processing/src/main/java/org/apache/druid/jackson/JacksonModule.java
+++ b/processing/src/main/java/org/apache/druid/jackson/JacksonModule.java
@@ -32,6 +32,10 @@ import org.apache.druid.guice.annotations.Json;
import org.apache.druid.guice.annotations.JsonNonNull;
import org.apache.druid.guice.annotations.Smile;
+import javax.annotation.Nullable;
+
+import java.util.Properties;
+
/**
*/
public class JacksonModule implements Module
@@ -43,28 +47,37 @@ public class JacksonModule implements Module
}
@Provides @LazySingleton @Json
- public ObjectMapper jsonMapper()
+ public ObjectMapper jsonMapper(Properties props)
{
- return new DefaultObjectMapper();
+ return new DefaultObjectMapper(getServiceName(props));
}
/**
* Provides ObjectMapper that suppress serializing properties with null
values
*/
@Provides @LazySingleton @JsonNonNull
- public ObjectMapper jsonMapperOnlyNonNullValue()
+ public ObjectMapper jsonMapperOnlyNonNullValue(Properties props)
{
- return new
DefaultObjectMapper().setSerializationInclusion(JsonInclude.Include.NON_NULL);
+ return new
DefaultObjectMapper(getServiceName(props)).setSerializationInclusion(JsonInclude.Include.NON_NULL);
}
@Provides @LazySingleton @Smile
- public ObjectMapper smileMapper()
+ public ObjectMapper smileMapper(Properties props)
{
final SmileFactory smileFactory = new SmileFactory();
smileFactory.configure(SmileGenerator.Feature.ENCODE_BINARY_AS_7BIT,
false);
smileFactory.delegateToTextual(true);
- final ObjectMapper retVal = new DefaultObjectMapper(smileFactory);
+ final ObjectMapper retVal = new DefaultObjectMapper(smileFactory,
getServiceName(props));
retVal.getFactory().setCodec(retVal);
return retVal;
}
+
+ @Nullable
+ private String getServiceName(Properties properties)
+ {
+ if (null == properties) {
+ return null;
+ }
+ return properties.getProperty("druid.service");
+ }
}
diff --git
a/processing/src/test/java/org/apache/druid/jackson/DefaultObjectMapperTest.java
b/processing/src/test/java/org/apache/druid/jackson/DefaultObjectMapperTest.java
index ac20e86867..989d137770 100644
---
a/processing/src/test/java/org/apache/druid/jackson/DefaultObjectMapperTest.java
+++
b/processing/src/test/java/org/apache/druid/jackson/DefaultObjectMapperTest.java
@@ -19,12 +19,15 @@
package org.apache.druid.jackson;
+import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.guava.Sequences;
import org.apache.druid.java.util.common.guava.Yielders;
+import org.apache.druid.query.Query;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.junit.Assert;
@@ -67,4 +70,36 @@ public class DefaultObjectMapperTest
mapper.writeValueAsString(Yielders.each(sequence))
);
}
+
+ @Test
+ public void testUnknownType() throws JsonProcessingException
+ {
+ DefaultObjectMapper objectMapper = new DefaultObjectMapper("testService");
+ try {
+
objectMapper.readValue("{\"queryType\":\"random\",\"name\":\"does-not-matter\"}",
Query.class);
+ }
+ catch (InvalidTypeIdException e) {
+ String message = e.getMessage();
+ Assert.assertTrue(message, message.startsWith("Please make sure to load
all the necessary extensions and " +
+ "jars with type 'random' on 'testService' service."));
+ return;
+ }
+ Assert.fail("We expect InvalidTypeIdException to be thrown");
+ }
+
+ @Test
+ public void testUnknownTypeWithUnknownService() throws
JsonProcessingException
+ {
+ DefaultObjectMapper objectMapper = new DefaultObjectMapper((String) null);
+ try {
+
objectMapper.readValue("{\"queryType\":\"random\",\"name\":\"does-not-matter\"}",
Query.class);
+ }
+ catch (InvalidTypeIdException e) {
+ String message = e.getMessage();
+ Assert.assertTrue(message, message.startsWith("Please make sure to load
all the necessary extensions and " +
+ "jars with type 'random'."));
+ return;
+ }
+ Assert.fail("We expect InvalidTypeIdException to be thrown");
+ }
}
diff --git
a/processing/src/test/java/org/apache/druid/jackson/JacksonModuleTest.java
b/processing/src/test/java/org/apache/druid/jackson/JacksonModuleTest.java
new file mode 100644
index 0000000000..507f10bcf5
--- /dev/null
+++ b/processing/src/test/java/org/apache/druid/jackson/JacksonModuleTest.java
@@ -0,0 +1,75 @@
+/*
+ * 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.databind.deser.DeserializationProblemHandler;
+import com.fasterxml.jackson.databind.util.LinkedNode;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Properties;
+
+public class JacksonModuleTest
+{
+ @Test
+ public void testNullServiceNameInProperties()
+ {
+ JacksonModule module = new JacksonModule();
+ verifyUnknownServiceName((DefaultObjectMapper) module.jsonMapper(new
Properties()));
+ verifyUnknownServiceName((DefaultObjectMapper) module.smileMapper(new
Properties()));
+ verifyUnknownServiceName((DefaultObjectMapper)
module.jsonMapperOnlyNonNullValue(new Properties()));
+ }
+
+ @Test
+ public void testNullProperties()
+ {
+ JacksonModule module = new JacksonModule();
+ verifyUnknownServiceName((DefaultObjectMapper) module.jsonMapper(null));
+ verifyUnknownServiceName((DefaultObjectMapper) module.smileMapper(null));
+ verifyUnknownServiceName((DefaultObjectMapper)
module.jsonMapperOnlyNonNullValue(null));
+ }
+
+ @Test
+ public void testServiceNameInProperties()
+ {
+ JacksonModule module = new JacksonModule();
+ Properties properties = new Properties();
+ properties.setProperty("druid.service", "myService_1");
+ verifyServiceName((DefaultObjectMapper) module.jsonMapper(properties),
"myService_1");
+ properties.setProperty("druid.service", "myService_2");
+ verifyServiceName((DefaultObjectMapper) module.smileMapper(properties),
"myService_2");
+ properties.setProperty("druid.service", "myService_3");
+ verifyServiceName((DefaultObjectMapper)
module.jsonMapperOnlyNonNullValue(properties), "myService_3");
+ }
+
+ private void verifyUnknownServiceName(DefaultObjectMapper objectMapper)
+ {
+ verifyServiceName(objectMapper, null);
+ }
+
+ private void verifyServiceName(DefaultObjectMapper objectMapper, String
expectedServiceName)
+ {
+ LinkedNode<DeserializationProblemHandler> handlers =
objectMapper.getDeserializationConfig().getProblemHandlers();
+ Assert.assertNull(handlers.next());
+ DefaultObjectMapper.DefaultDeserializationProblemHandler handler =
+ (DefaultObjectMapper.DefaultDeserializationProblemHandler)
handlers.value();
+ Assert.assertEquals(expectedServiceName, handler.getServiceName());
+ }
+}
diff --git
a/server/src/test/java/org/apache/druid/client/BrokerServerViewTest.java
b/server/src/test/java/org/apache/druid/client/BrokerServerViewTest.java
index 5855c9d2e7..bbd9a5844f 100644
--- a/server/src/test/java/org/apache/druid/client/BrokerServerViewTest.java
+++ b/server/src/test/java/org/apache/druid/client/BrokerServerViewTest.java
@@ -698,7 +698,7 @@ public class BrokerServerViewTest extends CuratorTestBase
final SmileFactory smileFactory = new SmileFactory();
smileFactory.configure(SmileGenerator.Feature.ENCODE_BINARY_AS_7BIT,
false);
smileFactory.delegateToTextual(true);
- final ObjectMapper retVal = new DefaultObjectMapper(smileFactory);
+ final ObjectMapper retVal = new DefaultObjectMapper(smileFactory,
"broker");
retVal.getFactory().setCodec(retVal);
return retVal;
}
diff --git
a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java
b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java
index f5fdf24ebe..90e9bc209a 100644
---
a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java
+++
b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java
@@ -87,7 +87,7 @@ public final class CachingClusteredClientTestUtils
public static ObjectMapper createObjectMapper()
{
final SmileFactory factory = new SmileFactory();
- final ObjectMapper objectMapper = new DefaultObjectMapper(factory);
+ final ObjectMapper objectMapper = new DefaultObjectMapper(factory,
"broker");
factory.setCodec(objectMapper);
return objectMapper;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
