You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2022/12/05 10:21:14 UTC
[kylin] 22/22: KYLIN-5326 Fix request parameter json deserializer
This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 21a6b9f7f611d8d4a23a8003abbcc139e687c394
Author: Jiale He <35...@users.noreply.github.com>
AuthorDate: Wed Oct 19 08:40:18 2022 +0800
KYLIN-5326 Fix request parameter json deserializer
---
.../common/util/ArgsTypeJsonDeserializer.java | 35 ++++-
.../common/util/ArgsTypeJsonDeserializerTest.java | 156 +++++++++++++++++++++
.../controller/open/OpenTableControllerTest.java | 50 ++++---
3 files changed, 214 insertions(+), 27 deletions(-)
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializer.java b/src/core-common/src/main/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializer.java
index e9509b1314..6b47bc12f1 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializer.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializer.java
@@ -23,11 +23,13 @@ import static org.apache.kylin.common.exception.code.ErrorCodeServer.ARGS_TYPE_C
import java.io.IOException;
import java.util.List;
+import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.exception.KylinException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.google.common.collect.Lists;
public class ArgsTypeJsonDeserializer {
@@ -36,12 +38,27 @@ public class ArgsTypeJsonDeserializer {
}
public static class BooleanJsonDeserializer extends JsonDeserializer<Boolean> {
+
+ private final List<String> boolList = Lists.newArrayList("true", "false", "TRUE", "FALSE", "null");
+
+ @Override
+ public Boolean getNullValue(DeserializationContext ctxt) {
+ return Boolean.FALSE;
+ }
+
@Override
public Boolean deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
try {
+ String text = p.getText();
+ if (StringUtils.isEmpty(text)) {
+ return Boolean.FALSE;
+ }
+ if (boolList.contains(text)) {
+ return Boolean.parseBoolean(text);
+ }
return p.getBooleanValue();
} catch (Exception e) {
- throw new KylinException(ARGS_TYPE_CHECK, p.getText(), "Boolean");
+ throw new KylinException(ARGS_TYPE_CHECK, e, p.getText(), "Boolean");
}
}
}
@@ -52,18 +69,28 @@ public class ArgsTypeJsonDeserializer {
try {
return p.readValueAs(List.class);
} catch (Exception e) {
- throw new KylinException(ARGS_TYPE_CHECK, p.getText(), "List");
+ throw new KylinException(ARGS_TYPE_CHECK, e, p.getText(), "List");
}
}
}
public static class IntegerJsonDeserializer extends JsonDeserializer<Integer> {
+
+ @Override
+ public Integer getNullValue(DeserializationContext ctxt) {
+ return 0;
+ }
+
@Override
public Integer deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
try {
- return p.getIntValue();
+ String text = p.getText();
+ if (StringUtils.isEmpty(text) || StringUtils.equals("null", text)) {
+ return 0;
+ }
+ return Integer.parseInt(text);
} catch (Exception e) {
- throw new KylinException(ARGS_TYPE_CHECK, p.getText(), "Integer");
+ throw new KylinException(ARGS_TYPE_CHECK, e, p.getText(), "Integer");
}
}
}
diff --git a/src/core-common/src/test/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializerTest.java b/src/core-common/src/test/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializerTest.java
new file mode 100644
index 0000000000..dd412883c5
--- /dev/null
+++ b/src/core-common/src/test/java/org/apache/kylin/common/util/ArgsTypeJsonDeserializerTest.java
@@ -0,0 +1,156 @@
+/*
+ * 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.kylin.common.util;
+
+import static org.apache.kylin.common.exception.code.ErrorCodeServer.ARGS_TYPE_CHECK;
+
+import java.util.HashMap;
+
+import org.apache.kylin.common.exception.KylinException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.google.common.collect.Maps;
+
+import lombok.Data;
+
+class ArgsTypeJsonDeserializerTest {
+
+ private static final ObjectMapper mapper = new ObjectMapper();
+
+ @Test
+ void testDeserialize() throws Exception {
+ {
+ // "null" -> false
+ // "null" -> 0
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", "null");
+ map.put("int_value", "null");
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(false, request.getBolValue());
+ Assertions.assertEquals(0, request.getIntValue());
+ }
+
+ {
+ // "" -> false
+ // "" -> 0
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", "");
+ map.put("int_value", "");
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(false, request.getBolValue());
+ Assertions.assertEquals(0, request.getIntValue());
+ }
+
+ {
+ // null -> false
+ // null -> 0
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", null);
+ map.put("int_value", null);
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(false, request.getBolValue());
+ Assertions.assertEquals(0, request.getIntValue());
+ }
+
+ {
+ // null -> true
+ // null -> 1
+ HashMap<Object, Object> map = Maps.newHashMap();
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(true, request.getBolValue());
+ Assertions.assertEquals(1, request.getIntValue());
+ }
+
+ {
+ // "true" -> true
+ // "99" -> 99
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", "true");
+ map.put("int_value", "99");
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(true, request.getBolValue());
+ Assertions.assertEquals(99, request.getIntValue());
+ }
+
+ {
+ // "TRUE" -> true
+ // "99" -> 99
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", "true");
+ map.put("int_value", "99");
+ String jsonStr = mapper.writeValueAsString(map);
+ MockRequest request = mapper.readValue(jsonStr, MockRequest.class);
+ Assertions.assertEquals(true, request.getBolValue());
+ Assertions.assertEquals(99, request.getIntValue());
+ }
+
+ {
+ // "abd" -> exception
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("bol_value", "abc");
+ String jsonStr = mapper.writeValueAsString(map);
+ try {
+ mapper.readValue(jsonStr, MockRequest.class);
+ } catch (Exception e) {
+ Assertions.assertTrue(e instanceof JsonMappingException);
+ Assertions.assertTrue(e.getCause() instanceof KylinException);
+ KylinException kylinException = (KylinException) e.getCause();
+ Assertions.assertEquals(ARGS_TYPE_CHECK.getErrorCode().getCode(),
+ kylinException.getErrorCode().getCodeString());
+ }
+ }
+
+ {
+ // "abc" -> exception
+ HashMap<Object, Object> map = Maps.newHashMap();
+ map.put("int_value", "abc");
+ String jsonStr = mapper.writeValueAsString(map);
+ try {
+ mapper.readValue(jsonStr, MockRequest.class);
+ } catch (Exception e) {
+ Assertions.assertTrue(e instanceof JsonMappingException);
+ Assertions.assertTrue(e.getCause() instanceof KylinException);
+ KylinException kylinException = (KylinException) e.getCause();
+ Assertions.assertEquals(ARGS_TYPE_CHECK.getErrorCode().getCode(),
+ kylinException.getErrorCode().getCodeString());
+ }
+ }
+ }
+
+ @Data
+ static class MockRequest {
+ @JsonDeserialize(using = ArgsTypeJsonDeserializer.BooleanJsonDeserializer.class)
+ @JsonProperty("bol_value")
+ private Boolean bolValue = true;
+
+ @JsonDeserialize(using = ArgsTypeJsonDeserializer.IntegerJsonDeserializer.class)
+ @JsonProperty("int_value")
+ private Integer intValue = 1;
+ }
+}
diff --git a/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/open/OpenTableControllerTest.java b/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/open/OpenTableControllerTest.java
index 9cafc5e6c5..1ba31ca693 100644
--- a/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/open/OpenTableControllerTest.java
+++ b/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/open/OpenTableControllerTest.java
@@ -148,6 +148,7 @@ public class OpenTableControllerTest extends NLocalFileMetadataTestCase {
tableLoadRequest.setTables(new String[] { "hh.kk" });
tableLoadRequest.setNeedSampling(false);
tableLoadRequest.setProject("default");
+ tableLoadRequest.setSamplingRows(0);
Mockito.doNothing().when(openTableController).updateDataSourceType("default", 9);
Mockito.doAnswer(x -> null).when(nTableController).loadTables(tableLoadRequest);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables") //
@@ -203,27 +204,27 @@ public class OpenTableControllerTest extends NLocalFileMetadataTestCase {
Mockito.doNothing().when(openTableController).updateDataSourceType("default", 9);
Mockito.doAnswer(x -> null).when(nTableController).loadAWSTablesCompatibleCrossAccount(tableLoadRequest);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables/compatibility/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isOk());
Mockito.verify(openTableController).loadAWSTablesCompatibleCrossAccount(tableLoadRequest);
tableLoadRequest.setNeedSampling(true);
tableLoadRequest.setSamplingRows(10000);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables/compatibility/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isOk());
Mockito.verify(openTableController).loadAWSTablesCompatibleCrossAccount(tableLoadRequest);
tableLoadRequest.setNeedSampling(true);
tableLoadRequest.setSamplingRows(1000);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables/compatibility/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(tableLoadRequest)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isInternalServerError());
Mockito.verify(openTableController).loadAWSTablesCompatibleCrossAccount(tableLoadRequest);
@@ -248,11 +249,12 @@ public class OpenTableControllerTest extends NLocalFileMetadataTestCase {
request.setTables(tableExtInfoList);
mockMvc.perform(MockMvcRequestBuilders.put("/api/tables/ext/prop/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(request)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(request)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isOk());
- Mockito.verify(openTableController).updateLoadedAWSTableExtProp(Mockito.any(UpdateAWSTableExtDescRequest.class));
+ Mockito.verify(openTableController)
+ .updateLoadedAWSTableExtProp(Mockito.any(UpdateAWSTableExtDescRequest.class));
}
@Test
@@ -349,12 +351,13 @@ public class OpenTableControllerTest extends NLocalFileMetadataTestCase {
request.setNeedSampling(false);
request.setS3TableExtInfo(s3TableExtInfo);
- Mockito.doReturn(new Pair<String, List<String>>()).when(tableService).reloadAWSTableCompatibleCrossAccount(request.getProject(),
- request.getS3TableExtInfo(), request.getNeedSampling(), 0, false, ExecutablePO.DEFAULT_PRIORITY, null);
+ Mockito.doReturn(new Pair<String, List<String>>()).when(tableService).reloadAWSTableCompatibleCrossAccount(
+ request.getProject(), request.getS3TableExtInfo(), request.getNeedSampling(), 0, false,
+ ExecutablePO.DEFAULT_PRIORITY, null);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables/reload/compatibility/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(request)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(request)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isOk());
Mockito.verify(openTableController).reloadAWSTablesCompatibleCrossAccount(request);
@@ -364,12 +367,13 @@ public class OpenTableControllerTest extends NLocalFileMetadataTestCase {
request2.setS3TableExtInfo(s3TableExtInfo);
request2.setNeedSampling(true);
request2.setSamplingRows(10000);
- Mockito.doReturn(new Pair<String, List<String>>()).when(tableService).reloadAWSTableCompatibleCrossAccount(request2.getProject(),
- request2.getS3TableExtInfo(), request2.getNeedSampling(), request2.getSamplingRows(), false, ExecutablePO.DEFAULT_PRIORITY, null);
+ Mockito.doReturn(new Pair<String, List<String>>()).when(tableService).reloadAWSTableCompatibleCrossAccount(
+ request2.getProject(), request2.getS3TableExtInfo(), request2.getNeedSampling(),
+ request2.getSamplingRows(), false, ExecutablePO.DEFAULT_PRIORITY, null);
mockMvc.perform(MockMvcRequestBuilders.post("/api/tables/reload/compatibility/aws") //
- .contentType(MediaType.APPLICATION_JSON) //
- .content(JsonUtil.writeValueAsString(request2)) //
- .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
+ .contentType(MediaType.APPLICATION_JSON) //
+ .content(JsonUtil.writeValueAsString(request2)) //
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON))) //
.andExpect(MockMvcResultMatchers.status().isOk());
Mockito.verify(openTableController).reloadAWSTablesCompatibleCrossAccount(request2);
}