You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/12/05 06:32:50 UTC

[GitHub] wujimin closed pull request #1016: [SCB-1054]when download file, we should ignore consumer acceptType

wujimin closed pull request #1016: [SCB-1054]when download file, we should ignore consumer acceptType
URL: https://github.com/apache/servicecomb-java-chassis/pull/1016
 
 
   

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-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java
index ee7c40137..99d7a8970 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java
@@ -25,6 +25,7 @@
 import java.util.Locale;
 import java.util.Map;
 
+import javax.servlet.http.Part;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 
@@ -35,9 +36,12 @@
 import org.apache.servicecomb.common.rest.definition.path.URLPathBuilder;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
+import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.databind.JavaType;
+
 import io.swagger.models.Operation;
 import io.swagger.models.Swagger;
 import io.swagger.models.parameters.Parameter;
@@ -52,6 +56,9 @@
 
   protected boolean formData;
 
+  // make sure if response is file
+  protected boolean downloadFile;
+
   protected List<RestParam> paramList = new ArrayList<>();
 
   // key为参数名
@@ -80,13 +87,14 @@ public void init(OperationMeta operationMeta) {
       this.produces = swagger.getProduces();
     }
 
+    this.downloadFile = checkDownloadFileFlag();
     this.createProduceProcessors();
 
     Method method = operationMeta.getMethod();
     Type[] genericParamTypes = method.getGenericParameterTypes();
     if (genericParamTypes.length != operation.getParameters().size()) {
       throw new Error("Param count is not equal between swagger and method, path=" + absolutePath
-        + ";operation=" + operationMeta.getMicroserviceQualifiedName());
+          + ";operation=" + operationMeta.getMicroserviceQualifiedName());
     }
 
     // 初始化所有rest param
@@ -105,6 +113,15 @@ public void init(OperationMeta operationMeta) {
     setAbsolutePath(concatPath(swagger.getBasePath(), operationMeta.getOperationPath()));
   }
 
+  private boolean checkDownloadFileFlag() {
+    ResponseMeta responseMeta = operationMeta.findResponseMeta(200);
+    if (responseMeta != null) {
+      JavaType javaType = responseMeta.getJavaType();
+      return javaType.getRawClass().equals(Part.class);
+    }
+    return false;
+  }
+
   public boolean isFormData() {
     return formData;
   }
@@ -214,12 +231,17 @@ public ProduceProcessor ensureFindProduceProcessor(HttpServletRequestEx requestE
   }
 
   public ProduceProcessor ensureFindProduceProcessor(String acceptType) {
+    if (downloadFile) {
+      //do not check accept type, when the produces of provider is text/plain there will return text/plain processor
+      //when the produces of provider is application/json there will return the application/json processor
+      //so do not care what accept type the consumer will set.
+      return this.produceProcessorMap.get(MediaType.WILDCARD);
+    }
     if (StringUtils.isEmpty(acceptType)) {
       return defaultProcessor;
     }
-
-    List<String> mimeTyps = MimeTypesUtils.getSortedAcceptableMimeTypes(acceptType.toLowerCase(Locale.US));
-    for (String mime : mimeTyps) {
+    List<String> mimeTypes = MimeTypesUtils.getSortedAcceptableMimeTypes(acceptType.toLowerCase(Locale.US));
+    for (String mime : mimeTypes) {
       ProduceProcessor processor = this.produceProcessorMap.get(mime);
       if (null != processor) {
         return processor;
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/TestRestOperationMeta.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/TestRestOperationMeta.java
index 34621d6e2..623e1171a 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/TestRestOperationMeta.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/TestRestOperationMeta.java
@@ -171,6 +171,20 @@ public void testEnsureFindProduceProcessorAcceptFound() {
         operationMeta.ensureFindProduceProcessor("text/plain;q=0.7;charset=utf-8,application/json;q=0.8"));
   }
 
+  @Test
+  public void testEnsureFindProduceProcessorWithDownload() {
+    RestOperationMeta operationMeta = new RestOperationMeta();
+    operationMeta.produces = Arrays.asList(MediaType.APPLICATION_JSON);
+    operationMeta.downloadFile = true;
+    operationMeta.createProduceProcessors();
+
+    Assert.assertSame(ProduceProcessorManager.JSON_PROCESSOR,
+        operationMeta.ensureFindProduceProcessor("text/plain"));
+
+    operationMeta.downloadFile = false;
+    Assert.assertNull(operationMeta.ensureFindProduceProcessor("text/plain"));
+  }
+
   @Test
   public void testEnsureFindProduceProcessorAcceptNotFound() {
     RestOperationMeta operationMeta = new RestOperationMeta();
diff --git a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDownload.java b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDownload.java
index 9a3947c5f..a370e4953 100644
--- a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDownload.java
+++ b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDownload.java
@@ -32,6 +32,11 @@
 import org.apache.servicecomb.it.testcase.support.DownloadSchemaIntf;
 import org.junit.Assert;
 import org.junit.Test;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
 
 import com.google.common.collect.Iterables;
 
@@ -96,17 +101,35 @@ private ReadStreamPart templateGet(String methodPath) {
             content);
   }
 
+  private ReadStreamPart templateExchange(String methodPath, String type) {
+    HttpHeaders headers = new HttpHeaders();
+    headers.add("accept", type);
+    HttpEntity<?> entity = new HttpEntity<>(headers);
+    ResponseEntity<ReadStreamPart> response = consumers.getSCBRestTemplate()
+        .exchange("/" + methodPath + "?content={content}",
+            HttpMethod.GET,
+            entity,
+            ReadStreamPart.class, content);
+    return response.getBody();
+  }
+
   @Test
   @SuppressWarnings("unchecked")
   public void runRest() {
     futures.add(checkFile(consumers.getIntf().tempFileEntity(content)));
     futures.add(checkFuture(templateGet("tempFileEntity").saveAsBytes()));
+    futures.add(checkFuture(templateExchange("tempFileEntity", MediaType.TEXT_PLAIN_VALUE).saveAsBytes()));
+    futures.add(checkFuture(templateExchange("tempFileEntity", MediaType.APPLICATION_JSON_VALUE).saveAsBytes()));
 
     futures.add(checkFile(consumers.getIntf().tempFilePart(content)));
     futures.add(checkFuture(templateGet("tempFilePart").saveAsString()));
+    futures.add(checkFuture(templateExchange("tempFilePart", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("tempFilePart", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     futures.add(checkFile(consumers.getIntf().file(content)));
     futures.add(checkFuture(templateGet("file").saveAsString()));
+    futures.add(checkFuture(templateExchange("file", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("file", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     {
       ReadStreamPart part = consumers.getIntf().chineseAndSpaceFile(content);
@@ -116,22 +139,40 @@ public void runRest() {
       part = templateGet("chineseAndSpaceFile");
       Assert.assertEquals("测 试.test.txt", part.getSubmittedFileName());
       futures.add(checkFuture(part.saveAsString()));
+
+      ReadStreamPart part2 = templateExchange("chineseAndSpaceFile", MediaType.TEXT_PLAIN_VALUE);
+      Assert.assertEquals("测 试.test.txt", part2.getSubmittedFileName());
+      futures.add(checkFuture(part2.saveAsString()));
+
+      ReadStreamPart part3 = templateExchange("chineseAndSpaceFile", MediaType.APPLICATION_JSON_VALUE);
+      Assert.assertEquals("测 试.test.txt", part3.getSubmittedFileName());
+      futures.add(checkFuture(part3.saveAsString()));
     }
 
     futures.add(checkFile(consumers.getIntf().resource(content)));
     futures.add(checkFuture(templateGet("resource").saveAsString()));
+    futures.add(checkFuture(templateExchange("resource", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("resource", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     futures.add(checkFile(consumers.getIntf().entityResource(content)));
     futures.add(checkFuture(templateGet("entityResource").saveAsString()));
+    futures.add(checkFuture(templateExchange("entityResource", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("entityResource", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     futures.add(checkFile(consumers.getIntf().entityInputStream(content)));
     futures.add(checkFuture(templateGet("entityInputStream").saveAsString()));
+    futures.add(checkFuture(templateExchange("entityInputStream", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("entityInputStream", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     futures.add(checkFile(consumers.getIntf().bytes(content)));
     futures.add(checkFuture(templateGet("bytes").saveAsString()));
+    futures.add(checkFuture(templateExchange("bytes", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("bytes", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     futures.add(checkFile(consumers.getIntf().netInputStream(content)));
     futures.add(checkFuture(templateGet("netInputStream").saveAsString()));
+    futures.add(checkFuture(templateExchange("netInputStream", MediaType.TEXT_PLAIN_VALUE).saveAsString()));
+    futures.add(checkFuture(templateExchange("netInputStream", MediaType.APPLICATION_JSON_VALUE).saveAsString()));
 
     try {
       CompletableFuture
diff --git a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java
index 5ba7d8b85..2f3ba5420 100644
--- a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java
+++ b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java
@@ -192,12 +192,11 @@ public String getFilename() {
     URL url = new URL("http://localhost:" + server.getLocalPort() + "/download/netInputStream?content="
         + URLEncoder.encode(content, StandardCharsets.UTF_8.name()));
     HttpURLConnection conn = (HttpURLConnection) url.openConnection();
-    ResponseEntity<InputStream> responseEntity = ResponseEntity
+    return ResponseEntity
         .ok()
         .header(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_PLAIN_VALUE)
         .header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=netInputStream.txt")
         .body(conn.getInputStream());
-    return responseEntity;
   }
 
   private Thread slowInputStreamThread;


 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services