This is an automated email from the ASF dual-hosted git repository. liubao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit 147802f8d14335cc103f0576f920e9363743b338 Author: weichao666 <[email protected]> AuthorDate: Wed Nov 28 16:09:00 2018 +0800 [SCB-925] delete containerType is null --- .../rest/codec/param/CookieProcessorCreator.java | 5 +- .../rest/codec/param/FormProcessorCreator.java | 12 +-- .../rest/codec/param/HeaderProcessorCreator.java | 17 ++--- .../rest/codec/param/QueryProcessorCreator.java | 9 +-- .../rest/codec/param/TestCookieProcessor.java | 28 ++++--- .../common/rest/codec/param/TestFormProcessor.java | 16 ++-- .../rest/codec/param/TestHeaderProcessor.java | 13 ++-- .../servicecomb/it/testcase/TestDefaultValue.java | 85 ++++++++-------------- .../it/schema/DefaultValueSpringmvcSchema.java | 22 ++++-- 9 files changed, 91 insertions(+), 116 deletions(-) diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/CookieProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/CookieProcessorCreator.java index 80f1231..90f3946 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/CookieProcessorCreator.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/CookieProcessorCreator.java @@ -43,7 +43,7 @@ public class CookieProcessorCreator implements ParamValueProcessorCreator { } @Override - public Object getValue(HttpServletRequest request) throws Exception { + public Object getValue(HttpServletRequest request) { Cookie[] cookies = request.getCookies(); Object value = null; if (cookies == null || cookies.length == 0) { @@ -54,6 +54,7 @@ public class CookieProcessorCreator implements ParamValueProcessorCreator { for (Cookie cookie : cookies) { if (paramPath.equals(cookie.getName())) { value = cookie.getValue(); + break; } } if (value == null) { @@ -62,7 +63,7 @@ public class CookieProcessorCreator implements ParamValueProcessorCreator { return convertValue(value, targetType); } - private Object checkRequiredAndDefaultValue() throws Exception { + private Object checkRequiredAndDefaultValue() { if (isRequired()) { throw new InvocationException(Status.BAD_REQUEST, "Parameter is required."); } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java index bbb70d2..04ea5bf 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java @@ -44,7 +44,7 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { } @Override - public Object getValue(HttpServletRequest request) throws Exception { + public Object getValue(HttpServletRequest request) { @SuppressWarnings("unchecked") Map<String, Object> forms = (Map<String, Object>) request.getAttribute(RestConst.FORM_PARAMETERS); if (forms != null && !forms.isEmpty()) { @@ -52,12 +52,8 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { } if (targetType.isContainerType()) { - Object values = request.getParameterValues(paramPath); - //Even if the paramPath does not exist, it won't be null at now, may be optimized in the future - if (values == null) { - values = checkRequiredAndDefaultValue(); - } - return convertValue(values, targetType); + //Even if the paramPath does not exist, it won't be null at now + return convertValue(request.getParameterValues(paramPath), targetType); } Object value = request.getParameter(paramPath); @@ -68,7 +64,7 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { return convertValue(value, targetType); } - private Object checkRequiredAndDefaultValue() throws Exception { + private Object checkRequiredAndDefaultValue() { if (isRequired()) { throw new InvocationException(Status.BAD_REQUEST, "Parameter is required."); } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/HeaderProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/HeaderProcessorCreator.java index 726e637..4d91261 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/HeaderProcessorCreator.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/HeaderProcessorCreator.java @@ -47,20 +47,15 @@ public class HeaderProcessorCreator implements ParamValueProcessorCreator { } @Override - public Object getValue(HttpServletRequest request) throws Exception { + public Object getValue(HttpServletRequest request) { Object value = null; if (targetType.isContainerType()) { - Enumeration<?> headerValues = request.getHeaders(paramPath); - //Even if the paramPath does not exist, it won't be null at now, may be optimized in the future + Enumeration<String> headerValues = request.getHeaders(paramPath); if (headerValues == null) { - Object obj = checkRequiredAndDefaultValue(); - if (obj instanceof Enumeration) { - headerValues = (Enumeration<?>) obj; - } - } - if (headerValues != null) { - value = Collections.list(headerValues); + //Even if the paramPath does not exist, headerValues won't be null at now + return null; } + value = Collections.list(headerValues); } else { value = request.getHeader(paramPath); if (value == null) { @@ -71,7 +66,7 @@ public class HeaderProcessorCreator implements ParamValueProcessorCreator { return convertValue(value, targetType); } - private Object checkRequiredAndDefaultValue() throws Exception { + private Object checkRequiredAndDefaultValue() { if (isRequired()) { throw new InvocationException(Status.BAD_REQUEST, "Parameter is required."); } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/QueryProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/QueryProcessorCreator.java index 2cff361..e226730 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/QueryProcessorCreator.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/QueryProcessorCreator.java @@ -57,15 +57,12 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { } @Override - public Object getValue(HttpServletRequest request) throws Exception { + public Object getValue(HttpServletRequest request) { Object value = null; if (targetType.isContainerType() && SwaggerParamCollectionFormat.MULTI.equals(collectionFormat)) { value = request.getParameterValues(paramPath); - //Even if the paramPath does not exist, it won't be null at now, may be optimized in the future - if (value == null) { - value = checkRequiredAndDefaultValue(); - } + //Even if the paramPath does not exist, value won't be null at now } else { value = request.getParameter(paramPath); // make some old systems happy @@ -85,7 +82,7 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { return convertValue(value, targetType); } - private Object checkRequiredAndDefaultValue() throws Exception { + private Object checkRequiredAndDefaultValue() { if (isRequired()) { throw new InvocationException(Status.BAD_REQUEST, "Parameter is required."); } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestCookieProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestCookieProcessor.java index ecb347e..63364c4 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestCookieProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestCookieProcessor.java @@ -70,7 +70,21 @@ public class TestCookieProcessor { } }; - CookieProcessor processor = createProcessor("c1", String.class); + CookieProcessor processor = createProcessor("c1", String.class, null, false); + Object value = processor.getValue(request); + Assert.assertNull(value); + } + + @Test + public void testNoCookieAndRequired() throws Exception { + new Expectations() { + { + request.getCookies(); + result = null; + } + }; + + CookieProcessor processor = createProcessor("c1", String.class, null, true); try { processor.getValue(request); Assert.assertEquals("required is true, throw exception", "not throw exception"); @@ -89,13 +103,9 @@ public class TestCookieProcessor { } }; - CookieProcessor processor = createProcessor("c2", String.class); - try { - processor.getValue(request); - Assert.assertEquals("required is true, throw exception", "not throw exception"); - } catch (Exception e) { - Assert.assertTrue(e.getMessage().contains("Parameter is required.")); - } + CookieProcessor processor = createProcessor("c2", String.class, null, false); + Object value = processor.getValue(request); + Assert.assertNull(value); } @Test @@ -123,7 +133,7 @@ public class TestCookieProcessor { } }; - CookieProcessor processor = createProcessor("c1", String.class); + CookieProcessor processor = createProcessor("c1", String.class, null, true); try { processor.getValue(request); Assert.assertEquals("required is true, throw exception", "not throw exception"); diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestFormProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestFormProcessor.java index b91b18a..c14403e 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestFormProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestFormProcessor.java @@ -51,6 +51,10 @@ public class TestFormProcessor { return new FormProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true); } + private FormProcessor createProcessor(String name, Class<?> type, String defaultValue, boolean required) { + return new FormProcessor(name, TypeFactory.defaultInstance().constructType(type), defaultValue, required); + } + private void createClientRequest() { clientRequest = new MockUp<RestClientRequest>() { @Mock @@ -117,13 +121,9 @@ public class TestFormProcessor { } }; - ParamValueProcessor processor = createProcessor("name", String[].class); - try { - processor.getValue(request); - Assert.assertEquals("required is true, throw exception", "not throw exception"); - } catch (Exception e) { - Assert.assertTrue(e.getMessage().contains("Parameter is required.")); - } + ParamValueProcessor processor = createProcessor("name", String[].class, null, false); + String[] value = (String[]) processor.getValue(request); + Assert.assertNull(value); } @Test @@ -135,7 +135,7 @@ public class TestFormProcessor { } }; - ParamValueProcessor processor = createProcessor("name", String.class); + ParamValueProcessor processor = createProcessor("name", String.class, null, true); try { processor.getValue(request); Assert.assertEquals("required is true, throw exception", "not throw exception"); diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestHeaderProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestHeaderProcessor.java index 37ddc56..a205336 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestHeaderProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestHeaderProcessor.java @@ -106,13 +106,9 @@ public class TestHeaderProcessor { } }; - HeaderProcessor processor = createProcessor("h1", String[].class); - try { - processor.getValue(request); - Assert.assertEquals("required is true, throw exception", "not throw exception"); - } catch (Exception e) { - Assert.assertTrue(e.getMessage().contains("Parameter is required.")); - } + HeaderProcessor processor = createProcessor("h1", String[].class, null, false); + String[] value = (String[]) processor.getValue(request); + Assert.assertNull(value); } @Test @@ -189,7 +185,8 @@ public class TestHeaderProcessor { }; HeaderProcessor processor = - new HeaderProcessor("h1", TypeFactory.defaultInstance().constructCollectionType(Set.class, String.class), null, true); + new HeaderProcessor("h1", TypeFactory.defaultInstance().constructCollectionType(Set.class, String.class), null, + true); Object value = processor.getValue(request); Assert.assertThat((Set<String>) value, Matchers.contains("h1v")); } diff --git a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultValue.java b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultValue.java index 8062ca1..d0b8039 100644 --- a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultValue.java +++ b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDefaultValue.java @@ -218,14 +218,13 @@ public class TestDefaultValue { } @Test - public void stringQueryTrue_springmvc_rt() { + public void stringQueryRequiredTrue_springmvc_rt() { try { - consumersSpringmvc.getSCBRestTemplate().getForObject("/stringQueryTrue", String.class); + consumersSpringmvc.getSCBRestTemplate().getForObject("/stringQueryRequiredTrue", String.class); assertEquals("required is true, throw exception", "not throw exception"); } catch (InvocationException e) { assertEquals(400, e.getStatusCode()); - assertEquals("InvocationException: code=400;msg=CommonExceptionData [message=Parameter is not valid.]", - e.getMessage()); + assertEquals(true, e.getMessage().contains("Parameter is not valid")); } } @@ -262,14 +261,13 @@ public class TestDefaultValue { } @Test - public void stringHeaderTrue_springmvc_rt() { + public void stringHeaderRequiredTrue_springmvc_rt() { try { - consumersSpringmvc.getSCBRestTemplate().getForObject("/stringHeaderTrue", String.class); + consumersSpringmvc.getSCBRestTemplate().getForObject("/stringHeaderRequiredTrue", String.class); assertEquals("required is true, throw exception", "not throw exception"); } catch (InvocationException e) { assertEquals(400, e.getStatusCode()); - assertEquals("InvocationException: code=400;msg=CommonExceptionData [message=Parameter is not valid.]", - e.getMessage()); + assertEquals(true, e.getMessage().contains("Parameter is not valid")); } } @@ -307,6 +305,17 @@ public class TestDefaultValue { } @Test + public void stringFormRequiredTrue_springmvc_rt() { + try { + consumersSpringmvc.getSCBRestTemplate().getForObject("/stringFormRequiredTrue", String.class); + assertEquals("required is true, throw exception", "not throw exception"); + } catch (InvocationException e) { + assertEquals(400, e.getStatusCode()); + assertEquals(true, e.getMessage().contains("Parameter is not valid")); + } + } + + @Test public void intQuery_require_springmvc_intf() { assertEquals(defaultInt, consumersSpringmvc.getIntf().intQueryRequire(null)); } @@ -374,62 +383,35 @@ public class TestDefaultValue { @Test public void intForm_require_springmvc_intf() { - try { - consumersSpringmvc.getIntf().intFormRequire(null); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultInt, consumersSpringmvc.getIntf().intFormRequire(null)); } @Test public void doubleForm_require_springmvc_intf() { - try { - consumersSpringmvc.getIntf().doubleFormRequire(null); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultDouble, consumersSpringmvc.getIntf().doubleFormRequire(null), 0.0); } @Test public void stringForm_require_springmvc_intf() { - try { - consumersSpringmvc.getIntf().stringFormRequire(null); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultStr, consumersSpringmvc.getIntf().stringFormRequire(null)); } @Test public void intForm_require_springmvc_rt() { - try { - consumersSpringmvc.getSCBRestTemplate().postForObject("/intFormRequire", null, int.class); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultInt, + (int) consumersSpringmvc.getSCBRestTemplate().postForObject("/intFormRequire", null, int.class)); } @Test public void doubleForm_require_springmvc_rt() { - try { - consumersSpringmvc.getSCBRestTemplate().postForObject("/doubleFormRequire", null, double.class); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultDouble, + consumersSpringmvc.getSCBRestTemplate().postForObject("/doubleFormRequire", null, double.class), 0.0); } @Test public void stringForm_require_springmvc_rt() { - try { - consumersSpringmvc.getSCBRestTemplate().postForObject("/stringFormRequire", null, String.class); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultStr, + consumersSpringmvc.getSCBRestTemplate().postForObject("/stringFormRequire", null, String.class)); } //float @@ -524,21 +506,12 @@ public class TestDefaultValue { @Test public void floatForm_require_springmvc_intf() { - try { - consumersSpringmvc.getIntf().floatFormRequire(null); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultFloat, consumersSpringmvc.getIntf().floatFormRequire(null), 0.0f); } @Test public void floatForm_require_springmvc_rt() { - try { - consumersSpringmvc.getSCBRestTemplate().postForObject("/floatFormRequire", null, float.class); - assertEquals("required is true, throw exception", "but not throw exception"); - } catch (Exception e) { - assertEquals(true, e.getMessage().contains("Parameter is not valid")); - } + assertEquals(defaultFloat, + consumersSpringmvc.getSCBRestTemplate().postForObject("/floatFormRequire", null, float.class), 0.0f); } } diff --git a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DefaultValueSpringmvcSchema.java b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DefaultValueSpringmvcSchema.java index 0685858..f0501d2 100644 --- a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DefaultValueSpringmvcSchema.java +++ b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DefaultValueSpringmvcSchema.java @@ -22,6 +22,7 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RequestPart; import io.swagger.annotations.ApiImplicitParam; import io.swagger.annotations.ApiImplicitParams; @@ -59,7 +60,7 @@ public class DefaultValueSpringmvcSchema { } @ApiImplicitParams({ - @ApiImplicitParam(name = "input", dataType = "integer", format = "int32", paramType = "form", value = "a required form param", required = true, defaultValue = "13")}) + @ApiImplicitParam(name = "input", dataType = "integer", format = "int32", paramType = "form", value = "a defaultValue form param", required = false, defaultValue = "13")}) @PostMapping(path = "intFormRequire") public int intFormRequire(int input) { return input; @@ -71,8 +72,8 @@ public class DefaultValueSpringmvcSchema { return input; } - @GetMapping("stringQueryTrue") - public String stringQueryTrue(@RequestParam(value = "input") String input) { + @GetMapping("stringQueryRequiredTrue") + public String stringQueryRequiredTrue(@RequestParam(value = "input") String input) { return input; } @@ -81,8 +82,8 @@ public class DefaultValueSpringmvcSchema { return input; } - @GetMapping("stringHeaderTrue") - public String stringHeaderTrue(@RequestHeader(value = "input") String input) { + @GetMapping("stringHeaderRequiredTrue") + public String stringHeaderRequiredTrue(@RequestHeader(value = "input") String input) { return input; } @@ -93,6 +94,11 @@ public class DefaultValueSpringmvcSchema { return input; } + @GetMapping("stringFormRequiredTrue") + public String stringFormRequiredTrue(@RequestPart(value = "input") String input) { + return input; + } + // springmvc rule: required should be false because defaultValue have value @GetMapping(path = "stringQueryRequire") public String stringQueryRequire( @@ -108,7 +114,7 @@ public class DefaultValueSpringmvcSchema { } @ApiImplicitParams({ - @ApiImplicitParam(name = "input", dataType = "string", paramType = "form", value = "a required form param", required = true, defaultValue = "string")}) + @ApiImplicitParam(name = "input", dataType = "string", paramType = "form", value = "a defalutValue form param", required = false, defaultValue = "string")}) @PostMapping(path = "stringFormRequire") public String stringFormRequire(String input) { return input; @@ -147,7 +153,7 @@ public class DefaultValueSpringmvcSchema { } @ApiImplicitParams({ - @ApiImplicitParam(name = "input", dataType = "number", format = "double", paramType = "form", value = "a required form param", required = true, defaultValue = "10.2")}) + @ApiImplicitParam(name = "input", dataType = "number", format = "double", paramType = "form", value = "a defaultValue form param", required = false, defaultValue = "10.2")}) @PostMapping(path = "doubleFormRequire") public double doubleFormRequire(double input) { return input; @@ -185,7 +191,7 @@ public class DefaultValueSpringmvcSchema { } @ApiImplicitParams({ - @ApiImplicitParam(name = "input", dataType = "number", format = "float", paramType = "form", value = "a required form param", required = true, defaultValue = "10.2")}) + @ApiImplicitParam(name = "input", dataType = "number", format = "float", paramType = "form", value = "a defaultValue form param", required = false, defaultValue = "10.2")}) @PostMapping(path = "floatFormRequire") public float floatFormRequire(float input) { return input;
