[GitHub] wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest vertx file download

2018-04-19 Thread GitBox
wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest 
vertx file download
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/653#discussion_r182644564
 
 

 ##
 File path: 
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/inner/ServerRestArgsFilter.java
 ##
 @@ -50,7 +54,7 @@ public Response afterReceiveRequest(Invocation invocation, 
HttpServletRequestEx
   }
 
   @Override
-  public void beforeSendResponse(Invocation invocation, HttpServletResponseEx 
responseEx) {
+  public CompletableFuture asyncBeforeSendResponse(Invocation 
invocation, HttpServletResponseEx responseEx) {
 
 Review comment:
   done.


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


With regards,
Apache Git Services


[GitHub] wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest vertx file download

2018-04-19 Thread GitBox
wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest 
vertx file download
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/653#discussion_r182643228
 
 

 ##
 File path: 
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")
 
 Review comment:
   yes, already declared in this commit comments
   this PR just for producer
   
   https://issues.apache.org/jira/browse/SCB-487 is for consumer


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


With regards,
Apache Git Services


[GitHub] wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest vertx file download

2018-04-17 Thread GitBox
wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest 
vertx file download
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/653#discussion_r182296939
 
 

 ##
 File path: 
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/stream/InputStreamToReadStream.java
 ##
 @@ -34,7 +34,7 @@
 public class InputStreamToReadStream implements ReadStream {
   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;
 
 Review comment:
   only a internal buf size, customers not perceive this?


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


With regards,
Apache Git Services


[GitHub] wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest vertx file download

2018-04-17 Thread GitBox
wujimin commented on a change in pull request #653: [SCB-483] Springmvc rest 
vertx file download
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/653#discussion_r182294530
 
 

 ##
 File path: 
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
 ##
 @@ -220,10 +216,38 @@ protected void sendResponse(Response response) throws 
Exception {
 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 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
 
 Review comment:
   traced by https://issues.apache.org/jira/browse/SCB-500


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


With regards,
Apache Git Services