[ https://issues.apache.org/jira/browse/SCB-483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16445115#comment-16445115 ]
ASF GitHub Bot commented on SCB-483: ------------------------------------ liubao68 closed pull request #653: [SCB-483] Springmvc rest vertx file download URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/653 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/common/common-javassist/src/test/java/org/apache/servicecomb/common/javassist/TestJavassistUtils.java b/common/common-javassist/src/test/java/org/apache/servicecomb/common/javassist/TestJavassistUtils.java index 73c4c7ff1..a69eb0fd7 100644 --- a/common/common-javassist/src/test/java/org/apache/servicecomb/common/javassist/TestJavassistUtils.java +++ b/common/common-javassist/src/test/java/org/apache/servicecomb/common/javassist/TestJavassistUtils.java @@ -152,7 +152,7 @@ public void singleWrapperInt() throws Exception { public void multiWrapper() throws Exception { ClassConfig classConfig = new ClassConfig(); classConfig.setClassName("cse.ut.multi.Wrapper"); - classConfig.addField("intField", (Type) int.class); + classConfig.addField("intField", int.class); classConfig.addField("strField", String.class); JavassistUtils.genMultiWrapperInterface(classConfig); @@ -161,7 +161,7 @@ public void multiWrapper() throws Exception { MultiWrapper instance = (MultiWrapper) wrapperClass.newInstance(); instance.writeFields(new Object[] {100, "test"}); - Object[] fieldValues = (Object[]) instance.readFields(); + Object[] fieldValues = instance.readFields(); Assert.assertEquals(100, fieldValues[0]); Assert.assertEquals("test", fieldValues[1]); } diff --git a/common/common-protobuf/src/main/java/org/apache/servicecomb/codec/protobuf/utils/ProtobufSchemaUtils.java b/common/common-protobuf/src/main/java/org/apache/servicecomb/codec/protobuf/utils/ProtobufSchemaUtils.java index f1c985fed..8fd1e89dd 100644 --- a/common/common-protobuf/src/main/java/org/apache/servicecomb/codec/protobuf/utils/ProtobufSchemaUtils.java +++ b/common/common-protobuf/src/main/java/org/apache/servicecomb/codec/protobuf/utils/ProtobufSchemaUtils.java @@ -178,7 +178,7 @@ public static WrapSchema getOrCreateArgsSchema(OperationMeta operationMeta) { return getOrCreateSchema(type, () -> { if (!isArgsNeedWrap(method)) { // 可以直接使用 - Class<?> cls = (Class<?>) method.getParameterTypes()[0]; + Class<?> cls = method.getParameterTypes()[0]; Schema<?> schema = RuntimeSchema.createFrom(cls); return WrapSchemaFactory.createSchema(schema, WrapType.ARGS_NOT_WRAP); } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java index 3dfb5cc62..9a3769b88 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java @@ -17,11 +17,13 @@ package org.apache.servicecomb.common.rest; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.Response.Status; @@ -31,6 +33,7 @@ import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessorManager; import org.apache.servicecomb.common.rest.definition.RestOperationMeta; import org.apache.servicecomb.common.rest.filter.HttpServerFilter; +import org.apache.servicecomb.common.rest.filter.HttpServerFilterBeforeSendResponseExecutor; import org.apache.servicecomb.common.rest.locator.OperationLocator; import org.apache.servicecomb.common.rest.locator.ServicePathManager; import org.apache.servicecomb.core.Const; @@ -194,18 +197,11 @@ protected void sendResponseQuietly(Response response) { LOGGER.error("Failed to send rest response, operation:{}.", invocation.getMicroserviceQualifiedName(), e); - } finally { - requestEx.getAsyncContext().complete(); - // if failed to locate path, then will not create invocation - // TODO: statistics this case - if (invocation != null) { - invocation.onFinish(response); - } } } @SuppressWarnings("deprecation") - protected void sendResponse(Response response) throws Exception { + protected void sendResponse(Response response) { if (response.getHeaders().getHeaderMap() != null) { for (Entry<String, List<Object>> entry : response.getHeaders().getHeaderMap().entrySet()) { for (Object value : entry.getValue()) { @@ -216,14 +212,41 @@ protected void sendResponse(Response response) throws Exception { } } responseEx.setStatus(response.getStatusCode(), response.getReasonPhrase()); - responseEx.setContentType(produceProcessor.getName() + "; charset=utf-8"); responseEx.setAttribute(RestConst.INVOCATION_HANDLER_RESPONSE, response); responseEx.setAttribute(RestConst.INVOCATION_HANDLER_PROCESSOR, produceProcessor); - for (HttpServerFilter filter : httpServerFilters) { - filter.beforeSendResponse(invocation, responseEx); + executeHttpServerFilters(response); + } + + protected void executeHttpServerFilters(Response response) { + HttpServerFilterBeforeSendResponseExecutor exec = + new HttpServerFilterBeforeSendResponseExecutor(httpServerFilters, invocation, responseEx); + CompletableFuture<Void> future = exec.run(); + future.whenComplete((v, e) -> { + onExecuteHttpServerFiltersFinish(response, e); + }); + } + + protected void onExecuteHttpServerFiltersFinish(Response response, Throwable e) { + if (e != null) { + LOGGER.error("Failed to execute HttpServerFilters, operation:{}.", + invocation.getMicroserviceQualifiedName(), + e); } - responseEx.flushBuffer(); + try { + responseEx.flushBuffer(); + } catch (IOException flushException) { + LOGGER.error("Failed to flush rest response, operation:{}.", + invocation.getMicroserviceQualifiedName(), + flushException); + } + + requestEx.getAsyncContext().complete(); + // if failed to locate path, then will not create invocation + // TODO: statistics this case + if (invocation != null) { + invocation.onFinish(response); + } } } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java index c38b038c2..95659e8dc 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java @@ -29,6 +29,6 @@ public PathVarParamWriter(RestParam param) { @Override public void write(StringBuilder builder, Object[] args) throws Exception { - builder.append((Object) getParamValue(args)); + builder.append(getParamValue(args)); } } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java index af0f6b1c8..8e56e9956 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java @@ -17,6 +17,8 @@ package org.apache.servicecomb.common.rest.filter; +import java.util.concurrent.CompletableFuture; + import org.apache.servicecomb.core.Invocation; import org.apache.servicecomb.core.definition.OperationMeta; import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx; @@ -30,10 +32,30 @@ default boolean needCacheRequest(OperationMeta operationMeta) { return false; } - // if finished, then return a none null response - // if return a null response, then sdk will call next filter.afterReceiveRequest + /** + * @return if finished, then return a none null response<br> + * if return a null response, then sdk will call next filter.afterReceiveRequest + */ Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx requestEx); - // invocation maybe null - void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx); + /** + * @param invocation maybe null + */ + default CompletableFuture<Void> beforeSendResponseAsync(Invocation invocation, HttpServletResponseEx responseEx) { + CompletableFuture<Void> future = new CompletableFuture<>(); + try { + beforeSendResponse(invocation, responseEx); + future.complete(null); + } catch (Throwable e) { + future.completeExceptionally(e); + } + return future; + } + + /** + * @param invocation maybe null + */ + default void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) { + + } } diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java new file mode 100644 index 000000000..b07485a0f --- /dev/null +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java @@ -0,0 +1,78 @@ +/* + * 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.common.rest.filter; + +import java.util.List; +import java.util.concurrent.CompletableFuture; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx; + +public class HttpServerFilterBeforeSendResponseExecutor { + private List<HttpServerFilter> httpServerFilters; + + private Invocation invocation; + + private HttpServletResponseEx responseEx; + + private int currentIndex; + + private CompletableFuture<Void> future = new CompletableFuture<Void>(); + + public HttpServerFilterBeforeSendResponseExecutor(List<HttpServerFilter> httpServerFilters, Invocation invocation, + HttpServletResponseEx responseEx) { + this.httpServerFilters = httpServerFilters; + this.invocation = invocation; + this.responseEx = responseEx; + } + + public CompletableFuture<Void> run() { + doRun(); + + return future; + } + + protected CompletableFuture<Void> safeInvoke(HttpServerFilter httpServerFilter) { + try { + return httpServerFilter.beforeSendResponseAsync(invocation, responseEx); + } catch (Throwable e) { + CompletableFuture<Void> eFuture = new CompletableFuture<Void>(); + eFuture.completeExceptionally(e); + return eFuture; + } + } + + protected void doRun() { + if (currentIndex == httpServerFilters.size()) { + future.complete(null); + return; + } + + HttpServerFilter httpServerFilter = httpServerFilters.get(currentIndex); + currentIndex++; + + CompletableFuture<Void> stepFuture = safeInvoke(httpServerFilter); + stepFuture.whenComplete((v, e) -> { + if (e == null) { + doRun(); + return; + } + + future.completeExceptionally(e); + }); + } +} diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/inner/ServerRestArgsFilter.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/inner/ServerRestArgsFilter.java index b690cbf66..c806fc0ce 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/inner/ServerRestArgsFilter.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/inner/ServerRestArgsFilter.java @@ -17,6 +17,10 @@ package org.apache.servicecomb.common.rest.filter.inner; +import java.util.concurrent.CompletableFuture; + +import javax.servlet.http.Part; + import org.apache.servicecomb.common.rest.RestConst; import org.apache.servicecomb.common.rest.codec.RestCodec; import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor; @@ -50,7 +54,7 @@ public Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx } @Override - public void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) { + public CompletableFuture<Void> beforeSendResponseAsync(Invocation invocation, HttpServletResponseEx responseEx) { Response response = (Response) responseEx.getAttribute(RestConst.INVOCATION_HANDLER_RESPONSE); ProduceProcessor produceProcessor = (ProduceProcessor) responseEx.getAttribute(RestConst.INVOCATION_HANDLER_PROCESSOR); @@ -59,13 +63,21 @@ public void beforeSendResponse(Invocation invocation, HttpServletResponseEx resp body = ((InvocationException) body).getErrorData(); } + if (Part.class.isInstance(body)) { + return responseEx.sendPart((Part) body); + } + + responseEx.setContentType(produceProcessor.getName() + "; charset=utf-8"); + + CompletableFuture<Void> future = new CompletableFuture<Void>(); try (BufferOutputStream output = new BufferOutputStream(Unpooled.compositeBuffer())) { produceProcessor.encodeResponse(output, body); responseEx.setBodyBuffer(output.getBuffer()); + future.complete(null); } catch (Throwable e) { - throw ExceptionFactory.convertProducerException(e); + future.completeExceptionally(ExceptionFactory.convertProducerException(e)); } + return future; } - } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java index 72d0b1344..811ea47f3 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java @@ -29,6 +29,7 @@ import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessorManager; import org.apache.servicecomb.common.rest.definition.RestOperationMeta; import org.apache.servicecomb.common.rest.filter.HttpServerFilter; +import org.apache.servicecomb.common.rest.filter.HttpServerFilterBaseForTest; import org.apache.servicecomb.common.rest.locator.OperationLocator; import org.apache.servicecomb.common.rest.locator.ServicePathManager; import org.apache.servicecomb.core.Const; @@ -403,6 +404,7 @@ protected void doInvoke() { @Override protected void sendResponse(Response response) { result.value = response; + super.sendResponse(response); } }; initRestInvocation(); @@ -443,7 +445,7 @@ public void sendResponseStatusAndContentTypeAndHeader(@Mocked Response response) response.getReasonPhrase(); result = "reason"; response.getResult(); - result = new Error("stop"); + result = "result"; } }; @@ -480,12 +482,8 @@ void setContentType(String type) { initRestInvocation(); - try { - restInvocation.sendResponse(response); - Assert.fail("must throw exception"); - } catch (Error e) { - Assert.assertEquals(expected, result); - } + restInvocation.sendResponse(response); + Assert.assertEquals(expected, result); } @Test @@ -648,12 +646,12 @@ void setBodyBuffer(Buffer bodyBuffer) { } }.getMockInstance(); - HttpServerFilter filter = new MockUp<HttpServerFilter>() { - @Mock - void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) { + HttpServerFilter filter = new HttpServerFilterBaseForTest() { + @Override + public void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) { buffer.appendString("-filter"); } - }.getMockInstance(); + }; initRestInvocation(); List<HttpServerFilter> httpServerFilters = SPIServiceUtils.loadSortedService(HttpServerFilter.class); diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java new file mode 100644 index 000000000..24ccd95a8 --- /dev/null +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java @@ -0,0 +1,33 @@ +/* + * 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.common.rest.filter; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx; +import org.apache.servicecomb.swagger.invocation.Response; + +public class HttpServerFilterBaseForTest implements HttpServerFilter { + @Override + public int getOrder() { + return 0; + } + + @Override + public Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx requestEx) { + return null; + } +} diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.java new file mode 100644 index 000000000..38c1f97c7 --- /dev/null +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.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.common.rest.filter; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class TestHttpServerFilter { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void asyncSucc() throws InterruptedException, ExecutionException { + HttpServerFilter filter = new HttpServerFilterBaseForTest(); + + CompletableFuture<Void> future = filter.beforeSendResponseAsync(null, null); + Assert.assertNull(future.get()); + } + + @Test + public void asyncFailed() throws InterruptedException, ExecutionException { + HttpServerFilter filter = new HttpServerFilterBaseForTest() { + @Override + public void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) { + throw new Error(""); + } + }; + + expectedException.expect(ExecutionException.class); + expectedException.expectCause(Matchers.instanceOf(Error.class)); + + CompletableFuture<Void> future = filter.beforeSendResponseAsync(null, null); + future.get(); + } +} diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java new file mode 100644 index 000000000..0708f8fd1 --- /dev/null +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java @@ -0,0 +1,78 @@ +/* + * 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.common.rest.filter; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import mockit.Mocked; + +public class TestHttpServerFilterBeforeSendResponseExecutor { + @Mocked + Invocation invocation; + + @Mocked + HttpServletResponseEx responseEx; + + List<HttpServerFilter> httpServerFilters = new ArrayList<>(); + + HttpServerFilterBeforeSendResponseExecutor executor = + new HttpServerFilterBeforeSendResponseExecutor(httpServerFilters, invocation, responseEx); + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Before + public void setup() { + httpServerFilters.add(new HttpServerFilterBaseForTest()); + } + + @Test + public void runSucc() throws InterruptedException, ExecutionException { + CompletableFuture<Void> result = executor.run(); + + Assert.assertNull(result.get()); + } + + @Test + public void runFail() throws InterruptedException, ExecutionException { + httpServerFilters.add(new HttpServerFilterBaseForTest() { + @Override + public CompletableFuture<Void> beforeSendResponseAsync(Invocation invocation, HttpServletResponseEx responseEx) { + throw new Error(""); + } + }); + + CompletableFuture<Void> result = executor.run(); + + expectedException.expect(ExecutionException.class); + expectedException.expectCause(Matchers.instanceOf(Error.class)); + + result.get(); + } +} diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/inner/TestServerRestArgsFilter.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/inner/TestServerRestArgsFilter.java new file mode 100644 index 000000000..8a322e07d --- /dev/null +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/inner/TestServerRestArgsFilter.java @@ -0,0 +1,73 @@ +/* + * 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.common.rest.filter.inner; + +import java.util.concurrent.CompletableFuture; + +import javax.servlet.http.Part; + +import org.apache.servicecomb.common.rest.RestConst; +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx; +import org.apache.servicecomb.swagger.invocation.Response; +import org.junit.Assert; +import org.junit.Test; + +import mockit.Expectations; +import mockit.Mock; +import mockit.MockUp; +import mockit.Mocked; + +public class TestServerRestArgsFilter { + @Mocked + Invocation invocation; + + @Mocked + HttpServletResponseEx responseEx; + + @Mocked + Response response; + + @Mocked + Part part; + + boolean invokedSendPart; + + ServerRestArgsFilter filter = new ServerRestArgsFilter(); + + @Test + public void asyncBeforeSendResponse_part() { + new Expectations() { + { + responseEx.getAttribute(RestConst.INVOCATION_HANDLER_RESPONSE); + result = response; + response.getResult(); + result = part; + } + }; + new MockUp<HttpServletResponseEx>(responseEx) { + @Mock + CompletableFuture<Void> sendPart(Part body) { + invokedSendPart = true; + return null; + } + }; + + Assert.assertNull(filter.beforeSendResponseAsync(invocation, responseEx)); + Assert.assertTrue(invokedSendPart); + } +} diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/DownloadSchema.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/DownloadSchema.java new file mode 100644 index 000000000..0fbd6ee69 --- /dev/null +++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/DownloadSchema.java @@ -0,0 +1,79 @@ +/* + * 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.demo.springmvc.server; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import org.apache.servicecomb.provider.rest.common.RestSchema; +import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.Resource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; + +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; + +@RestSchema(schemaId = "download") +@RequestMapping(path = "/download", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) +public class DownloadSchema { + @GetMapping(path = "/file") + public File downloadFile() throws IOException { + return new File(this.getClass().getClassLoader().getResource("microservice.yaml").getFile()); + } + + @GetMapping(path = "/resource") + @ApiResponses({ + @ApiResponse(code = 200, response = File.class, message = ""), + }) + public Resource downloadResource() throws IOException { + return new ByteArrayResource("abc".getBytes(StandardCharsets.UTF_8)) { + @Override + public String getFilename() { + return "abc.txt"; + } + }; + } + + @GetMapping(path = "/entityResource") + @ApiResponses({ + @ApiResponse(code = 200, response = File.class, message = ""), + }) + public ResponseEntity<Resource> downloadEntityResource() throws IOException { + return ResponseEntity + .ok() + .header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=entityResource.txt") + .body(new ByteArrayResource("entityResource".getBytes(StandardCharsets.UTF_8))); + } + + @GetMapping(path = "/entityInputStream") + @ApiResponses({ + @ApiResponse(code = 200, response = File.class, message = ""), + }) + public ResponseEntity<InputStream> downloadEntityInputStream() throws IOException { + return ResponseEntity + .ok() + .header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=entityInputStream.txt") + .body(new ByteArrayInputStream("entityInputStream".getBytes(StandardCharsets.UTF_8))); + } +} diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/AbstractHttpServletResponse.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/AbstractHttpServletResponse.java index a09e77380..21404299d 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/AbstractHttpServletResponse.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/AbstractHttpServletResponse.java @@ -23,9 +23,11 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; +import java.util.concurrent.CompletableFuture; import javax.servlet.ServletOutputStream; import javax.servlet.http.Cookie; +import javax.servlet.http.Part; import javax.ws.rs.core.Response.StatusType; public abstract class AbstractHttpServletResponse extends BodyBufferSupportImpl implements HttpServletResponseEx { @@ -230,4 +232,9 @@ public void setAttribute(String key, Object value) { public Object getAttribute(String key) { return this.attributes.get(key); } + + @Override + public CompletableFuture<Void> sendPart(Part body) { + throw new Error("not supported method"); + } } diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/HttpServletResponseEx.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/HttpServletResponseEx.java index 9749f4b8e..a1ee34ec9 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/HttpServletResponseEx.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/HttpServletResponseEx.java @@ -17,7 +17,10 @@ package org.apache.servicecomb.foundation.vertx.http; +import java.util.concurrent.CompletableFuture; + import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.Part; import javax.ws.rs.core.Response.StatusType; public interface HttpServletResponseEx extends HttpServletResponse, BodyBufferSupport { @@ -26,4 +29,6 @@ void setAttribute(String key, Object value); Object getAttribute(String key); + + CompletableFuture<Void> sendPart(Part body); } diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/StandardHttpServletResponseEx.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/StandardHttpServletResponseEx.java index c2c140863..13c0ba710 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/StandardHttpServletResponseEx.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/StandardHttpServletResponseEx.java @@ -20,9 +20,11 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CompletableFuture; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; +import javax.servlet.http.Part; import javax.ws.rs.core.Response.StatusType; import org.apache.servicecomb.foundation.common.http.HttpStatus; @@ -95,4 +97,9 @@ public void setAttribute(String key, Object value) { public Object getAttribute(String key) { return this.attributes.get(key); } + + @Override + public CompletableFuture<Void> sendPart(Part body) { + throw new Error("not supported method"); + } } diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java index ca5510c59..9f5db5549 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java @@ -18,17 +18,23 @@ package org.apache.servicecomb.foundation.vertx.http; import java.io.IOException; +import java.io.InputStream; import java.util.Collection; import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import javax.servlet.http.Part; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.Response.StatusType; +import org.apache.commons.io.IOUtils; import org.apache.servicecomb.foundation.common.http.HttpStatus; +import org.apache.servicecomb.foundation.vertx.stream.InputStreamToReadStream; import io.vertx.core.Context; import io.vertx.core.Vertx; import io.vertx.core.http.HttpServerResponse; +import io.vertx.core.streams.Pump; public class VertxServerResponseToHttpServletResponse extends AbstractHttpServletResponse { private Context context; @@ -118,4 +124,50 @@ public void internalFlushBuffer() { serverResponse.end(bodyBuffer); } + + @Override + public CompletableFuture<Void> sendPart(Part part) { + CompletableFuture<Void> future = new CompletableFuture<Void>(); + + prepareSendPartHeader(part); + + try { + InputStream is = part.getInputStream(); + context.runOnContext(v -> { + InputStreamToReadStream aa = new InputStreamToReadStream(context.owner(), is); + aa.exceptionHandler(t -> { + clearPartResource(part, is); + future.completeExceptionally(t); + }); + aa.endHandler(V -> { + clearPartResource(part, is); + future.complete(null); + }); + Pump.pump(aa, serverResponse).start(); + }); + } catch (IOException e) { + future.completeExceptionally(e); + } + + return future; + } + + protected void prepareSendPartHeader(Part part) { + if (!serverResponse.headers().contains(HttpHeaders.CONTENT_LENGTH)) { + serverResponse.setChunked(true); + } + + if (!serverResponse.headers().contains(HttpHeaders.CONTENT_TYPE)) { + serverResponse.putHeader(HttpHeaders.CONTENT_TYPE, part.getContentType()); + } + + if (!serverResponse.headers().contains(HttpHeaders.CONTENT_DISPOSITION)) { + serverResponse.putHeader(HttpHeaders.CONTENT_DISPOSITION, + "attachment;filename=" + part.getSubmittedFileName()); + } + } + + protected void clearPartResource(Part part, InputStream is) { + IOUtils.closeQuietly(is); + } } diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/stream/InputStreamToReadStream.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/stream/InputStreamToReadStream.java index 2b94af4af..2066dfe9c 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/stream/InputStreamToReadStream.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/stream/InputStreamToReadStream.java @@ -34,7 +34,7 @@ public class InputStreamToReadStream implements ReadStream<Buffer> { private static final Logger LOGGER = LoggerFactory.getLogger(InputStreamToReadStream.class); - public static final int DEFAULT_READ_BUFFER_SIZE = 8192; + public static final int DEFAULT_READ_BUFFER_SIZE = 1024 * 1024; private Vertx vertx; diff --git a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestAbstractHttpServletResponse.java b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestAbstractHttpServletResponse.java index 24627725e..ec884eeea 100644 --- a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestAbstractHttpServletResponse.java +++ b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestAbstractHttpServletResponse.java @@ -20,10 +20,12 @@ import java.io.IOException; import org.hamcrest.Matchers; +import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; + public class TestAbstractHttpServletResponse { @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -304,4 +306,17 @@ public void testGetStatusType() { response.getStatusType(); } + + @Test + public void attribute() { + response.setAttribute("k", "v"); + Assert.assertEquals("v", response.getAttribute("k")); + } + + @Test + public void sendPart() { + setExceptionExpected(); + + response.sendPart(null); + } } diff --git a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestStandardHttpServletResponseEx.java b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestStandardHttpServletResponseEx.java index 82853b234..00cb71e8e 100644 --- a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestStandardHttpServletResponseEx.java +++ b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestStandardHttpServletResponseEx.java @@ -23,9 +23,12 @@ import javax.servlet.WriteListener; import javax.servlet.http.HttpServletResponse; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import io.vertx.core.buffer.Buffer; import mockit.Mock; @@ -38,6 +41,14 @@ StandardHttpServletResponseEx responseEx; + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private void setExceptionExpected() { + expectedException.expect(Error.class); + expectedException.expectMessage(Matchers.is("not supported method")); + } + @Before public void setup() { responseEx = new StandardHttpServletResponseEx(response); @@ -114,4 +125,17 @@ ServletOutputStream getOutputStream() { responseEx.flushBuffer(); Assert.assertEquals("body", buffer.toString()); } + + @Test + public void attribute() { + responseEx.setAttribute("k", "v"); + Assert.assertEquals("v", responseEx.getAttribute("k")); + } + + @Test + public void sendPart() { + setExceptionExpected(); + + responseEx.sendPart(null); + } } diff --git a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java index d821d1412..c69d41cc2 100644 --- a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java +++ b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java @@ -18,7 +18,11 @@ package org.apache.servicecomb.foundation.vertx.http; import java.io.IOException; +import java.io.InputStream; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import javax.servlet.http.Part; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.Response.StatusType; @@ -30,9 +34,12 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import io.vertx.core.AsyncResult; import io.vertx.core.Context; +import io.vertx.core.Future; import io.vertx.core.Handler; import io.vertx.core.MultiMap; +import io.vertx.core.Vertx; import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpServerResponse; import io.vertx.core.impl.VertxImpl; @@ -55,12 +62,17 @@ boolean runOnContextInvoked; + @Mocked + Vertx vertx; + @Mocked Context context; @Rule public ExpectedException expectedException = ExpectedException.none(); + boolean chunked; + @Before public void setup() { serverResponse = new MockUp<HttpServerResponse>() { @@ -91,6 +103,12 @@ MultiMap headers() { return headers; } + @Mock + HttpServerResponse putHeader(String name, String value) { + headers.set(name, value); + return serverResponse; + } + @Mock void end() { flushWithBody = false; @@ -100,6 +118,17 @@ void end() { void end(Buffer chunk) { flushWithBody = true; } + + @Mock + HttpServerResponse setChunked(boolean chunked) { + TestVertxServerResponseToHttpServletResponse.this.chunked = chunked; + return serverResponse; + } + + @Mock + boolean isChunked() { + return chunked; + } }.getMockInstance(); new Expectations(VertxImpl.class) { @@ -115,6 +144,22 @@ void runOnContext(Handler<Void> action) { runOnContextInvoked = true; action.handle(null); } + + @Mock + Vertx owner() { + return vertx; + } + }; + + new MockUp<Vertx>(vertx) { + @Mock + <T> void executeBlocking(Handler<Future<T>> blockingCodeHandler, boolean ordered, + Handler<AsyncResult<T>> resultHandler) { + Future<T> future = Future.future(); + future.setHandler(resultHandler); + + blockingCodeHandler.handle(future); + } }; response = new VertxServerResponseToHttpServletResponse(serverResponse); @@ -246,4 +291,91 @@ public void internalFlushBufferWithBody() throws IOException { Assert.assertTrue(flushWithBody); } + + @Test + public void prepareSendPartHeader_update(@Mocked Part part) { + new Expectations() { + { + part.getContentType(); + result = "type"; + part.getSubmittedFileName(); + result = "name"; + } + }; + response.prepareSendPartHeader(part); + + Assert.assertTrue(serverResponse.isChunked()); + Assert.assertEquals("type", response.getHeader(HttpHeaders.CONTENT_TYPE)); + Assert.assertEquals("attachment;filename=name", response.getHeader(HttpHeaders.CONTENT_DISPOSITION)); + } + + @Test + public void prepareSendPartHeader_notUpdate(@Mocked Part part) { + headers.add(HttpHeaders.CONTENT_LENGTH, "10"); + headers.add(HttpHeaders.CONTENT_TYPE, "type"); + headers.add(HttpHeaders.CONTENT_DISPOSITION, "disposition"); + + response.prepareSendPartHeader(part); + + Assert.assertFalse(serverResponse.isChunked()); + Assert.assertEquals("type", response.getHeader(HttpHeaders.CONTENT_TYPE)); + Assert.assertEquals("disposition", response.getHeader(HttpHeaders.CONTENT_DISPOSITION)); + } + + @Test + public void sendPart_openInputStreamFailed(@Mocked Part part) + throws IOException, InterruptedException, ExecutionException { + IOException ioException = new IOException("forbid open stream"); + new Expectations() { + { + part.getInputStream(); + result = ioException; + } + }; + + CompletableFuture<Void> future = response.sendPart(part); + + expectedException.expect(ExecutionException.class); + expectedException.expectCause(Matchers.sameInstance(ioException)); + + future.get(); + } + + @Test + public void sendPart_inputStreamBreak(@Mocked Part part, @Mocked InputStream inputStream) + throws IOException, InterruptedException, ExecutionException { + IOException ioException = new IOException("forbid read"); + new Expectations() { + { + part.getInputStream(); + result = inputStream; + inputStream.read((byte[]) any); + result = ioException; + } + }; + + CompletableFuture<Void> future = response.sendPart(part); + + expectedException.expect(ExecutionException.class); + expectedException.expectCause(Matchers.sameInstance(ioException)); + + future.get(); + } + + @Test + public void sendPart_succ(@Mocked Part part, @Mocked InputStream inputStream) + throws IOException, InterruptedException, ExecutionException { + new Expectations() { + { + part.getInputStream(); + result = inputStream; + inputStream.read((byte[]) any); + result = -1; + } + }; + + CompletableFuture<Void> future = response.sendPart(part); + + Assert.assertNull(future.get()); + } } diff --git a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java index 64650de20..5e81ee1d8 100644 --- a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java +++ b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java @@ -26,10 +26,6 @@ import com.netflix.spectator.servo.ServoRegistry; public class DefaultRegistryInitializer implements MetricsInitializer { - public static final String METRICS_WINDOW_TIME = "servicecomb.metrics.window_time"; - - public static final int DEFAULT_METRICS_WINDOW_TIME = 5000; - public static final String SERVO_POLLERS = "servo.pollers"; private CompositeRegistry globalRegistry; ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > springmvc over transport rest vertx support file download > --------------------------------------------------------- > > Key: SCB-483 > URL: https://issues.apache.org/jira/browse/SCB-483 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis > Reporter: wujimin > Assignee: wujimin > Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)