[ 
https://issues.apache.org/jira/browse/SCB-1054?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16701948#comment-16701948
 ] 

ASF GitHub Bot commented on SCB-1054:
-------------------------------------

heyile 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..d08d43fb6 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
@@ -39,6 +39,7 @@
 import org.slf4j.LoggerFactory;
 
 import io.swagger.models.Operation;
+import io.swagger.models.Response;
 import io.swagger.models.Swagger;
 import io.swagger.models.parameters.Parameter;
 import io.vertx.ext.web.impl.MimeTypesUtils;
@@ -52,6 +53,9 @@
 
   protected boolean formData;
 
+  // make sure if response is file
+  protected boolean downloadFile;
+
   protected List<RestParam> paramList = new ArrayList<>();
 
   // key为参数名
@@ -80,6 +84,7 @@ public void init(OperationMeta operationMeta) {
       this.produces = swagger.getProduces();
     }
 
+    checkDownloadFile(operation);
     this.createProduceProcessors();
 
     Method method = operationMeta.getMethod();
@@ -105,6 +110,16 @@ public void init(OperationMeta operationMeta) {
     setAbsolutePath(concatPath(swagger.getBasePath(), 
operationMeta.getOperationPath()));
   }
 
+  private void checkDownloadFile(Operation operation) {
+    try {
+      Response response = operation.getResponses().get("200");
+      downloadFile = 
response.getSchema().getType().toLowerCase().equals("file");
+    } catch (Exception e) {
+      // if throw NullPointer Exception, set false
+      downloadFile = false;
+    }
+  }
+
   public boolean isFormData() {
     return formData;
   }
@@ -217,9 +232,14 @@ public ProduceProcessor ensureFindProduceProcessor(String 
acceptType) {
     if (StringUtils.isEmpty(acceptType)) {
       return defaultProcessor;
     }
-
-    List<String> mimeTyps = 
MimeTypesUtils.getSortedAcceptableMimeTypes(acceptType.toLowerCase(Locale.US));
-    for (String mime : mimeTyps) {
+    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 
application/json processor
+      //so do not care what accept type the consumer will set.
+      return this.produceProcessorMap.get(MediaType.WILDCARD);
+    }
+    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..414b4c1f0 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);
@@ -114,24 +137,40 @@ public void runRest() {
       futures.add(checkFile(part));
 
       part = templateGet("chineseAndSpaceFile");
+      ReadStreamPart part2 = templateExchange("chineseAndSpaceFile", 
MediaType.TEXT_PLAIN_VALUE);
+      ReadStreamPart part3 = templateExchange("chineseAndSpaceFile", 
MediaType.APPLICATION_JSON_VALUE);
       Assert.assertEquals("测 试.test.txt", part.getSubmittedFileName());
+      Assert.assertEquals("测 试.test.txt", part2.getSubmittedFileName());
+      Assert.assertEquals("测 试.test.txt", part3.getSubmittedFileName());
       futures.add(checkFuture(part.saveAsString()));
+      futures.add(checkFuture(part2.saveAsString()));
+      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 b1123b60f..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,13 +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());
-    conn.disconnect();
-    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:
[email protected]


> when download file, we should ignore consumer acceptType
> --------------------------------------------------------
>
>                 Key: SCB-1054
>                 URL: https://issues.apache.org/jira/browse/SCB-1054
>             Project: Apache ServiceComb
>          Issue Type: Bug
>            Reporter: 何一乐
>            Assignee: 何一乐
>            Priority: Critical
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to