From b108762d54394ca08a75d0ab5facf6ab8eb0a395 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Wed, 29 Jan 2025 13:44:31 +0200 Subject: [PATCH] Simple UI: Streaming upload (#2254) thus not loading whole artifact into memory Signed-off-by: Avgustin Marinov --- .../hawkbit/rest/RestConfiguration.java | 26 +-- .../eclipse/hawkbit/sdk/HawkbitClient.java | 161 +++++++++++++++++- hawkbit-simple-ui/.gitignore | 3 +- hawkbit-simple-ui/pom.xml | 4 + .../ui/simple/view/DistributionSetView.java | 3 +- .../ui/simple/view/SoftwareModuleView.java | 11 +- .../src/main/resources/application.properties | 3 +- 7 files changed, 188 insertions(+), 23 deletions(-) diff --git a/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java b/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java index 7f9c70a8e..127f02556 100644 --- a/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java +++ b/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java @@ -159,6 +159,7 @@ public class RestConfiguration { @ExceptionHandler(AbstractServerRtException.class) public ResponseEntity handleSpServerRtExceptions(final HttpServletRequest request, final Exception ex) { logRequest(request, ex); + final ExceptionInfo response = createExceptionInfo(ex); final HttpStatus responseStatus; if (ex instanceof AbstractServerRtException abstractServerRtException) { @@ -172,8 +173,7 @@ public class RestConfiguration { /** * Method for handling exception of type {@link FileStreamingFailedException} which is thrown in case the streaming of a file failed * due to an internal server error. As the streaming of the file has already begun, no JSON response but only the ResponseStatus 500 - * is returned. - * Called by the Spring-Framework for exception handling. + * is returned. Called by the Spring-Framework for exception handling. * * @param request the Http request * @param ex the exception which occurred @@ -182,6 +182,7 @@ public class RestConfiguration { @ExceptionHandler(FileStreamingFailedException.class) public ResponseEntity handleFileStreamingFailedException(final HttpServletRequest request, final Exception ex) { logRequest(request, ex); + log.warn("File streaming failed: {}", ex.getMessage()); return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); } @@ -201,22 +202,22 @@ public class RestConfiguration { IllegalArgumentException.class }) public ResponseEntity handleExceptionCausedByIncorrectRequestBody(final HttpServletRequest request, final Exception ex) { logRequest(request, ex); + final ExceptionInfo response = createExceptionInfo(new MessageNotReadableException()); return new ResponseEntity<>(response, HttpStatus.BAD_REQUEST); } /** * Method for handling exception of type ConstraintViolationException which is thrown in case the request is rejected due to a - * constraint violation. - * Called by the Spring-Framework for exception handling. + * constraint violation. Called by the Spring-Framework for exception handling. * * @param request the Http request * @param ex the exception which occurred * @return the entity to be responded containing the exception information as entity. */ @ExceptionHandler(ConstraintViolationException.class) - public ResponseEntity handleConstraintViolationException(final HttpServletRequest request, - final ConstraintViolationException ex) { + public ResponseEntity handleConstraintViolationException( + final HttpServletRequest request, final ConstraintViolationException ex) { logRequest(request, ex); final ExceptionInfo response = new ExceptionInfo(); @@ -251,8 +252,7 @@ public class RestConfiguration { /** * Method for handling exception of type {@link MultipartException} which is thrown in case the request body is not well-formed and - * cannot be deserialized. - * Called by the Spring-Framework for exception handling. + * cannot be deserialized. Called by the Spring-Framework for exception handling. * * @param request the Http request * @param ex the exception which occurred @@ -266,8 +266,7 @@ public class RestConfiguration { final Throwable responseCause = throwables.get(throwables.size() - 1); if (ObjectUtils.isEmpty(responseCause.getMessage())) { - log.warn("Request {} lead to MultipartException without root cause message:\n{}", request.getRequestURL(), - ex.getStackTrace()); + log.warn("Request {} lead to MultipartException without root cause message:\n{}", request.getRequestURL(), ex.getStackTrace()); } return new ResponseEntity<>(createExceptionInfo(new MultiPartFileUploadException(responseCause)), HttpStatus.BAD_REQUEST); @@ -277,6 +276,10 @@ public class RestConfiguration { return ERROR_TO_HTTP_STATUS.getOrDefault(error, DEFAULT_RESPONSE_STATUS); } + // enable certain level of debug with + // -> logging.level.org.eclipse.hawkbit.rest.RestConfiguration=DEBUG + // or for more detailed log + // -> logging.level.org.eclipse.hawkbit.rest.RestConfiguration=TRACE private void logRequest(final HttpServletRequest request, final Exception ex) { if (log.isTraceEnabled()) { log.trace("Handling exception {} of request {}", ex.getClass().getName(), request.getRequestURL(), ex); @@ -295,7 +298,6 @@ public class RestConfiguration { } return response; } - } /** @@ -309,7 +311,7 @@ public class RestConfiguration { private final String[] excludeAntPaths; private final AntPathMatcher antMatcher = new AntPathMatcher(); - public ExcludePathAwareShallowETagFilter(final String... excludeAntPaths) { + public ExcludePathAwareShallowETagFilter(final String... excludeAntPaths) { this.excludeAntPaths = excludeAntPaths; } diff --git a/hawkbit-sdk/hawkbit-sdk-commons/src/main/java/org/eclipse/hawkbit/sdk/HawkbitClient.java b/hawkbit-sdk/hawkbit-sdk-commons/src/main/java/org/eclipse/hawkbit/sdk/HawkbitClient.java index efded9543..9e0b68c0c 100644 --- a/hawkbit-sdk/hawkbit-sdk-commons/src/main/java/org/eclipse/hawkbit/sdk/HawkbitClient.java +++ b/hawkbit-sdk/hawkbit-sdk-commons/src/main/java/org/eclipse/hawkbit/sdk/HawkbitClient.java @@ -9,21 +9,44 @@ */ package org.eclipse.hawkbit.sdk; +import java.io.BufferedOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.lang.annotation.Annotation; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Proxy; +import java.net.HttpURLConnection; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.List; import java.util.Objects; +import java.util.UUID; import java.util.function.BiFunction; +import java.util.stream.Stream; +import com.fasterxml.jackson.databind.ObjectMapper; import feign.Client; import feign.Contract; import feign.Feign; import feign.RequestInterceptor; +import feign.RequestTemplate; import feign.codec.Decoder; import feign.codec.Encoder; import feign.codec.ErrorDecoder; import lombok.Builder; import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ResponseEntity; import org.springframework.util.ObjectUtils; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RequestPart; +import org.springframework.web.multipart.MultipartFile; @Slf4j @Builder @@ -104,15 +127,145 @@ public class HawkbitClient { } private T service(final Class serviceType, final Tenant tenant, final Controller controller) { + final T service = service0(serviceType, tenant, controller); + if (serviceType.isInterface() // proxy only interfaces + && Stream.of(serviceType.getDeclaredMethods()) // and has MultipartFile argument + .anyMatch(method -> method.getAnnotation(PostMapping.class) != null + && List.of(method.getParameterTypes()).contains(MultipartFile.class))) { + // doesn't use feign client since it doesn't (?) support streaming - loading all in memory which could lead to OOM + // https://github.com/OpenFeign/feign-form/issues/121 (?) + return proxy(serviceType, service, tenant, controller); + } else { // default + return service; + } + } + + @SuppressWarnings("unchecked") + private T proxy(final Class serviceType, final T service, final Tenant tenant, final Controller controller) { + final ObjectMapper objectMapper = new ObjectMapper(); + return (T) Proxy.newProxyInstance(service.getClass().getClassLoader(), new Class[] { serviceType }, (proxy, method, args) -> { + try { + final Class[] parameterTypes = method.getParameterTypes(); + if (method.getDeclaringClass() == serviceType && List.of(parameterTypes).contains(MultipartFile.class)) { + return processMultipartFormData(method, args, tenant, controller, parameterTypes, objectMapper); + } else { + return method.invoke(service, args); + } + } catch (final InvocationTargetException e) { + throw e.getTargetException() == null ? e : e.getTargetException(); + } + }); + } + + private static final String CRLF = "\r\n"; + + private Object processMultipartFormData( + final Method method, final Object[] args, + final Tenant tenant, final Controller controller, + final Class[] parameterTypes, final ObjectMapper objectMapper) throws IOException { + final PostMapping postMapping = method.getAnnotation(PostMapping.class); + final Annotation[][] parametersAnnotations = method.getParameterAnnotations(); + // build path - replace @PathVariables + String path = postMapping.value()[0]; + for (int i = 0; i < args.length; i++) { + final PathVariable pathVariable = getAnnotation(PathVariable.class, parametersAnnotations[i]); + if (pathVariable != null) { + path = path.replace("{" + pathVariable.value() + "}", args[i].toString()); + } + } + + final HttpURLConnection conn = (HttpURLConnection) new URL( + (controller == null ? hawkBitServer.getMgmtUrl() : hawkBitServer.getDdiUrl()) + path).openConnection(); + conn.setRequestMethod("POST"); + + // deal with authentication - only from headers1 + final RequestTemplate requestTemplate = new RequestTemplate(); + requestInterceptorFn.apply(tenant, controller).apply(requestTemplate); + requestTemplate.headers().forEach((k, v) -> v.forEach(e -> conn.setRequestProperty(k, e))); + + final String boundary = UUID.randomUUID().toString().replace("-", ""); + conn.setRequestProperty("content-type", "multipart/form-data;boundary=" + boundary); + // consumes what the method produces + final String[] consumes = postMapping.produces(); + if (!ObjectUtils.isEmpty(consumes)) { + conn.setRequestProperty("accept", String.join(",", consumes)); + } + + conn.setDoOutput(true); + conn.setDoInput(true); + + try (final OutputStream out = new BufferedOutputStream(conn.getOutputStream())) { + for (int i = 0; i < args.length; i++) { + final Class type = parameterTypes[i]; + if (MultipartFile.class.isAssignableFrom(type)) { + final MultipartFile multipartFile = (MultipartFile) args[i]; + if (multipartFile != null) { + writeMultipartFile(multipartFile, out, boundary, parametersAnnotations[i]); + } + } else { + writeSimpleFormData(args[i], out, boundary, parametersAnnotations[i]); + } + } + out.write(("--" + boundary + "--\r\n").getBytes(StandardCharsets.UTF_8)); + } + return method.getReturnType() == ResponseEntity.class + ? new ResponseEntity( + objectMapper.readValue( + conn.getInputStream(), + (Class) ((ParameterizedType) method.getGenericReturnType()).getActualTypeArguments()[0]), + HttpStatusCode.valueOf(conn.getResponseCode())) + : objectMapper.readValue(conn.getInputStream(), method.getReturnType()); + } + + private void writeMultipartFile( + final MultipartFile multipartFile, final OutputStream out, final String boundary, final Annotation[] parametersAnnotations) + throws IOException { + final String name = Objects.requireNonNull( + getAnnotation(RequestPart.class, parametersAnnotations), "MultipartFile shall have RequestPart annotation") + .value(); + try (final InputStream in = multipartFile.getInputStream()) { + out.write(("--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"" + name + "\"; filename=\"" + multipartFile.getName() + "\"\r\n" + + "Content-Type: " + multipartFile.getContentType() + "\r\n\r\n" + ).getBytes(StandardCharsets.UTF_8)); + final byte[] buff = new byte[8096]; + for (int read; (read = in.read(buff)) != -1; ) { + out.write(buff, 0, read); + } + out.write(CRLF.getBytes(StandardCharsets.UTF_8)); + } + } + + private void writeSimpleFormData( + final Object arg, final OutputStream out, final String boundary, final Annotation[] parameterAnnotations) throws IOException { + if (arg != null) { + final RequestParam requestParam = getAnnotation(RequestParam.class, parameterAnnotations); + if (requestParam != null) { + out.write(("--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"" + requestParam.value() + "\"\r\n\r\n" + + arg + CRLF + ).getBytes(StandardCharsets.UTF_8)); + } + } // otherwise default + } + + @SuppressWarnings("unchecked") + private T getAnnotation(final Class annotationClass, final Annotation[] annotations) { + for (final Annotation annotation : annotations) { + if (annotation.annotationType().equals(annotationClass)) { + return (T) annotation; + } + } + return null; + } + + private T service0(final Class serviceType, final Tenant tenant, final Controller controller) { return Feign.builder().client(client) .encoder(encoder) .decoder(decoder) .errorDecoder(errorDecoder) .contract(contract) .requestInterceptor(requestInterceptorFn.apply(tenant, controller)) - .target(serviceType, - controller == null ? - hawkBitServer.getMgmtUrl() : - hawkBitServer.getDdiUrl()); + .target(serviceType, controller == null ? hawkBitServer.getMgmtUrl() : hawkBitServer.getDdiUrl()); } } \ No newline at end of file diff --git a/hawkbit-simple-ui/.gitignore b/hawkbit-simple-ui/.gitignore index 10d4fb7ac..d1e692c4b 100644 --- a/hawkbit-simple-ui/.gitignore +++ b/hawkbit-simple-ui/.gitignore @@ -1 +1,2 @@ -frontend \ No newline at end of file +frontend +bundles \ No newline at end of file diff --git a/hawkbit-simple-ui/pom.xml b/hawkbit-simple-ui/pom.xml index aa27a471c..645af2e51 100644 --- a/hawkbit-simple-ui/pom.xml +++ b/hawkbit-simple-ui/pom.xml @@ -103,6 +103,10 @@ compile + + true + false + diff --git a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/DistributionSetView.java b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/DistributionSetView.java index e5c6a8ae5..4a8413e71 100644 --- a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/DistributionSetView.java +++ b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/DistributionSetView.java @@ -142,8 +142,7 @@ public class DistributionSetView extends TableView { return Filter.filter( Map.of( "name", name.getOptionalValue(), - "type", type.getSelectedItems().stream().map(MgmtDistributionSetType::getKey) - .toList(), + "type", type.getSelectedItems().stream().map(MgmtDistributionSetType::getKey).toList(), "tag", tag.getSelectedItems())); } } diff --git a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/SoftwareModuleView.java b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/SoftwareModuleView.java index 2e4accc5d..8999afb94 100644 --- a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/SoftwareModuleView.java +++ b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/SoftwareModuleView.java @@ -45,6 +45,7 @@ import com.vaadin.flow.component.upload.receivers.FileBuffer; import com.vaadin.flow.data.renderer.ComponentRenderer; import com.vaadin.flow.router.PageTitle; import com.vaadin.flow.router.Route; +import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.mgmt.json.model.PagedList; import org.eclipse.hawkbit.mgmt.json.model.artifact.MgmtArtifact; import org.eclipse.hawkbit.mgmt.json.model.softwaremodule.MgmtSoftwareModule; @@ -63,6 +64,7 @@ import org.springframework.web.multipart.MultipartFile; @Route(value = "software_modules", layout = MainLayout.class) @RolesAllowed({ "SOFTWARE_MODULE_READ" }) @Uses(Icon.class) +@Slf4j public class SoftwareModuleView extends TableView { @Autowired @@ -307,9 +309,11 @@ public class SoftwareModuleView extends TableView { uploadBtn.setDropAllowed(true); uploadBtn.addSucceededListener(e -> { final MgmtArtifact artifact = hawkbitClient.getSoftwareModuleRestApi() - .uploadArtifact(softwareModuleId, - new MultipartFileImpl(fileBuffer, e.getContentLength(), e.getMIMEType()), fileBuffer.getFileName(), null, null, - null).getBody(); + .uploadArtifact( + softwareModuleId, + new MultipartFileImpl(fileBuffer, e.getContentLength(), e.getMIMEType()), + fileBuffer.getFileName(), null, null, null) + .getBody(); artifacts.add(artifact); artifactGrid.refreshGrid(false); }); @@ -367,6 +371,7 @@ public class SoftwareModuleView extends TableView { @Override public byte[] getBytes() throws IOException { + log.warn("Multipart file getBytes() is called. Whole input stream is loaded into the memory!"); try (final InputStream is = getInputStream()) { return is.readAllBytes(); } diff --git a/hawkbit-simple-ui/src/main/resources/application.properties b/hawkbit-simple-ui/src/main/resources/application.properties index 4485de798..86f29fe8b 100644 --- a/hawkbit-simple-ui/src/main/resources/application.properties +++ b/hawkbit-simple-ui/src/main/resources/application.properties @@ -16,8 +16,9 @@ logging.level.org.springframework.boot.actuate.audit.listener.AuditListener=WARN # logging pattern logging.pattern.console=%clr(%d{${logging.pattern.dateformat:yyyy-MM-dd'T'HH:mm:ss.SSSXXX}}){faint} %clr(${logging.pattern.level:%5p}) %clr(${PID:}){magenta} %clr(---){faint} %clr([${spring.application.name}] [%X{tenant}:%X{user}] [%15.15t]){faint} %clr(${logging.pattern.correlation:}){faint}%clr(%-40.40logger{39}){cyan} %clr(:){faint} %m%n${logging.exception-conversion-word:%wEx} -### Vaadin start ###` +### Vaadin start ### # build with mvn vaadin:build-frontend to enable / disable +vaadin.productionMode=true vaadin.frontend.hotdeploy=false logging.level.org.atmosphere=warn spring.mustache.check-template-location=false