You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2019/01/16 10:00:17 UTC
[servicecomb-java-chassis] 04/04: [SCB-1046] file upload support
file array for the same name:fix as reviewed
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
commit 1252f9c12f8d5219dac921b04495ae66974c5ef1
Author: heyile <25...@qq.com>
AuthorDate: Mon Jan 14 11:41:00 2019 +0800
[SCB-1046] file upload support file array for the same name:fix as reviewed
---
.../rest/codec/param/FormProcessorCreator.java | 13 ++--
.../rest/codec/param/RestClientRequestImpl.java | 71 +++++++---------------
.../common/rest/definition/RestOperationMeta.java | 10 +++
.../codec/param/TestRestClientRequestImpl.java | 9 ++-
.../VertxServerRequestToHttpServletRequest.java | 6 +-
.../reference/CommonToHttpServletRequest.java | 24 +++-----
.../springmvc/reference/CseClientHttpRequest.java | 2 +-
.../reference/TestCommonToHttpServletRequest.java | 49 +++------------
.../swagger/generator/core/OperationGenerator.java | 2 +-
.../swagger/invocation/converter/ConverterMgr.java | 2 +-
10 files changed, 65 insertions(+), 123 deletions(-)
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java
index 48de6ad..6980e94 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/FormProcessorCreator.java
@@ -18,7 +18,6 @@
package org.apache.servicecomb.common.rest.codec.param;
import java.lang.reflect.Type;
-import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@@ -29,7 +28,6 @@ import javax.ws.rs.core.Response.Status;
import org.apache.servicecomb.common.rest.RestConst;
import org.apache.servicecomb.common.rest.codec.RestClientRequest;
-import org.apache.servicecomb.foundation.vertx.http.FileUploadPart;
import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import com.fasterxml.jackson.databind.JavaType;
@@ -41,6 +39,7 @@ import io.swagger.models.properties.FileProperty;
import io.swagger.models.properties.Property;
public class FormProcessorCreator implements ParamValueProcessorCreator {
+
public static final String PARAMTYPE = "formData";
public static class FormProcessor extends AbstractParamProcessor {
@@ -108,9 +107,7 @@ public class FormProcessorCreator implements ParamValueProcessorCreator {
FormParameter formParameter = (FormParameter) parameter;
if ("array".equals(formParameter.getType())) {
Property items = formParameter.getItems();
- if (items != null) {
- return new FileProperty().getType().equals(items.getType());
- }
+ return new FileProperty().getType().equals(items.getType());
}
return new FileProperty().getType().equals(formParameter.getType());
}
@@ -126,10 +123,10 @@ public class FormProcessorCreator implements ParamValueProcessorCreator {
JavaType contentType = targetType.getContentType();
if (contentType != null && contentType.getRawClass().equals(Part.class)) {
//get all parts
- Collection<Part> parts = request.getParts();
- List<Part> collect = parts.stream().filter(part -> part.getName().equals(paramPath))
+ return request.getParts()
+ .stream()
+ .filter(part -> part.getName().equals(paramPath))
.collect(Collectors.toList());
- return collect;
}
}
return request.getPart(paramPath);
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
index fbf56f2..2bb92a9 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
@@ -22,7 +22,6 @@ import static javax.ws.rs.core.MediaType.MULTIPART_FORM_DATA;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -41,6 +40,9 @@ import org.apache.servicecomb.swagger.invocation.AsyncResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
+
import io.vertx.core.Context;
import io.vertx.core.MultiMap;
import io.vertx.core.buffer.Buffer;
@@ -54,8 +56,7 @@ public class RestClientRequestImpl implements RestClientRequest {
protected AsyncResponse asyncResp;
- // maybe part or list of parts
- private final Map<String, Object> uploads = new HashMap<>();
+ private final Multimap<String, Part> uploads = ArrayListMultimap.create();
protected HttpClientRequest request;
@@ -83,12 +84,19 @@ public class RestClientRequestImpl implements RestClientRequest {
}
@Override
+ @SuppressWarnings("unchecked")
public void attach(String name, Object part) {
if (null == part) {
LOGGER.debug("null file is ignored, file name = [{}]", name);
return;
}
- uploads.put(name, part);
+ if (List.class.isAssignableFrom(part.getClass())) {
+ List<Part> parts = (List<Part>) part;
+ uploads.putAll(name, parts);
+ return;
+ }
+ // must be part
+ uploads.put(name, (Part) part);
}
@Override
@@ -158,48 +166,34 @@ public class RestClientRequestImpl implements RestClientRequest {
}
private void attachFiles(String boundary) {
- Iterator<Entry<String, Object>> uploadsIterator = uploads.entrySet().iterator();
- List<NamePartMap> namePartMaps = new ArrayList<>();
- while (uploadsIterator.hasNext()) {
- Entry<String, Object> entry = uploadsIterator.next();
- String name = entry.getKey();
- Object tmpPart = entry.getValue();
- if (tmpPart instanceof Part) {
- namePartMaps.add(new NamePartMap(name, (Part) tmpPart));
- } else {
- //it's a list
- @SuppressWarnings("unchecked")
- List<Part> partList = (List<Part>) tmpPart;
- partList.forEach(part -> namePartMaps.add(new NamePartMap(name, part)));
- }
- }
- attachFile(boundary, namePartMaps.iterator());
+ Iterator<Entry<String, Part>> uploadsIterator = uploads.entries().iterator();
+ attachFile(boundary, uploadsIterator);
}
- private void attachFile(String boundary, Iterator<NamePartMap> uploadsIterator) {
+ private void attachFile(String boundary, Iterator<Entry<String, Part>> uploadsIterator) {
if (!uploadsIterator.hasNext()) {
request.write(boundaryEndInfo(boundary));
request.end();
return;
}
- NamePartMap next = uploadsIterator.next();
+ Entry<String, Part> entry = uploadsIterator.next();
// do not use part.getName() to get parameter name
// because pojo consumer not easy to set name to part
-
- String filename = next.getPart().getSubmittedFileName();
-
- Buffer fileHeader = fileBoundaryInfo(boundary, next.getName(), next.getPart());
+ String name = entry.getKey();
+ Part part = entry.getValue();
+ String filename = part.getSubmittedFileName();
+ Buffer fileHeader = fileBoundaryInfo(boundary, name, part);
request.write(fileHeader);
- new PumpFromPart(context, next.getPart()).toWriteStream(request).whenComplete((v, e) -> {
+ new PumpFromPart(context, part).toWriteStream(request).whenComplete((v, e) -> {
if (e != null) {
- LOGGER.debug("Failed to sending file [{}:{}].", next.getName(), filename, e);
+ LOGGER.debug("Failed to sending file [{}:{}].", name, filename, e);
asyncResp.consumerFail(e);
return;
}
- LOGGER.debug("finish sending file [{}:{}].", next.getName(), filename);
+ LOGGER.debug("finish sending file [{}:{}].", name, filename);
attachFile(boundary, uploadsIterator);
});
}
@@ -294,23 +288,4 @@ public class RestClientRequestImpl implements RestClientRequest {
public MultiMap getHeaders() {
return request.headers();
}
-
- public static class NamePartMap {
- String name;
-
- Part part;
-
- public NamePartMap(String name, Part part) {
- this.name = name;
- this.part = part;
- }
-
- public String getName() {
- return name;
- }
-
- public Part getPart() {
- return part;
- }
- }
}
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 a417ef2..2c2f2fc 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
@@ -30,6 +30,7 @@ import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import org.apache.commons.lang3.StringUtils;
+import org.apache.servicecomb.common.rest.codec.param.FormProcessorCreator.PartProcessor;
import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor;
import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessorManager;
import org.apache.servicecomb.common.rest.definition.path.PathRegExp;
@@ -64,6 +65,8 @@ public class RestOperationMeta {
// key为参数名
protected Map<String, RestParam> paramMap = new LinkedHashMap<>();
+ protected List<String> fileKeys = new ArrayList<>();
+
// key为数据类型，比如json之类
private Map<String, ProduceProcessor> produceProcessorMap = new LinkedHashMap<>();
@@ -219,6 +222,9 @@ public class RestOperationMeta {
}
private void addParam(RestParam param) {
+ if (param.getParamProcessor() instanceof PartProcessor) {
+ fileKeys.add(param.getParamName());
+ }
paramList.add(param);
paramMap.put(param.getParamName(), param);
}
@@ -258,6 +264,10 @@ public class RestOperationMeta {
return operationMeta.getHttpMethod();
}
+ public List<String> getFileKeys() {
+ return fileKeys;
+ }
+
public List<String> getProduces() {
return produces;
}
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestRestClientRequestImpl.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestRestClientRequestImpl.java
index 137063f..6bcb3f9 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestRestClientRequestImpl.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestRestClientRequestImpl.java
@@ -16,6 +16,7 @@
*/
package org.apache.servicecomb.common.rest.codec.param;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
@@ -28,6 +29,8 @@ import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
+import com.google.common.collect.Multimap;
+
import io.vertx.core.MultiMap;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.CaseInsensitiveHeaders;
@@ -129,9 +132,9 @@ public class TestRestClientRequestImpl {
restClientRequest.attach(fileName, part);
- Map<String, Part> uploads = Deencapsulation.getField(restClientRequest, "uploads");
+ Multimap<String, Part> uploads = Deencapsulation.getField(restClientRequest, "uploads");
Assert.assertEquals(1, uploads.size());
- Assert.assertThat(uploads, Matchers.hasEntry(fileName, part));
+ Assert.assertThat(uploads.asMap(), Matchers.hasEntry(fileName, Arrays.asList(part)));
}
@Test
@@ -140,7 +143,7 @@ public class TestRestClientRequestImpl {
restClientRequest.attach("fileName", null);
- Map<String, Part> uploads = Deencapsulation.getField(restClientRequest, "uploads");
+ Multimap<String, Part> uploads = Deencapsulation.getField(restClientRequest, "uploads");
Assert.assertTrue(uploads.isEmpty());
}
diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerRequestToHttpServletRequest.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerRequestToHttpServletRequest.java
index 6280ef1..d7dfdcc 100644
--- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerRequestToHttpServletRequest.java
+++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerRequestToHttpServletRequest.java
@@ -17,7 +17,6 @@
package org.apache.servicecomb.foundation.vertx.http;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
@@ -26,6 +25,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
import javax.servlet.AsyncContext;
import javax.servlet.ServletInputStream;
@@ -280,9 +280,7 @@ public class VertxServerRequestToHttpServletRequest extends AbstractHttpServletR
@Override
public Collection<Part> getParts() {
Set<FileUpload> fileUploads = context.fileUploads();
- List<Part> partList = new ArrayList<>();
- fileUploads.stream().map(FileUploadPart::new).forEach(partList::add);
- return partList;
+ return fileUploads.stream().map(FileUploadPart::new).collect(Collectors.toList());
}
public RoutingContext getContext() {
diff --git a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CommonToHttpServletRequest.java b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CommonToHttpServletRequest.java
index 56e3c5a..5bfe5e3 100644
--- a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CommonToHttpServletRequest.java
+++ b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CommonToHttpServletRequest.java
@@ -35,8 +35,6 @@ import javax.servlet.http.Part;
import javax.ws.rs.core.HttpHeaders;
import org.apache.servicecomb.common.rest.RestConst;
-import org.apache.servicecomb.common.rest.codec.param.FormProcessorCreator.PartProcessor;
-import org.apache.servicecomb.common.rest.definition.RestParam;
import org.apache.servicecomb.foundation.common.part.FilePart;
import org.apache.servicecomb.foundation.common.part.InputStreamPart;
import org.apache.servicecomb.foundation.common.part.ResourcePart;
@@ -59,15 +57,9 @@ public class CommonToHttpServletRequest extends AbstractHttpServletRequest {
@SuppressWarnings("unchecked")
public CommonToHttpServletRequest(Map<String, String> pathParams, Map<String, List<String>> queryParams,
- Map<String, List<String>> httpHeaders, Object bodyObject, boolean isFormData, List<RestParam> paramList) {
+ Map<String, List<String>> httpHeaders, Object bodyObject, boolean isFormData, List<String> fileKeys) {
setAttribute(RestConst.PATH_PARAMETERS, pathParams);
- if (paramList != null && paramList.size() != 0) {
- for (RestParam param : paramList) {
- if (param.getParamProcessor() instanceof PartProcessor) {
- fileKeys.add(param.getParamName());
- }
- }
- }
+ this.fileKeys = fileKeys;
if (isFormData) {
setAttribute(RestConst.FORM_PARAMETERS, (Map<String, Object>) bodyObject);
} else {
@@ -228,7 +220,6 @@ public class CommonToHttpServletRequest extends AbstractHttpServletRequest {
}
private void filePartListWithForm(List<Part> partList, Map<String, Object> form) {
-
for (String key : fileKeys) {
Object value = form.get(key);
if (value == null) {
@@ -236,20 +227,19 @@ public class CommonToHttpServletRequest extends AbstractHttpServletRequest {
}
if (Collection.class.isInstance(value)) {
Collection<?> collection = (Collection<?>) value;
- if (collection.isEmpty()) {
- continue;
- }
for (Object part : collection) {
partList.add(getSinglePart(key, part));
}
- } else if (value.getClass().isArray()) {
+ continue;
+ }
+ if (value.getClass().isArray()) {
Object[] params = (Object[]) value;
for (int i = 0; i < params.length; i++) {
partList.add(getSinglePart(key, params[i]));
}
- } else {
- partList.add(getSinglePart(key, value));
+ continue;
}
+ partList.add(getSinglePart(key, value));
}
}
diff --git a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
index cc6d6d1..1191391 100644
--- a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
+++ b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
@@ -207,7 +207,7 @@ public class CseClientHttpRequest implements ClientHttpRequest {
protected Object[] collectArguments() {
HttpServletRequest mockRequest = new CommonToHttpServletRequest(requestMeta.getPathParams(), queryParams,
httpHeaders, requestBody, requestMeta.getSwaggerRestOperation().isFormData(),
- requestMeta.getSwaggerRestOperation().getParamList());
+ requestMeta.getSwaggerRestOperation().getFileKeys());
return RestCodec.restToArgs(mockRequest, requestMeta.getSwaggerRestOperation());
}
}
diff --git a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCommonToHttpServletRequest.java b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCommonToHttpServletRequest.java
index 52d925c..6461e6a 100644
--- a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCommonToHttpServletRequest.java
+++ b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCommonToHttpServletRequest.java
@@ -33,16 +33,11 @@ import javax.servlet.http.Part;
import javax.ws.rs.core.HttpHeaders;
import org.apache.servicecomb.common.rest.RestConst;
-import org.apache.servicecomb.common.rest.codec.param.FormProcessorCreator.PartProcessor;
-import org.apache.servicecomb.common.rest.definition.RestParam;
import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
-import mockit.Expectations;
-import mockit.Mocked;
-
public class TestCommonToHttpServletRequest {
@Test
public void testConstructFormTrue() {
@@ -63,24 +58,11 @@ public class TestCommonToHttpServletRequest {
}
@Test
- public void testConstructNormal(@Mocked RestParam param1, @Mocked RestParam param2,
- @Mocked PartProcessor partProcessor) {
- List<RestParam> restParams = new ArrayList<>();
- restParams.add(param1);
- restParams.add(param2);
- new Expectations() {
- {
- param1.getParamProcessor();
- result = partProcessor;
- param1.getParamName();
- result = "test1";
- param2.getParamProcessor();
- result = partProcessor;
- param2.getParamName();
- result = "test2";
- }
- };
- HttpServletRequest request = new CommonToHttpServletRequest(null, null, null, null, false, restParams);
+ public void testConstructNormal() {
+ List<String> fileKeys = new ArrayList<>();
+ fileKeys.add("test1");
+ fileKeys.add("test2");
+ HttpServletRequest request = new CommonToHttpServletRequest(null, null, null, null, false, fileKeys);
Assert.assertEquals(2, ((CommonToHttpServletRequest) request).getFileKeys().size());
Assert.assertEquals("test1", ((CommonToHttpServletRequest) request).getFileKeys().get(0));
Assert.assertEquals("test2", ((CommonToHttpServletRequest) request).getFileKeys().get(1));
@@ -270,25 +252,12 @@ public class TestCommonToHttpServletRequest {
}
@Test
- public void testGetParts(@Mocked RestParam param1, @Mocked RestParam param2,
- @Mocked PartProcessor partProcessor) {
- List<RestParam> restParams = new ArrayList<>();
- restParams.add(param1);
- restParams.add(param2);
+ public void testGetParts() {
+ List<String> restParams = new ArrayList<>();
+ restParams.add("test1");
+ restParams.add("test2");
File file1 = new File("file1.txt");
File file2 = new File("file2.txt");
- new Expectations() {
- {
- param1.getParamProcessor();
- result = partProcessor;
- param1.getParamName();
- result = "test1";
- param2.getParamProcessor();
- result = partProcessor;
- param2.getParamName();
- result = "test2";
- }
- };
File[] files = {file1, file2};
List<File> list = Arrays.asList(files);
Map<String, Object> objectMap = new HashMap<>();
diff --git a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/OperationGenerator.java b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/OperationGenerator.java
index cd3b0f5..feacd86 100644
--- a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/OperationGenerator.java
+++ b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/OperationGenerator.java
@@ -359,7 +359,7 @@ public class OperationGenerator {
}
}
- // if is list<?> check and get respond type
+ // check whether is ArrayList , LinkedArrayList ... or not
private Type checkAndGetType(Type type) {
if (ParameterizedType.class.isAssignableFrom(type.getClass())) {
ParameterizedType targetType = (ParameterizedType) type;
diff --git a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/converter/ConverterMgr.java b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/converter/ConverterMgr.java
index e86a55e..faf65ec 100644
--- a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/converter/ConverterMgr.java
+++ b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/converter/ConverterMgr.java
@@ -118,7 +118,7 @@ public class ConverterMgr {
return null;
}
- // if is list<?> check and get respond type
+ // check whether is ArrayList , LinkedArrayList ... or not
private Type checkAndGetType(Type type) {
if (ParameterizedType.class.isAssignableFrom(type.getClass())) {
ParameterizedType targetType = (ParameterizedType) type;