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 <35652389+jial...@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); }