[
https://issues.apache.org/jira/browse/SCB-933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16629610#comment-16629610
]
ASF GitHub Bot commented on SCB-933:
------------------------------------
liubao68 closed pull request #923: [SCB-933]Revert changes to RestObjectMapper
that fail on primitive types not present
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/923
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestObjectMapper.java
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestObjectMapper.java
index 327175b22..8f20acbfc 100644
---
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestObjectMapper.java
+++
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestObjectMapper.java
@@ -61,12 +61,12 @@ public StringBuffer format(Date date, StringBuffer
toAppendTo, FieldPosition fie
});
getFactory().disable(Feature.AUTO_CLOSE_SOURCE);
+ // Enable features that can tolerance errors and not enable those make
more constraints for compatible reasons.
+ // Developers can use validation api to do more checks.
disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
enable(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS);
enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);
- // If required=true, need to fail, while required=false, default values is
given.
- enable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES);
SimpleModule module = new SimpleModule();
// custom types
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
index 7b22f9fa6..3c68b97b1 100644
---
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
+++
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
@@ -20,6 +20,7 @@
import java.util.Arrays;
import javax.servlet.http.HttpServletRequest;
+import javax.validation.constraints.Min;
import javax.ws.rs.core.MediaType;
import org.apache.servicecomb.demo.controller.Person;
@@ -37,7 +38,7 @@
@RequestMapping(path = "/springmvc/controller", produces =
MediaType.APPLICATION_JSON)
public class ControllerImpl {
@GetMapping(path = "/add")
- public int add(@RequestParam("a") int a, @RequestParam("b") int b) {
+ public int add(@Min(1) @RequestParam("a") int a, @Min(1) @RequestParam("b")
int b) {
return a + b;
}
diff --git
a/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestBaseModel.java
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestBaseModel.java
new file mode 100644
index 000000000..30e163142
--- /dev/null
+++
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestBaseModel.java
@@ -0,0 +1,49 @@
+/*
+ * 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.servicecomb.it.schema;
+
+public class RequestBaseModel {
+ private int type;
+
+ private Integer integerType;
+
+ private String message;
+
+ public int getType() {
+ return type;
+ }
+
+ public void setType(int type) {
+ this.type = type;
+ }
+
+ public Integer getIntegerType() {
+ return integerType;
+ }
+
+ public void setIntegerType(Integer integerType) {
+ this.integerType = integerType;
+ }
+
+ public String getMessage() {
+ return message;
+ }
+
+ public void setMessage(String message) {
+ this.message = message;
+ }
+}
diff --git
a/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestModel.java
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestModel.java
new file mode 100644
index 000000000..945a87f81
--- /dev/null
+++
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/RequestModel.java
@@ -0,0 +1,49 @@
+/*
+ * 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.servicecomb.it.schema;
+
+public class RequestModel extends RequestBaseModel {
+ private int catalog;
+
+ private Integer integerCatalog;
+
+ private String extendedMessage;
+
+ public int getCatalog() {
+ return catalog;
+ }
+
+ public void setCatalog(int catalog) {
+ this.catalog = catalog;
+ }
+
+ public Integer getIntegerCatalog() {
+ return integerCatalog;
+ }
+
+ public void setIntegerCatalog(Integer integerCatalog) {
+ this.integerCatalog = integerCatalog;
+ }
+
+ public String getExtendedMessage() {
+ return extendedMessage;
+ }
+
+ public void setExtendedMessage(String extendedMessage) {
+ this.extendedMessage = extendedMessage;
+ }
+}
diff --git
a/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseBaseModel.java
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseBaseModel.java
new file mode 100644
index 000000000..e045ef7c3
--- /dev/null
+++
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseBaseModel.java
@@ -0,0 +1,49 @@
+/*
+ * 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.servicecomb.it.schema;
+
+public class ResponseBaseModel {
+ private int type;
+
+ private Integer integerType;
+
+ private String message;
+
+ public int getType() {
+ return type;
+ }
+
+ public void setType(int type) {
+ this.type = type;
+ }
+
+ public Integer getIntegerType() {
+ return integerType;
+ }
+
+ public void setIntegerType(Integer integerType) {
+ this.integerType = integerType;
+ }
+
+ public String getMessage() {
+ return message;
+ }
+
+ public void setMessage(String message) {
+ this.message = message;
+ }
+}
diff --git
a/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseModel.java
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseModel.java
new file mode 100644
index 000000000..e816aa604
--- /dev/null
+++
b/integration-tests/it-common/src/main/java/org/apache/servicecomb/it/schema/ResponseModel.java
@@ -0,0 +1,49 @@
+/*
+ * 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.servicecomb.it.schema;
+
+public class ResponseModel extends ResponseBaseModel {
+ private int catalog;
+
+ private Integer integerCatalog;
+
+ private String extendedMessage;
+
+ public int getCatalog() {
+ return catalog;
+ }
+
+ public void setCatalog(int catalog) {
+ this.catalog = catalog;
+ }
+
+ public Integer getIntegerCatalog() {
+ return integerCatalog;
+ }
+
+ public void setIntegerCatalog(Integer integerCatalog) {
+ this.integerCatalog = integerCatalog;
+ }
+
+ public String getExtendedMessage() {
+ return extendedMessage;
+ }
+
+ public void setExtendedMessage(String extendedMessage) {
+ this.extendedMessage = extendedMessage;
+ }
+}
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index 37e5df892..12a59f061 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -29,6 +29,7 @@
import org.apache.servicecomb.it.testcase.TestIgnoreMethod;
import org.apache.servicecomb.it.testcase.TestParamCodec;
import org.apache.servicecomb.it.testcase.TestParamCodecEdge;
+import org.apache.servicecomb.it.testcase.TestRequestBodySpringMvcSchema;
import org.apache.servicecomb.it.testcase.TestRestServerConfig;
import org.apache.servicecomb.it.testcase.TestRestServerConfigEdge;
import org.apache.servicecomb.it.testcase.TestTrace;
@@ -130,6 +131,8 @@ private static void testStandalone() throws Throwable {
ITJUnitUtils.runWithRest(TestRestServerConfig.class);
ITJUnitUtils.run(TestRestServerConfigEdge.class);
+ ITJUnitUtils.run(TestRequestBodySpringMvcSchema.class);
+
ITJUnitUtils.getParents().pop();
deploys.getBaseProducer().stop();
}
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultJsonValueJaxrsSchema.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultJsonValueJaxrsSchema.java
index 629d9a304..6594c9d35 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultJsonValueJaxrsSchema.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultJsonValueJaxrsSchema.java
@@ -51,7 +51,7 @@ public void invokeFromEdgeWithQuery() {
Assert.assertEquals(result, "expected:3:3");
result = client.getForObject("/queryInput", String.class);
- Assert.assertEquals(result, "expected:0:null");
+ Assert.assertEquals(result, "expected:0:0");
result = client.getForObject("/queryInput?size=", String.class);
Assert.assertEquals(result, "expected:0:null");
@@ -67,7 +67,7 @@ public void invokeFromEdgeWithRawJson() {
Map result =
client.postForObject("/jsonInput", entity, Map.class);
Assert.assertEquals(result.get("type"), 100);
- Assert.assertEquals(result.get("message"), "expected:30:0");
+ Assert.assertEquals(result.get("message"), "expected:null:null");
body = new HashMap<>();
body.put("type", 100);
@@ -76,7 +76,7 @@ public void invokeFromEdgeWithRawJson() {
result =
client.postForObject("/jsonInput", entity, Map.class);
Assert.assertEquals(result.get("type"), 100);
- Assert.assertEquals(result.get("message"), "expected:null:0");
+ Assert.assertEquals(result.get("message"), "expected:null:null");
body = new HashMap<>();
body.put("type", 200);
@@ -85,7 +85,7 @@ public void invokeFromEdgeWithRawJson() {
result =
client.postForObject("/jsonInput", entity, Map.class);
Assert.assertEquals(result.get("type"), 200);
- Assert.assertEquals(result.get("message"), "expected:-1:0");
+ Assert.assertEquals(result.get("message"), "expected:-1:null");
body = new HashMap<>();
body.put("type", 200);
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRequestBodySpringMvcSchema.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRequestBodySpringMvcSchema.java
new file mode 100644
index 000000000..f51322dcf
--- /dev/null
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRequestBodySpringMvcSchema.java
@@ -0,0 +1,103 @@
+/*
+ * 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.servicecomb.it.testcase;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.servicecomb.it.extend.engine.GateRestTemplate;
+import org.apache.servicecomb.it.junit.ITJUnitUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
+import org.springframework.web.client.RestTemplate;
+
+public class TestRequestBodySpringMvcSchema {
+ private static RestTemplate edgeClient;
+
+ private static String producerName;
+
+ @Before
+ public void prepare() {
+ if (!ITJUnitUtils.getProducerName().equals(producerName)) {
+ producerName = ITJUnitUtils.getProducerName();
+ edgeClient = new GateRestTemplate("it-edge", producerName,
"requestBodySpringMvcSchema");
+ }
+ }
+
+ @Test
+ public void basicRequestResponse() {
+ basicRequestResponseImpl();
+ basicRequestResponseImpl();
+ basicRequestResponseImpl();
+ }
+
+ @Test
+ public void testDefaultForPremitive() {
+ testDefaultForPremitiveImpl();
+ testDefaultForPremitiveImpl();
+ testDefaultForPremitiveImpl();
+ }
+
+ private void testDefaultForPremitiveImpl() {
+ HttpHeaders headers = new HttpHeaders();
+ headers.setContentType(MediaType.APPLICATION_JSON);
+ Map<String, Object> request = new HashMap<>();
+ request.put("integerType", 100);
+ request.put("message", "hi");
+ request.put("catalog", 100);
+ request.put("extendedMessage", "hi");
+
+ HttpEntity<Map<String, Object>> entity = new HttpEntity<>(request,
headers);
+ Map result =
+ edgeClient.postForObject("/base", entity, Map.class);
+ Assert.assertEquals(result.size(), 6);
+ Assert.assertEquals(result.get("type"), 0);
+ Assert.assertEquals(result.get("integerType"), request.get("integerType"));
+ Assert.assertEquals(result.get("message"), request.get("message"));
+ Assert.assertEquals(result.get("catalog"), request.get("catalog"));
+ Assert.assertEquals(result.get("integerCatalog"), null);
+ Assert.assertEquals(result.get("extendedMessage"),
request.get("extendedMessage"));
+ }
+
+ private void basicRequestResponseImpl() {
+ HttpHeaders headers = new HttpHeaders();
+ headers.setContentType(MediaType.APPLICATION_JSON);
+ Map<String, Object> request = new HashMap<>();
+ request.put("type", 100);
+ request.put("integerType", 100);
+ request.put("message", "hi");
+ request.put("catalog", 100);
+ request.put("integerCatalog", 100);
+ request.put("extendedMessage", "hi");
+
+ HttpEntity<Map<String, Object>> entity = new HttpEntity<>(request,
headers);
+ Map result =
+ edgeClient.postForObject("/base", entity, Map.class);
+ Assert.assertEquals(result.size(), request.size());
+ Assert.assertEquals(result.get("type"), request.get("type"));
+ Assert.assertEquals(result.get("integerType"), request.get("integerType"));
+ Assert.assertEquals(result.get("message"), request.get("message"));
+ Assert.assertEquals(result.get("catalog"), request.get("catalog"));
+ Assert.assertEquals(result.get("integerCatalog"),
request.get("integerCatalog"));
+ Assert.assertEquals(result.get("extendedMessage"),
request.get("extendedMessage"));
+ }
+}
diff --git a/integration-tests/it-edge/src/main/resources/microservice.yaml
b/integration-tests/it-edge/src/main/resources/microservice.yaml
index 88703c043..df5296ad4 100644
--- a/integration-tests/it-edge/src/main/resources/microservice.yaml
+++ b/integration-tests/it-edge/src/main/resources/microservice.yaml
@@ -25,12 +25,13 @@ servicecomb:
default: loadbalance
service:
it-auth: loadbalance
- rest:
- parameter:
- decodeAsObject: true
- query:
- ignoreDefaultValue: true
- emptyAsNull: true
+# this is not the majority, not testing this feature by default
+# rest:
+# parameter:
+# decodeAsObject: true
+# query:
+# ignoreDefaultValue: true
+# emptyAsNull: true
operation:
it-producer:
defaultJsonValueJaxrs:
diff --git
a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/RequestBodySpringMvcSchema.java
b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/RequestBodySpringMvcSchema.java
new file mode 100644
index 000000000..6efdcbb78
--- /dev/null
+++
b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/RequestBodySpringMvcSchema.java
@@ -0,0 +1,38 @@
+/*
+ * 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.servicecomb.it.schema;
+
+import org.apache.servicecomb.provider.rest.common.RestSchema;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+
+@RestSchema(schemaId = "requestBodySpringMvcSchema")
+@RequestMapping(path = "/v1/requestBodySpringMvcSchema")
+public class RequestBodySpringMvcSchema {
+ @PostMapping(path = "base")
+ public ResponseModel base(@RequestBody RequestModel request) {
+ ResponseModel response = new ResponseModel();
+ response.setType(request.getType());
+ response.setIntegerType(request.getIntegerType());
+ response.setMessage(request.getMessage());
+ response.setCatalog(request.getCatalog());
+ response.setIntegerCatalog(request.getIntegerCatalog());
+ response.setExtendedMessage(request.getExtendedMessage());
+ return response;
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Revert changes to RestObjectMapper that fail on primitive types not present
> ---------------------------------------------------------------------------
>
> Key: SCB-933
> URL: https://issues.apache.org/jira/browse/SCB-933
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
> Affects Versions: java-chassis-1.0.0
> Reporter: liubao
> Assignee: liubao
> Priority: Major
>
> This modification will cause compatible issues.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
