You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2020/06/05 06:14:37 UTC
[servicecomb-java-chassis] branch master updated: [SCB-1982] add
validator filter
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
The following commit(s) were added to refs/heads/master by this push:
new ba87d01 [SCB-1982] add validator filter
ba87d01 is described below
commit ba87d0182005131f94700b657dc3853bd5867545
Author: wujimin <wu...@huawei.com>
AuthorDate: Thu Jun 4 01:42:33 2020 +0800
[SCB-1982] add validator filter
---
core/pom.xml | 9 ++
.../servicecomb/core/exception/ExceptionCodes.java | 1 +
.../ConstraintViolationExceptionConverter.java | 75 +++++++++
.../converter/ValidateDetail.java} | 40 +++--
.../core/filter/impl/DefaultFilterProvider.java | 1 +
.../impl/JacksonPropertyNodeNameProvider.java | 57 +++++++
.../core/filter/impl/ParameterValidatorFilter.java | 72 +++++++++
...e.servicecomb.core.exception.ExceptionConverter | 1 +
.../filter/impl/ParameterValidatorFilterTest.java | 168 +++++++++++++++++++++
dependencies/default/pom.xml | 2 +-
10 files changed, 413 insertions(+), 13 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index 3290b9e..bc6afae 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -49,6 +49,15 @@
<artifactId>brave</artifactId>
</dependency>
<dependency>
+ <groupId>org.hibernate.validator</groupId>
+ <artifactId>hibernate-validator</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.glassfish</groupId>
+ <artifactId>jakarta.el</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.jmockit</groupId>
<artifactId>jmockit</artifactId>
<scope>provided</scope>
diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java
index ad3f5e4..c47e5b8 100644
--- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java
+++ b/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java
@@ -20,6 +20,7 @@ public interface ExceptionCodes {
String GENERIC_CLIENT = "SCB.0000";
String LB_ADDRESS_NOT_FOUND = "SCB.0001";
String NOT_DEFINED_ANY_SCHEMA = "SCB.0002";
+ String DEFAULT_VALIDATE = "SCB.0003";
String GENERIC_SERVER = "SCB.5000";
}
diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/converter/ConstraintViolationExceptionConverter.java b/core/src/main/java/org/apache/servicecomb/core/exception/converter/ConstraintViolationExceptionConverter.java
new file mode 100644
index 0000000..2e6c262
--- /dev/null
+++ b/core/src/main/java/org/apache/servicecomb/core/exception/converter/ConstraintViolationExceptionConverter.java
@@ -0,0 +1,75 @@
+/*
+ * 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.core.exception.converter;
+
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static org.apache.servicecomb.core.exception.ExceptionCodes.DEFAULT_VALIDATE;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import javax.validation.ConstraintViolationException;
+import javax.ws.rs.core.Response.StatusType;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.exception.ExceptionConverter;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.config.DynamicStringProperty;
+
+public class ConstraintViolationExceptionConverter implements ExceptionConverter<ConstraintViolationException> {
+ public static final String KEY_CODE = "servicecomb.filters.validate.code";
+
+ private DynamicStringProperty code;
+
+ public ConstraintViolationExceptionConverter() {
+ refreshCode();
+ }
+
+ /**
+ * during UT, DynamicPropertyFactory will be reset, this caused code can not changed by event
+ */
+ @VisibleForTesting
+ public void refreshCode() {
+ code = DynamicPropertyFactory.getInstance().getStringProperty(KEY_CODE, DEFAULT_VALIDATE);
+ }
+
+ @Override
+ public int getOrder() {
+ return Short.MAX_VALUE;
+ }
+
+ @Override
+ public boolean canConvert(Throwable throwable) {
+ return throwable instanceof ConstraintViolationException;
+ }
+
+ @Override
+ public InvocationException convert(Invocation invocation, ConstraintViolationException throwable,
+ StatusType genericStatus) {
+ List<ValidateDetail> details = throwable.getConstraintViolations().stream()
+ .map(violation -> new ValidateDetail(violation.getPropertyPath().toString(), violation.getMessage()))
+ .collect(Collectors.toList());
+
+ CommonExceptionData exceptionData = new CommonExceptionData(code.get(), "invalid parameters.");
+ exceptionData.putDynamic("validateDetail", details);
+ return new InvocationException(BAD_REQUEST, exceptionData);
+ }
+}
diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java b/core/src/main/java/org/apache/servicecomb/core/exception/converter/ValidateDetail.java
similarity index 55%
copy from core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java
copy to core/src/main/java/org/apache/servicecomb/core/exception/converter/ValidateDetail.java
index ab4e4ac..06140be 100644
--- a/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java
+++ b/core/src/main/java/org/apache/servicecomb/core/exception/converter/ValidateDetail.java
@@ -14,20 +14,36 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.servicecomb.core.filter.impl;
+package org.apache.servicecomb.core.exception.converter;
-import java.util.Arrays;
-import java.util.List;
+public class ValidateDetail {
+ private String propertyPath;
-import org.apache.servicecomb.core.filter.Filter;
-import org.apache.servicecomb.core.filter.FilterProvider;
+ private String message;
-public class DefaultFilterProvider implements FilterProvider {
- @Override
- public List<Class<? extends Filter>> getFilters() {
- return Arrays.asList(
- SimpleLoadBalanceFilter.class,
- ScheduleFilter.class,
- ProducerOperationFilter.class);
+ public ValidateDetail() {
+ }
+
+ public ValidateDetail(String propertyPath, String message) {
+ this.propertyPath = propertyPath;
+ this.message = message;
+ }
+
+ public String getPropertyPath() {
+ return propertyPath;
+ }
+
+ public ValidateDetail setPropertyPath(String propertyPath) {
+ this.propertyPath = propertyPath;
+ return this;
+ }
+
+ public String getMessage() {
+ return message;
+ }
+
+ public ValidateDetail setMessage(String message) {
+ this.message = message;
+ return this;
}
}
diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java
index ab4e4ac..3cfd20e 100644
--- a/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java
+++ b/core/src/main/java/org/apache/servicecomb/core/filter/impl/DefaultFilterProvider.java
@@ -28,6 +28,7 @@ public class DefaultFilterProvider implements FilterProvider {
return Arrays.asList(
SimpleLoadBalanceFilter.class,
ScheduleFilter.class,
+ ParameterValidatorFilter.class,
ProducerOperationFilter.class);
}
}
diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/JacksonPropertyNodeNameProvider.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/JacksonPropertyNodeNameProvider.java
new file mode 100644
index 0000000..403ca6c
--- /dev/null
+++ b/core/src/main/java/org/apache/servicecomb/core/filter/impl/JacksonPropertyNodeNameProvider.java
@@ -0,0 +1,57 @@
+/*
+ * 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.core.filter.impl;
+
+import org.hibernate.validator.spi.nodenameprovider.JavaBeanProperty;
+import org.hibernate.validator.spi.nodenameprovider.Property;
+import org.hibernate.validator.spi.nodenameprovider.PropertyNodeNameProvider;
+
+import com.fasterxml.jackson.databind.BeanDescription;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
+
+import io.vertx.core.json.jackson.DatabindCodec;
+
+/**
+ * hibernate validator will cache the resolved data<br>
+ * no need to worry about performance problem
+ */
+public class JacksonPropertyNodeNameProvider implements PropertyNodeNameProvider {
+ @Override
+ public String getName(Property property) {
+ if (property instanceof JavaBeanProperty) {
+ return getJavaBeanPropertyName((JavaBeanProperty) property);
+ }
+
+ return property.getName();
+ }
+
+ private String getJavaBeanPropertyName(JavaBeanProperty property) {
+ ObjectMapper objectMapper = DatabindCodec.mapper();
+ JavaType type = objectMapper.constructType(property.getDeclaringClass());
+ BeanDescription desc = objectMapper.getSerializationConfig().introspect(type);
+
+ return desc.findProperties()
+ .stream()
+ .filter(prop -> prop.getInternalName().equals(property.getName()))
+ .map(BeanPropertyDefinition::getName)
+ .findFirst()
+ .orElse(property.getName());
+ }
+}
\ No newline at end of file
diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilter.java
new file mode 100644
index 0000000..4562467
--- /dev/null
+++ b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilter.java
@@ -0,0 +1,72 @@
+/*
+ * 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.core.filter.impl;
+
+import static org.apache.servicecomb.swagger.invocation.InvocationType.PRODUCER;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.ConstraintViolationException;
+import javax.validation.Validation;
+import javax.validation.ValidatorFactory;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.groups.Default;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.filter.Filter;
+import org.apache.servicecomb.core.filter.FilterMeta;
+import org.apache.servicecomb.core.filter.FilterNode;
+import org.apache.servicecomb.foundation.common.utils.AsyncUtils;
+import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation;
+import org.apache.servicecomb.swagger.invocation.Response;
+import org.hibernate.validator.HibernateValidator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@FilterMeta(name = "validator", invocationType = PRODUCER)
+public class ParameterValidatorFilter implements Filter {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ParameterValidatorFilter.class);
+
+ private final ExecutableValidator validator;
+
+ public ParameterValidatorFilter() {
+ ValidatorFactory factory =
+ Validation.byProvider(HibernateValidator.class)
+ .configure()
+ .propertyNodeNameProvider(new JacksonPropertyNodeNameProvider())
+ .buildValidatorFactory();
+ validator = factory.getValidator().forExecutables();
+ }
+
+ @Override
+ public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode nextNode) {
+ SwaggerProducerOperation producerOperation = invocation.getOperationMeta().getSwaggerProducerOperation();
+ Object instance = producerOperation.getProducerInstance();
+ Method method = producerOperation.getProducerMethod();
+ Object[] args = invocation.toProducerArguments();
+ Set<ConstraintViolation<Object>> violations = validator.validateParameters(instance, method, args, Default.class);
+ if (violations.size() > 0) {
+ LOGGER.error("Parameter validation failed : " + violations.toString());
+ return AsyncUtils.completeExceptionally(new ConstraintViolationException(violations));
+ }
+
+ return nextNode.onFilter(invocation);
+ }
+}
diff --git a/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.exception.ExceptionConverter b/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.exception.ExceptionConverter
index f129ed1..aea2c1c 100644
--- a/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.exception.ExceptionConverter
+++ b/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.exception.ExceptionConverter
@@ -15,6 +15,7 @@
# limitations under the License.
#
+org.apache.servicecomb.core.exception.converter.ConstraintViolationExceptionConverter
org.apache.servicecomb.core.exception.converter.InvocationExceptionConverter
org.apache.servicecomb.core.exception.converter.IllegalArgumentExceptionConverter
org.apache.servicecomb.core.exception.converter.DefaultExceptionConverter
\ No newline at end of file
diff --git a/core/src/test/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilterTest.java b/core/src/test/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilterTest.java
new file mode 100644
index 0000000..2b6a1a0
--- /dev/null
+++ b/core/src/test/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilterTest.java
@@ -0,0 +1,168 @@
+/*
+ * 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.core.filter.impl;
+
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static org.apache.servicecomb.core.exception.ExceptionCodes.DEFAULT_VALIDATE;
+import static org.apache.servicecomb.core.exception.converter.ConstraintViolationExceptionConverter.KEY_CODE;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+
+import java.util.List;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+
+import org.apache.commons.lang3.reflect.MethodUtils;
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.exception.ExceptionConverter;
+import org.apache.servicecomb.core.exception.Exceptions;
+import org.apache.servicecomb.core.exception.converter.ConstraintViolationExceptionConverter;
+import org.apache.servicecomb.core.exception.converter.ValidateDetail;
+import org.apache.servicecomb.core.filter.FilterNode;
+import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+import mockit.Expectations;
+import mockit.Mocked;
+
+public class ParameterValidatorFilterTest {
+ public static class BaseModel {
+ @NotNull(message = "can not be null")
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public BaseModel setName(String name) {
+ this.name = name;
+ return this;
+ }
+ }
+
+ public static class Model extends BaseModel {
+ @NotBlank(message = "can not be blank")
+ @JsonProperty(value = "nick-name")
+ private String nickName;
+
+ public String getNickName() {
+ return nickName;
+ }
+
+ public Model setNickName(String nickName) {
+ this.nickName = nickName;
+ return this;
+ }
+ }
+
+ public static class Controller {
+ public void op(@NotNull(message = "not null") String query, @Valid Model model) {
+
+ }
+ }
+
+ static ParameterValidatorFilter filter = new ParameterValidatorFilter();
+
+ @Mocked
+ Invocation invocation;
+
+ @Mocked
+ SwaggerProducerOperation operation;
+
+ @Before
+ public void setUp() throws Exception {
+ new Expectations() {
+ {
+ operation.getProducerInstance();
+ result = new Controller();
+
+ operation.getProducerMethod();
+ result = MethodUtils.getAccessibleMethod(Controller.class, "op", String.class, Model.class);
+
+ invocation.toProducerArguments();
+ result = new Object[] {null, new Model()};
+ }
+ };
+ }
+
+ private InvocationException getException() {
+ Throwable throwable = catchThrowable(() -> filter.onFilter(invocation, FilterNode.EMPTY).get());
+ return Exceptions.convert(invocation, throwable, BAD_REQUEST);
+ }
+
+ private CommonExceptionData getExceptionData() {
+ InvocationException invocationException = getException();
+ return (CommonExceptionData) invocationException.getErrorData();
+ }
+
+ @Test
+ public void status_code_should_be_bad_request() {
+ InvocationException invocationException = getException();
+
+ assertThat(invocationException.getStatusCode()).isEqualTo(BAD_REQUEST.getStatusCode());
+ }
+
+ @Test
+ public void error_code_and_message_should_be_build_in_value() {
+ ArchaiusUtils.setProperty(KEY_CODE, null);
+ CommonExceptionData errorData = getExceptionData();
+
+ assertThat(errorData.getCode()).isEqualTo(DEFAULT_VALIDATE);
+ assertThat(errorData.getMessage()).isEqualTo("invalid parameters.");
+ }
+
+ @Test
+ public void should_allow_customize_error_code_by_configuration() {
+ ArchaiusUtils.setProperty(KEY_CODE, "test.0001");
+ SPIServiceUtils.getTargetService(ExceptionConverter.class, ConstraintViolationExceptionConverter.class)
+ .refreshCode();
+ CommonExceptionData errorData = getExceptionData();
+
+ assertThat(errorData.getCode()).isEqualTo("test.0001");
+ ArchaiusUtils.setProperty(KEY_CODE, null);
+ }
+
+ @Test
+ public void should_use_json_property_value_as_property_name() {
+ CommonExceptionData errorData = getExceptionData();
+ List<ValidateDetail> details = errorData.getDynamic("validateDetail");
+
+ assertThat(details.stream().map(ValidateDetail::getPropertyPath))
+ .contains("op.model.nick-name");
+ }
+
+ @Test
+ public void should_include_all_validate_detail() {
+ CommonExceptionData errorData = getExceptionData();
+ List<ValidateDetail> details = errorData.getDynamic("validateDetail");
+
+ assertThat(details.stream().map(ValidateDetail::getPropertyPath))
+ .contains("op.query", "op.model.name", "op.model.nick-name");
+ assertThat(details.stream().map(ValidateDetail::getMessage))
+ .contains("not null", "can not be null", "can not be blank");
+ }
+}
\ No newline at end of file
diff --git a/dependencies/default/pom.xml b/dependencies/default/pom.xml
index 1ec1c2f..f2e80e2 100644
--- a/dependencies/default/pom.xml
+++ b/dependencies/default/pom.xml
@@ -58,7 +58,7 @@
<hamcrest.version>1.3</hamcrest.version>
<hdr-histogram.version>2.1.10</hdr-histogram.version>
<hibernate.version>6.0.2.Final</hibernate.version>
- <hibernate-validator.version>6.0.14.Final</hibernate-validator.version>
+ <hibernate-validator.version>6.1.5.Final</hibernate-validator.version>
<httpcomponents.version>4.5.7</httpcomponents.version>
<hystrix.version>1.5.18</hystrix.version>
<jackson.version>2.10.0</jackson.version>