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 16b60ea [SCB-1968] add producer operation filter
16b60ea is described below
commit 16b60ea5fd445e894e55b6bb7e0a793a4c406fa9
Author: wujimin <wuji...@huawei.com>
AuthorDate: Mon Jun 1 21:20:42 2020 +0800
[SCB-1968] add producer operation filter
---
.../IllegalArgumentExceptionConverter.java | 49 ++++++
.../core/filter/impl/DefaultFilterProvider.java | 3 +-
.../core/filter/impl/ProducerOperationFilter.java | 94 ++++++++++
...e.servicecomb.core.exception.ExceptionConverter | 1 +
.../servicecomb/core/filter/FilterChainTest.java | 11 +-
.../filter/impl/ProducerOperationFilterTest.java | 193 +++++++++++++++++++++
6 files changed, 344 insertions(+), 7 deletions(-)
diff --git
a/core/src/main/java/org/apache/servicecomb/core/exception/converter/IllegalArgumentExceptionConverter.java
b/core/src/main/java/org/apache/servicecomb/core/exception/converter/IllegalArgumentExceptionConverter.java
new file mode 100644
index 0000000..2e3ccb9
--- /dev/null
+++
b/core/src/main/java/org/apache/servicecomb/core/exception/converter/IllegalArgumentExceptionConverter.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.core.exception.converter;
+
+import javax.annotation.Nullable;
+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.InvocationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class IllegalArgumentExceptionConverter implements
ExceptionConverter<IllegalArgumentException> {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(IllegalArgumentExceptionConverter.class);
+
+ @Override
+ public int getOrder() {
+ return Short.MAX_VALUE;
+ }
+
+ @Override
+ public boolean canConvert(Throwable throwable) {
+ return throwable instanceof IllegalArgumentException;
+ }
+
+ @Override
+ public InvocationException convert(@Nullable Invocation invocation,
IllegalArgumentException throwable,
+ StatusType genericStatus) {
+ LOGGER.error("convert IllegalArgumentException exception to
InvocationException.", throwable);
+
+ return new InvocationException(genericStatus,
ExceptionConverter.getGenericCode(genericStatus),
+ "Parameters not valid or types not match.", throwable);
+ }
+}
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 e2a2054..ab4e4ac 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
@@ -27,6 +27,7 @@ public class DefaultFilterProvider implements FilterProvider {
public List<Class<? extends Filter>> getFilters() {
return Arrays.asList(
SimpleLoadBalanceFilter.class,
- ScheduleFilter.class);
+ ScheduleFilter.class,
+ ProducerOperationFilter.class);
}
}
diff --git
a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
new file mode 100644
index 0000000..8819522
--- /dev/null
+++
b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
@@ -0,0 +1,94 @@
+/*
+ * 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.concurrent.CompletableFuture;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.exception.Exceptions;
+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.apache.servicecomb.swagger.invocation.context.ContextUtils;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@FilterMeta(name = "producer-operation", invocationType = PRODUCER)
+public class ProducerOperationFilter implements Filter {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ProducerOperationFilter.class);
+
+ @Override
+ public CompletableFuture<Response> onFilter(Invocation invocation,
FilterNode nextNode) {
+ invocation.onBusinessMethodStart();
+
+ SwaggerProducerOperation producerOperation =
invocation.getOperationMeta().getSwaggerProducerOperation();
+ Object instance = producerOperation.getProducerInstance();
+ Method method = producerOperation.getProducerMethod();
+ Object[] args = invocation.toProducerArguments();
+ return invoke(invocation, instance, method, args)
+ .thenApply(result -> convertResultToResponse(invocation,
producerOperation, result))
+ .whenComplete((response, throwable) -> whenComplete(invocation,
throwable));
+ }
+
+ @SuppressWarnings("unchecked")
+ private CompletableFuture<Object> invoke(Invocation invocation, Object
instance, Method method, Object[] args) {
+ ContextUtils.setInvocationContext(invocation);
+
+ try {
+ Object result = method.invoke(instance, args);
+ if (result instanceof CompletableFuture) {
+ return (CompletableFuture<Object>) result;
+ }
+
+ return CompletableFuture.completedFuture(result);
+ } catch (Throwable e) {
+ return AsyncUtils.completeExceptionally(Exceptions.unwrap(e));
+ } finally {
+ ContextUtils.removeInvocationContext();
+ }
+ }
+
+ private Response convertResultToResponse(Invocation invocation,
SwaggerProducerOperation producerOperation,
+ Object result) {
+ return
producerOperation.getResponseMapper().mapResponse(invocation.getStatus(),
result);
+ }
+
+ private void whenComplete(Invocation invocation, Throwable throwable) {
+ if (shouldPrintErrorLog(throwable)) {
+ LOGGER.error("unexpected error {},",
invocation.getInvocationQualifiedName(), throwable);
+ }
+
+ invocation.onBusinessMethodFinish();
+ invocation.onBusinessFinish();
+ }
+
+ protected boolean shouldPrintErrorLog(Throwable throwable) {
+ if (throwable == null) {
+ return false;
+ }
+
+ Throwable unwrapped = Exceptions.unwrap(throwable);
+ return !(unwrapped instanceof InvocationException);
+ }
+}
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 798c7d1..f129ed1 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
@@ -16,4 +16,5 @@
#
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/FilterChainTest.java
b/core/src/test/java/org/apache/servicecomb/core/filter/FilterChainTest.java
index a7d75c1..d184fa4 100644
--- a/core/src/test/java/org/apache/servicecomb/core/filter/FilterChainTest.java
+++ b/core/src/test/java/org/apache/servicecomb/core/filter/FilterChainTest.java
@@ -139,14 +139,13 @@ public class FilterChainTest {
};
SimpleRetryFilter retryFilter = new SimpleRetryFilter().setMaxRetry(3);
- ExecutionException executionException = (ExecutionException)
catchThrowable(
- () -> FilterNode.buildChain(retryFilter, recordThreadFilter,
exceptionFilter)
- .onFilter(invocation)
- .get());
+ CompletableFuture<Response> future = FilterNode.buildChain(retryFilter,
recordThreadFilter, exceptionFilter)
+ .onFilter(invocation);
assertThat(msg).containsExactly("main", "main", "main");
- assertThat(executionException.getCause())
- .isInstanceOf(IOException.class)
+ assertThat(future)
+ .hasFailedWithThrowableThat()
+ .isExactlyInstanceOf(IOException.class)
.hasMessage("net error");
}
}
\ No newline at end of file
diff --git
a/core/src/test/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilterTest.java
b/core/src/test/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilterTest.java
new file mode 100644
index 0000000..0ed0c4b
--- /dev/null
+++
b/core/src/test/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilterTest.java
@@ -0,0 +1,193 @@
+/*
+ * 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.INTERNAL_SERVER_ERROR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.CompletableFuture;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.exception.Exceptions;
+import org.apache.servicecomb.core.filter.FilterNode;
+import
org.apache.servicecomb.foundation.test.scaffolding.exception.RuntimeExceptionWithoutStackTrace;
+import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation;
+import org.apache.servicecomb.swagger.invocation.Response;
+import org.apache.servicecomb.swagger.invocation.context.ContextUtils;
+import org.apache.servicecomb.swagger.invocation.context.InvocationContext;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.junit.Test;
+
+import mockit.Expectations;
+import mockit.Injectable;
+import mockit.Mocked;
+import mockit.Verifications;
+
+public class ProducerOperationFilterTest {
+ ProducerOperationFilter filter = new ProducerOperationFilter();
+
+ @Injectable
+ Invocation invocation;
+
+ @Mocked
+ SwaggerProducerOperation producerOperation;
+
+ static InvocationContext threadInvocationContext;
+
+ public static class Controller {
+ public void sync() {
+ threadInvocationContext = ContextUtils.getInvocationContext();
+ }
+
+ public void syncException() {
+ throw new RuntimeExceptionWithoutStackTrace("syncException");
+ }
+
+ public CompletableFuture<Void> async() {
+ threadInvocationContext = ContextUtils.getInvocationContext();
+ return CompletableFuture.completedFuture(null);
+ }
+
+ public CompletableFuture<Void> asyncException() {
+ throw new RuntimeExceptionWithoutStackTrace("asyncException");
+ }
+ }
+
+ @Test
+ public void should_record_invocation_trace_time() {
+ filter.onFilter(invocation, FilterNode.EMPTY);
+
+ new Verifications() {
+ {
+ invocation.onBusinessMethodStart();
+ times = 1;
+
+ invocation.onBusinessMethodFinish();
+ times = 1;
+
+ invocation.onBusinessFinish();
+ times = 1;
+ }
+ };
+ }
+
+ private void setInvokeMethod(String name) throws NoSuchMethodException {
+ Controller instance = new Controller();
+ Method method = instance.getClass().getMethod(name);
+ new Expectations() {
+ {
+ producerOperation.getProducerInstance();
+ result = instance;
+
+ producerOperation.getProducerMethod();
+ result = method;
+ }
+ };
+ }
+
+ private void setInvokeSyncMethod() throws NoSuchMethodException {
+ setInvokeMethod("sync");
+ }
+
+ private void setInvokeAsyncMethod() throws NoSuchMethodException {
+ setInvokeMethod("async");
+ }
+
+ @Test
+ public void should_provide_thread_local_invocation_context_for_sync_method()
throws NoSuchMethodException {
+ setInvokeSyncMethod();
+
+ filter.onFilter(invocation, FilterNode.EMPTY);
+
+ assertThat(threadInvocationContext).isSameAs(invocation);
+ }
+
+ @Test
+ public void should_clear_thread_local_invocation_context_after_sync_method()
throws NoSuchMethodException {
+ setInvokeSyncMethod();
+
+ filter.onFilter(invocation, FilterNode.EMPTY);
+
+ assertThat(ContextUtils.getInvocationContext()).isNull();
+ }
+
+ @Test
+ public void should_catch_sync_business_exception() throws
NoSuchMethodException {
+ setInvokeMethod("syncException");
+
+ CompletableFuture<Response> future = filter.onFilter(invocation,
FilterNode.EMPTY);
+
+ assertThat(future).hasFailedWithThrowableThat()
+ .isInstanceOf(RuntimeExceptionWithoutStackTrace.class)
+ .hasMessage("syncException");
+ }
+
+ @Test
+ public void
should_provide_thread_local_invocation_context_for_async_method() throws
NoSuchMethodException {
+ setInvokeAsyncMethod();
+
+ filter.onFilter(invocation, FilterNode.EMPTY);
+
+ assertThat(threadInvocationContext).isSameAs(invocation);
+ }
+
+ @Test
+ public void
should_clear_thread_local_invocation_context_after_async_method() throws
NoSuchMethodException {
+ setInvokeAsyncMethod();
+
+ filter.onFilter(invocation, FilterNode.EMPTY);
+
+ assertThat(ContextUtils.getInvocationContext()).isNull();
+ }
+
+ @Test
+ public void should_catch_async_business_exception() throws
NoSuchMethodException {
+ setInvokeMethod("asyncException");
+
+ CompletableFuture<Response> future = filter.onFilter(invocation,
FilterNode.EMPTY);
+
+ assertThat(future).hasFailedWithThrowableThat()
+ .isInstanceOf(RuntimeExceptionWithoutStackTrace.class)
+ .hasMessage("asyncException");
+ }
+
+ @Test
+ public void
should_unify_IllegalArgumentException_message_when_convert_exception() throws
NoSuchMethodException {
+ setInvokeSyncMethod();
+ new Expectations() {
+ {
+ invocation.toProducerArguments();
+ result = new Object[] {1};
+ }
+ };
+
+ CompletableFuture<Response> future = filter.onFilter(invocation,
FilterNode.EMPTY);
+ assertThat(future).hasFailedWithThrowableThat()
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("wrong number of arguments");
+
+ InvocationException throwable = Exceptions
+ .convert(invocation, catchThrowable(() -> future.get()),
INTERNAL_SERVER_ERROR);
+ assertThat(throwable).hasCauseInstanceOf(IllegalArgumentException.class);
+ CommonExceptionData data = (CommonExceptionData) throwable.getErrorData();
+ assertThat(data.getMessage()).isEqualTo("Parameters not valid or types not
match.");
+ }
+}
\ No newline at end of file
