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 ea5e6ae676fe810e52c767b8e5a964d7874b60f1 Author: weichao666 <[email protected]> AuthorDate: Fri Nov 23 14:25:50 2018 +0800 [SCB-925] Springmvc, when have defaultValue, required should be false，when param not exist,should check isRequired --- .../rest/codec/param/AbstractParamProcessor.java | 9 +++- .../rest/codec/param/CookieProcessorCreator.java | 38 +++++++++----- .../rest/codec/param/FormProcessorCreator.java | 39 ++++++++++---- .../rest/codec/param/HeaderProcessorCreator.java | 38 ++++++++++---- .../rest/codec/param/PathProcessorCreator.java | 6 +-- .../rest/codec/param/QueryProcessorCreator.java | 28 +++++++--- .../rest/codec/param/TestCookieProcessor.java | 56 ++++++++++++++++++-- .../common/rest/codec/param/TestFormProcessor.java | 33 ++++++++++-- .../rest/codec/param/TestHeaderProcessor.java | 50 ++++++++++++++++-- .../common/rest/codec/param/TestPathProcessor.java | 2 +- .../rest/codec/param/TestQueryProcessor.java | 40 ++++++++++++++- .../main/resources/microservices/pojo/server.yaml | 2 +- .../springmvc/server/SpringMvcDefaultValues.java | 2 +- .../servicecomb/it/testcase/TestDefaultValue.java | 60 +++++++++++++++++----- .../tests/SpringMvcIntegrationTestBase.java | 14 +++-- .../parameter/AbstractParameterProcessor.java | 5 +- .../annotation/CookieValueAnnotationProcessor.java | 6 +++ .../RequestHeaderAnnotationProcessor.java | 9 +++- .../RequestParamAnnotationProcessor.java | 10 +++- 19 files changed, 360 insertions(+), 87 deletions(-) diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/AbstractParamProcessor.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/AbstractParamProcessor.java index 509e03e..147aa0a 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/AbstractParamProcessor.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/AbstractParamProcessor.java @@ -26,17 +26,24 @@ public abstract class AbstractParamProcessor implements ParamValueProcessor { protected Object defaultValue; + protected boolean required = false; + public Object getDefaultValue() { return defaultValue; } - public AbstractParamProcessor(String paramPath, JavaType targetType, Object defaultValue) { + public AbstractParamProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { this.paramPath = paramPath; this.targetType = targetType; this.defaultValue = defaultValue; + this.required = required; } public String getParameterPath() { return paramPath; } + + public boolean isRequired() { + return required; + } } 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 659637e..894a379 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 @@ -21,9 +21,12 @@ import java.lang.reflect.Type; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.core.Response.Status; import org.apache.servicecomb.common.rest.codec.RestClientRequest; import org.apache.servicecomb.common.rest.codec.RestObjectMapperFactory; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; +import org.springframework.util.ObjectUtils; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -35,33 +38,41 @@ public class CookieProcessorCreator implements ParamValueProcessorCreator { public static final String PARAMTYPE = "cookie"; public static class CookieProcessor extends AbstractParamProcessor { - public CookieProcessor(String paramPath, JavaType targetType, Object defaultValue) { - super(paramPath, targetType, defaultValue); + public CookieProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { + super(paramPath, targetType, defaultValue, required); } @Override public Object getValue(HttpServletRequest request) throws Exception { Cookie[] cookies = request.getCookies(); - if (cookies == null) { - return null; + Object value = null; + if (cookies == null || cookies.length == 0) { + value = checkRequiredAndDefaultValue(value); + return convertValue(value, targetType); } - String value = null; for (Cookie cookie : cookies) { if (paramPath.equals(cookie.getName())) { value = cookie.getValue(); - if (value == null) { - Object defaultValue = getDefaultValue(); - if (defaultValue != null) { - value = defaultValue.toString(); - } - } } } - + if (value == null) { + value = checkRequiredAndDefaultValue(value); + } return convertValue(value, targetType); } + private Object checkRequiredAndDefaultValue(Object value) { + if (isRequired()) { + throw new InvocationException(Status.BAD_REQUEST, "Parameter is not valid, required is true"); + } + Object defaultValue = getDefaultValue(); + if (defaultValue != null) { + return defaultValue; + } + return value; + } + @Override public void setValue(RestClientRequest clientRequest, Object arg) throws Exception { clientRequest.addCookie(paramPath, @@ -81,6 +92,7 @@ public class CookieProcessorCreator implements ParamValueProcessorCreator { @Override public ParamValueProcessor create(Parameter parameter, Type genericParamType) { JavaType targetType = TypeFactory.defaultInstance().constructType(genericParamType); - return new CookieProcessor(parameter.getName(), targetType, ((CookieParameter) parameter).getDefaultValue()); + return new CookieProcessor(parameter.getName(), targetType, ((CookieParameter) parameter).getDefaultValue(), + parameter.getRequired()); } } 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 81cf912..7e4f448 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 @@ -22,9 +22,11 @@ import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.Part; +import javax.ws.rs.core.Response.Status; import org.apache.servicecomb.common.rest.RestConst; import org.apache.servicecomb.common.rest.codec.RestClientRequest; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -37,8 +39,8 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { public static final String PARAMTYPE = "formData"; public static class FormProcessor extends AbstractParamProcessor { - public FormProcessor(String paramPath, JavaType targetType, Object defaultValue) { - super(paramPath, targetType, defaultValue); + public FormProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { + super(paramPath, targetType, defaultValue, required); } @Override @@ -50,20 +52,33 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { } if (targetType.isContainerType()) { - return convertValue(request.getParameterValues(paramPath), targetType); + 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(values); + } + return convertValue(values, targetType); } Object value = request.getParameter(paramPath); if (value == null) { - Object defaultValue = getDefaultValue(); - if (defaultValue != null) { - value = defaultValue; - } + value = checkRequiredAndDefaultValue(value); } return convertValue(value, targetType); } + private Object checkRequiredAndDefaultValue(Object values) { + if (isRequired()) { + throw new InvocationException(Status.BAD_REQUEST, "Parameter is not valid, required is true"); + } + Object defaultValue = getDefaultValue(); + if (defaultValue != null) { + return defaultValue; + } + return values; + } + @Override public void setValue(RestClientRequest clientRequest, Object arg) { clientRequest.addForm(paramPath, arg); @@ -84,9 +99,11 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { JavaType targetType = TypeFactory.defaultInstance().constructType(genericParamType); if (isPart(parameter)) { - return new PartProcessor(parameter.getName(), targetType, ((FormParameter) parameter).getDefaultValue()); + return new PartProcessor(parameter.getName(), targetType, ((FormParameter) parameter).getDefaultValue(), + parameter.getRequired()); } - return new FormProcessor(parameter.getName(), targetType, ((FormParameter) parameter).getDefaultValue()); + return new FormProcessor(parameter.getName(), targetType, ((FormParameter) parameter).getDefaultValue(), + parameter.getRequired()); } private boolean isPart(Parameter parameter) { @@ -94,8 +111,8 @@ public class FormProcessorCreator implements ParamValueProcessorCreator { } private static class PartProcessor extends AbstractParamProcessor { - PartProcessor(String paramPath, JavaType targetType, Object defaultValue) { - super(paramPath, targetType, defaultValue); + PartProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { + super(paramPath, targetType, defaultValue, required); } @Override 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 810e018..d946afd 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 @@ -22,9 +22,11 @@ import java.util.Collections; import java.util.Enumeration; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.core.Response.Status; import org.apache.servicecomb.common.rest.codec.RestClientRequest; import org.apache.servicecomb.common.rest.codec.RestObjectMapperFactory; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,33 +42,46 @@ public class HeaderProcessorCreator implements ParamValueProcessorCreator { public static final String PARAMTYPE = "header"; public static class HeaderProcessor extends AbstractParamProcessor { - public HeaderProcessor(String paramPath, JavaType targetType, Object defaultValue) { - super(paramPath, targetType, defaultValue); + public HeaderProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { + super(paramPath, targetType, defaultValue, required); } @Override public Object getValue(HttpServletRequest request) throws Exception { Object value = null; if (targetType.isContainerType()) { - Enumeration<String> headerValues = request.getHeaders(paramPath); + 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 if (headerValues == null) { - return null; + Object obj = checkRequiredAndDefaultValue(headerValues); + if (obj instanceof Enumeration) { + headerValues = (Enumeration<?>) obj; + } + } + if (headerValues != null) { + value = Collections.list(headerValues); } - - value = Collections.list(headerValues); } else { value = request.getHeader(paramPath); if (value == null) { - Object defaultValue = getDefaultValue(); - if (defaultValue != null) { - value = defaultValue; - } + value = checkRequiredAndDefaultValue(value); } } return convertValue(value, targetType); } + private Object checkRequiredAndDefaultValue(Object headerValue) { + if (isRequired()) { + throw new InvocationException(Status.BAD_REQUEST, "Parameter is not valid, required is true"); + } + Object defaultValue = getDefaultValue(); + if (defaultValue != null) { + return defaultValue; + } + return headerValue; + } + @Override public void setValue(RestClientRequest clientRequest, Object arg) throws Exception { if (null == arg) { @@ -91,6 +106,7 @@ public class HeaderProcessorCreator implements ParamValueProcessorCreator { @Override public ParamValueProcessor create(Parameter parameter, Type genericParamType) { JavaType targetType = TypeFactory.defaultInstance().constructType(genericParamType); - return new HeaderProcessor(parameter.getName(), targetType, ((HeaderParameter) parameter).getDefaultValue()); + return new HeaderProcessor(parameter.getName(), targetType, ((HeaderParameter) parameter).getDefaultValue(), + parameter.getRequired()); } } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java index dbf459c..6f5f3fd 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java @@ -36,8 +36,8 @@ public class PathProcessorCreator implements ParamValueProcessorCreator { public static final String PARAMTYPE = "path"; public static class PathProcessor extends AbstractParamProcessor { - public PathProcessor(String paramPath, JavaType targetType, Object defaultValue) { - super(paramPath, targetType, defaultValue); + public PathProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required) { + super(paramPath, targetType, defaultValue, required); } @Override @@ -73,6 +73,6 @@ public class PathProcessorCreator implements ParamValueProcessorCreator { @Override public ParamValueProcessor create(Parameter parameter, Type genericParamType) { JavaType targetType = TypeFactory.defaultInstance().constructType(genericParamType); - return new PathProcessor(parameter.getName(), targetType, ((PathParameter) parameter).getDefaultValue()); + return new PathProcessor(parameter.getName(), targetType, ((PathParameter) parameter).getDefaultValue(), true); } } 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 952c649..b449ae2 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 @@ -20,10 +20,12 @@ package org.apache.servicecomb.common.rest.codec.param; import java.lang.reflect.Type; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.core.Response.Status; import org.apache.commons.lang3.StringUtils; import org.apache.servicecomb.common.rest.codec.RestClientRequest; import org.apache.servicecomb.swagger.converter.property.SwaggerParamCollectionFormat; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -46,8 +48,9 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { private SwaggerParamCollectionFormat collectionFormat; - public QueryProcessor(String paramPath, JavaType targetType, Object defaultValue, String collectionFormat) { - super(paramPath, targetType, defaultValue); + public QueryProcessor(String paramPath, JavaType targetType, Object defaultValue, boolean required, + String collectionFormat) { + super(paramPath, targetType, defaultValue, required); if (StringUtils.isNoneEmpty(collectionFormat)) { this.collectionFormat = SwaggerParamCollectionFormat.valueOf(collectionFormat.toUpperCase()); } @@ -59,6 +62,10 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { 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(value); + } } else { value = request.getParameter(paramPath); // make some old systems happy @@ -68,10 +75,7 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { } } if (value == null) { - Object defaultValue = getDefaultValue(); - if (!ignoreDefaultValue && defaultValue != null) { - value = defaultValue; - } + value = checkRequiredAndDefaultValue(value); } if (null != collectionFormat) { value = collectionFormat.splitParam((String) value); @@ -81,6 +85,17 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { return convertValue(value, targetType); } + private Object checkRequiredAndDefaultValue(Object value) { + if (isRequired()) { + throw new InvocationException(Status.BAD_REQUEST, "Parameter is not valid, required is true"); + } + Object defaultValue = getDefaultValue(); + if (!ignoreDefaultValue && defaultValue != null) { + return defaultValue; + } + return value; + } + @Override public void setValue(RestClientRequest clientRequest, Object arg) throws Exception { // query不需要set @@ -105,6 +120,7 @@ public class QueryProcessorCreator implements ParamValueProcessorCreator { QueryParameter queryParameter = (QueryParameter) parameter; JavaType targetType = TypeFactory.defaultInstance().constructType(genericParamType); return new QueryProcessor(parameter.getName(), targetType, queryParameter.getDefaultValue(), + parameter.getRequired(), queryParameter.getCollectionFormat()); } } 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 3705434..4628b58 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 @@ -45,7 +45,11 @@ public class TestCookieProcessor { RestClientRequest clientRequest; private CookieProcessor createProcessor(String name, Class<?> type) { - return new CookieProcessor(name, TypeFactory.defaultInstance().constructType(type), null); + return new CookieProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true); + } + + private CookieProcessor createProcessor(String name, Class<?> type, String defaultValue, boolean required) { + return new CookieProcessor(name, TypeFactory.defaultInstance().constructType(type), defaultValue, required); } private void createClientRequest() { @@ -67,8 +71,12 @@ public class TestCookieProcessor { }; CookieProcessor processor = createProcessor("c1", String.class); - Object value = processor.getValue(request); - Assert.assertNull(value); + try { + processor.getValue(request); + Assert.assertEquals("required is true, throw exception", "not throw exception"); + } catch (Exception e) { + Assert.assertTrue(e.getMessage().contains("Parameter is not valid, required is true")); + } } @Test @@ -82,8 +90,12 @@ public class TestCookieProcessor { }; CookieProcessor processor = createProcessor("c2", String.class); - Object value = processor.getValue(request); - Assert.assertNull(value); + try { + processor.getValue(request); + Assert.assertEquals("required is true, throw exception", "not throw exception"); + } catch (Exception e) { + Assert.assertTrue(e.getMessage().contains("Parameter is not valid, required is true")); + } } @Test @@ -101,6 +113,40 @@ public class TestCookieProcessor { Assert.assertEquals("c1v", value); } + @Test + public void testGetValueRequiredTrue() throws Exception { + Cookie[] cookies = new Cookie[] {new Cookie("c1", null)}; + new Expectations() { + { + request.getCookies(); + result = cookies; + } + }; + + CookieProcessor processor = createProcessor("c1", 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 not valid, required is true")); + } + } + + @Test + public void testGetValueRequiredFalse() throws Exception { + Cookie[] cookies = new Cookie[] {new Cookie("c1", null)}; + new Expectations() { + { + request.getCookies(); + result = cookies; + } + }; + + CookieProcessor processor = createProcessor("c1", String.class, "test", false); + Object result = processor.getValue(request); + Assert.assertEquals("test", result); + } + @SuppressWarnings("deprecation") @Test public void testGetValueCookiesDate() throws 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 a756940..35f869d 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 @@ -48,7 +48,7 @@ public class TestFormProcessor { RestClientRequest clientRequest; private FormProcessor createProcessor(String name, Class<?> type) { - return new FormProcessor(name, TypeFactory.defaultInstance().constructType(type), null); + return new FormProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true); } private void createClientRequest() { @@ -118,8 +118,30 @@ public class TestFormProcessor { }; ParamValueProcessor processor = createProcessor("name", String[].class); - String[] value = (String[]) processor.getValue(request); - Assert.assertNull(value); + try { + processor.getValue(request); + Assert.assertEquals("required is true, throw exception", "not throw exception"); + } catch (Exception e) { + Assert.assertTrue(e.getMessage().contains("Parameter is not valid, required is true")); + } + } + + @Test + public void testGetValueNull() throws Exception { + new Expectations() { + { + request.getParameter("name"); + result = null; + } + }; + + 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 not valid, required is true")); + } } @Test @@ -148,7 +170,7 @@ public class TestFormProcessor { ParamValueProcessor processor = new FormProcessor("name", TypeFactory.defaultInstance().constructCollectionType(List.class, String.class), - null); + null, true); Object value = processor.getValue(request); Assert.assertThat((List<String>) value, Matchers.contains("value")); } @@ -164,7 +186,8 @@ public class TestFormProcessor { }; ParamValueProcessor processor = - new FormProcessor("name", TypeFactory.defaultInstance().constructCollectionType(Set.class, String.class), null); + new FormProcessor("name", TypeFactory.defaultInstance().constructCollectionType(Set.class, String.class), null, + true); Object value = processor.getValue(request); Assert.assertThat((Set<String>) value, Matchers.contains("value")); } 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 667e53b..70b605b 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 @@ -49,7 +49,11 @@ public class TestHeaderProcessor { RestClientRequest clientRequest; private HeaderProcessor createProcessor(String name, Class<?> type) { - return new HeaderProcessor(name, TypeFactory.defaultInstance().constructType(type), null); + return new HeaderProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true); + } + + private HeaderProcessor createProcessor(String name, Class<?> type, String defaultValue, boolean required) { + return new HeaderProcessor(name, TypeFactory.defaultInstance().constructType(type), defaultValue, required); } private void createClientRequest() { @@ -103,8 +107,44 @@ public class TestHeaderProcessor { }; HeaderProcessor processor = createProcessor("h1", String[].class); - String[] value = (String[]) processor.getValue(request); - Assert.assertNull(value); + try { + processor.getValue(request); + Assert.assertEquals("required is true, throw exception", "not throw exception"); + } catch (Exception e) { + Assert.assertTrue(e.getMessage().contains("Parameter is not valid, required is true")); + } + } + + @Test + public void testGetValueRequiredTrue() throws Exception { + new Expectations() { + { + request.getHeader("h1"); + result = null; + } + }; + + 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 not valid, required is true")); + } + } + + @Test + public void testGetValueRequiredFalse() throws Exception { + new Expectations() { + { + request.getHeader("h1"); + result = null; + } + }; + + HeaderProcessor processor = createProcessor("h1", String.class, "test", false); + Object value = processor.getValue(request); + Assert.assertEquals("test", value); } @Test @@ -133,7 +173,7 @@ public class TestHeaderProcessor { HeaderProcessor processor = new HeaderProcessor("h1", TypeFactory.defaultInstance().constructCollectionType(List.class, String.class), - null); + null, true); Object value = processor.getValue(request); Assert.assertThat((List<String>) value, Matchers.contains("h1v")); } @@ -149,7 +189,7 @@ public class TestHeaderProcessor { }; HeaderProcessor processor = - new HeaderProcessor("h1", TypeFactory.defaultInstance().constructCollectionType(Set.class, String.class), null); + 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/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java index 19cd51f..19cca94 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java @@ -41,7 +41,7 @@ public class TestPathProcessor { ParamValueProcessor processor; private void createProcessor(String name, Class<?> type) { - processor = new PathProcessor(name, TypeFactory.defaultInstance().constructType(type), null); + processor = new PathProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true); } private void prepareGetValue(String name, Class<?> type) { diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestQueryProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestQueryProcessor.java index ec896a2..c7e6d5b 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestQueryProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestQueryProcessor.java @@ -34,7 +34,13 @@ public class TestQueryProcessor { HttpServletRequest request; private ParamValueProcessor createProcessor(String name, Class<?> type, String collectionFormat) { - return new QueryProcessor(name, TypeFactory.defaultInstance().constructType(type), null, collectionFormat); + return new QueryProcessor(name, TypeFactory.defaultInstance().constructType(type), null, true, collectionFormat); + } + + private ParamValueProcessor createProcessor(String name, Class<?> type, String defaultValue, boolean required, + String collectionFormat) { + return new QueryProcessor(name, TypeFactory.defaultInstance().constructType(type), defaultValue, required, + collectionFormat); } @Test @@ -84,4 +90,36 @@ public class TestQueryProcessor { ParamValueProcessor processor = createProcessor("name", String.class, "multi"); Assert.assertEquals("query", processor.getProcessorType()); } + + @Test + public void testGetValueRequiredTrue() throws Exception { + new Expectations() { + { + request.getParameter("name"); + result = null; + } + }; + + ParamValueProcessor processor = createProcessor("name", String.class, "multi"); + try { + processor.getValue(request); + Assert.assertEquals("required is true, throw exception", "not throw exception"); + } catch (Exception e) { + Assert.assertTrue(e.getMessage().contains("Parameter is not valid, required is true")); + } + } + + @Test + public void testGetValueRequiredFalse() throws Exception { + new Expectations() { + { + request.getParameter("name"); + result = null; + } + }; + + ParamValueProcessor processor = createProcessor("name", String.class, "test", false, "multi"); + Object result = processor.getValue(request); + Assert.assertEquals("test", result); + } } diff --git a/demo/demo-schema/src/main/resources/microservices/pojo/server.yaml b/demo/demo-schema/src/main/resources/microservices/pojo/server.yaml index 985fcb7..15b1d82 100644 --- a/demo/demo-schema/src/main/resources/microservices/pojo/server.yaml +++ b/demo/demo-schema/src/main/resources/microservices/pojo/server.yaml @@ -57,7 +57,7 @@ paths: parameters: - name: code in: query - required: true + required: false type: string responses: 200: diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMvcDefaultValues.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMvcDefaultValues.java index d6b33eb..3e91509 100644 --- a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMvcDefaultValues.java +++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMvcDefaultValues.java @@ -61,7 +61,7 @@ public class SpringMvcDefaultValues { } @GetMapping("/query3") - public String query3(@RequestParam("a") @Min(value = 20) int a, @RequestParam("b") String b) { + public String query3(@RequestParam("a") @Min(value = 20) int a, @RequestParam(name = "b", required = false) String b) { return "Hello " + a + b; } 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 d4ad947..1c172ee 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 @@ -349,35 +349,62 @@ public class TestDefaultValue { @Test public void intForm_require_springmvc_intf() { - assertEquals(defaultInt, consumersSpringmvc.getIntf().intFormRequire(null)); + 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")); + } } @Test public void doubleForm_require_springmvc_intf() { - assertEquals(defaultDouble, consumersSpringmvc.getIntf().doubleFormRequire(null), 0.0); + 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")); + } } @Test public void stringForm_require_springmvc_intf() { - assertEquals(defaultStr, consumersSpringmvc.getIntf().stringFormRequire(null)); + 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")); + } } @Test public void intForm_require_springmvc_rt() { - assertEquals(defaultInt, - (int) consumersSpringmvc.getSCBRestTemplate().postForObject("/intFormRequire", null, int.class)); + 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")); + } } @Test public void doubleForm_require_springmvc_rt() { - assertEquals(defaultDouble, - consumersSpringmvc.getSCBRestTemplate().postForObject("/doubleFormRequire", null, double.class), 0.0); + 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")); + } } @Test public void stringForm_require_springmvc_rt() { - assertEquals(defaultStr, - consumersSpringmvc.getSCBRestTemplate().postForObject("/stringFormRequire", null, String.class)); + 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")); + } } //float @@ -472,12 +499,21 @@ public class TestDefaultValue { @Test public void floatForm_require_springmvc_intf() { - assertEquals(defaultFloat, consumersSpringmvc.getIntf().floatFormRequire(null), 0.0f); + 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")); + } } @Test public void floatForm_require_springmvc_rt() { - assertEquals(defaultFloat, - consumersSpringmvc.getSCBRestTemplate().postForObject("/floatFormRequire", null, float.class), 0.0f); + 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")); + } } } diff --git a/integration-tests/springmvc-tests/springmvc-tests-common/src/test/java/org/apache/servicecomb/demo/springmvc/tests/SpringMvcIntegrationTestBase.java b/integration-tests/springmvc-tests/springmvc-tests-common/src/test/java/org/apache/servicecomb/demo/springmvc/tests/SpringMvcIntegrationTestBase.java index c153a5a..c84ef3e 100644 --- a/integration-tests/springmvc-tests/springmvc-tests-common/src/test/java/org/apache/servicecomb/demo/springmvc/tests/SpringMvcIntegrationTestBase.java +++ b/integration-tests/springmvc-tests/springmvc-tests-common/src/test/java/org/apache/servicecomb/demo/springmvc/tests/SpringMvcIntegrationTestBase.java @@ -48,6 +48,7 @@ import org.apache.servicecomb.demo.compute.Person; import org.apache.servicecomb.demo.server.User; import org.apache.servicecomb.provider.springmvc.reference.RestTemplateBuilder; import org.apache.servicecomb.provider.springmvc.reference.async.CseAsyncRestTemplate; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import org.junit.ClassRule; import org.junit.Ignore; import org.junit.Test; @@ -66,6 +67,7 @@ import org.springframework.util.MultiValueMap; import org.springframework.util.concurrent.ListenableFuture; import org.springframework.util.concurrent.ListenableFutureCallback; import org.springframework.web.client.AsyncRestTemplate; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; @Ignore @@ -321,10 +323,14 @@ public class SpringMvcIntegrationTestBase { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.MULTIPART_FORM_DATA); - ResponseEntity<String> response = restTemplate - .postForEntity(codeFirstUrl + "uploadWithoutAnnotation", new HttpEntity<>(map, headers), String.class); - assertThat(response.getStatusCodeValue(), is(590)); - assertThat(response.getBody(), is("CommonExceptionData [message=Cse Internal Server Error]")); + ResponseEntity<String> response = null; + try { + response = restTemplate + .postForEntity(codeFirstUrl + "uploadWithoutAnnotation", new HttpEntity<>(map, headers), String.class); + assertEquals("required is true, throw exception", "but not throw exception"); + } catch (RestClientException e) { + assertEquals("400 Bad Request",e.getMessage()); + } } @Test diff --git a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/processor/parameter/AbstractParameterProcessor.java b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/processor/parameter/AbstractParameterProcessor.java index 04d992c..54ec775 100644 --- a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/processor/parameter/AbstractParameterProcessor.java +++ b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/processor/parameter/AbstractParameterProcessor.java @@ -59,10 +59,9 @@ public abstract class AbstractParameterProcessor<T extends AbstractSerializableP protected void setParameterDefaultValue(Object annotation, T parameter) { String defaultValue = getAnnotationParameterDefaultValue(annotation); - if (StringUtils.isNotEmpty(defaultValue)) { - parameter.setDefaultValue(defaultValue); + if (StringUtils.isNotEmpty(defaultValue)) { + parameter.setDefaultValue(defaultValue); } - } protected String getAnnotationParameterDefaultValue(Object annotation) { diff --git a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/CookieValueAnnotationProcessor.java b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/CookieValueAnnotationProcessor.java index f96802d..7fb4d91 100644 --- a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/CookieValueAnnotationProcessor.java +++ b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/CookieValueAnnotationProcessor.java @@ -19,6 +19,7 @@ package org.apache.servicecomb.swagger.generator.springmvc.processor.annotation; import org.apache.servicecomb.swagger.generator.core.OperationGenerator; import org.apache.servicecomb.swagger.generator.core.processor.parameter.AbstractParameterProcessor; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.CookieValue; import org.springframework.web.bind.annotation.ValueConstants; @@ -44,6 +45,11 @@ public class CookieValueAnnotationProcessor extends AbstractParameterProcessor<C CookieParameter parameter) { super.fillParameter(annotation, operationGenerator, paramIdx, parameter); + Object defaultValue = parameter.getDefaultValue(); + if (!ObjectUtils.isEmpty(defaultValue)) { + parameter.setRequired(false); + return; + } CookieValue cookie = (CookieValue) annotation; parameter.setRequired(cookie.required()); } diff --git a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestHeaderAnnotationProcessor.java b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestHeaderAnnotationProcessor.java index d34c296..63be305 100644 --- a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestHeaderAnnotationProcessor.java +++ b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestHeaderAnnotationProcessor.java @@ -19,6 +19,7 @@ package org.apache.servicecomb.swagger.generator.springmvc.processor.annotation; import org.apache.servicecomb.swagger.generator.core.OperationGenerator; import org.apache.servicecomb.swagger.generator.core.processor.parameter.AbstractParameterProcessor; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.ValueConstants; @@ -44,10 +45,15 @@ public class RequestHeaderAnnotationProcessor extends AbstractParameterProcessor HeaderParameter parameter) { super.fillParameter(annotation, operationGenerator, paramIdx, parameter); + Object defaultValue = parameter.getDefaultValue(); + if (!ObjectUtils.isEmpty(defaultValue)) { + parameter.setRequired(false); + return; + } RequestHeader requestHeader = (RequestHeader) annotation; parameter.setRequired(requestHeader.required()); } - + @Override protected String getAnnotationParameterDefaultValue(Object annotation) { String defaultValue = ((RequestHeader) annotation).defaultValue(); @@ -56,5 +62,4 @@ public class RequestHeaderAnnotationProcessor extends AbstractParameterProcessor } return defaultValue; } - } diff --git a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestParamAnnotationProcessor.java b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestParamAnnotationProcessor.java index 8ad9a4b..828a782 100644 --- a/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestParamAnnotationProcessor.java +++ b/swagger/swagger-generator/generator-springmvc/src/main/java/org/apache/servicecomb/swagger/generator/springmvc/processor/annotation/RequestParamAnnotationProcessor.java @@ -17,13 +17,14 @@ package org.apache.servicecomb.swagger.generator.springmvc.processor.annotation; +import io.swagger.models.parameters.QueryParameter; + import org.apache.servicecomb.swagger.generator.core.OperationGenerator; import org.apache.servicecomb.swagger.generator.core.processor.parameter.AbstractParameterProcessor; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ValueConstants; -import io.swagger.models.parameters.QueryParameter; - public class RequestParamAnnotationProcessor extends AbstractParameterProcessor<QueryParameter> { @Override public QueryParameter createParameter() { @@ -44,6 +45,11 @@ public class RequestParamAnnotationProcessor extends AbstractParameterProcessor< QueryParameter parameter) { super.fillParameter(annotation, operationGenerator, paramIdx, parameter); + Object defaultValue = parameter.getDefaultValue(); + if (!ObjectUtils.isEmpty(defaultValue)) { + parameter.setRequired(false); + return; + } RequestParam requestParam = (RequestParam) annotation; parameter.setRequired(requestParam.required()); }
