From d4c1e820904dcf9529e7fa26a8e3219c1a7d6f01 Mon Sep 17 00:00:00 2001 From: Stefan Klotz <35995139+StefanKlt@users.noreply.github.com> Date: Mon, 12 Nov 2018 08:57:10 +0100 Subject: [PATCH] Feature do not temp store uploaded files (#763) * Upload artefacts directly without creating a temp file * Remove redundant code that checks if user tries to update a directory * Handle UI upload exceptions, use Spring thread pool * Clean up code * Fix review findings * Fix Sonar findings Signed-off-by: Stefan Klotz Signed-off-by: Stefan Behl --- .../model/TargetWithActionType.java | 13 +- .../jpa/AbstractDsAssignmentStrategy.java | 1 + .../jpa/JpaDeploymentManagement.java | 66 ++++++- .../repository/jpa/model/JpaAction.java | 3 +- .../upload/AbstractFileTransferHandler.java | 182 +++++++++++------- .../FileTransferHandlerStreamVariable.java | 33 ++-- .../FileTransferHandlerVaadinUpload.java | 40 ++-- .../artifacts/upload/FileUploadProgress.java | 21 ++ .../upload/UploadDropAreaLayout.java | 28 +-- .../hawkbit/ui/utils/SpringContextHelper.java | 15 ++ .../src/main/resources/messages.properties | 1 + 11 files changed, 266 insertions(+), 137 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetWithActionType.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetWithActionType.java index 858c694af..2391bbea5 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetWithActionType.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetWithActionType.java @@ -25,9 +25,7 @@ public class TargetWithActionType { private String maintenanceWindowTimeZone; public TargetWithActionType(final String controllerId) { - this.controllerId = controllerId; - this.actionType = ActionType.FORCED; - this.forceTime = 0; + this(controllerId, ActionType.FORCED, 0); } public TargetWithActionType(final String controllerId, final ActionType actionType, final long forceTime) { @@ -115,4 +113,13 @@ public class TargetWithActionType { public String getMaintenanceWindowTimeZone() { return maintenanceWindowTimeZone; } + + @Override + public String toString() { + return "TargetWithActionType [controllerId=" + controllerId + ", actionType=" + getActionType() + ", forceTime=" + + getForceTime() + ", maintenanceSchedule=" + getMaintenanceSchedule() + ", maintenanceWindowDuration=" + + getMaintenanceWindowDuration() + ", maintenanceWindowTimeZone=" + getMaintenanceWindowTimeZone() + + "]"; + } + } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java index ce0bfb2cc..86baeba8f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java @@ -215,6 +215,7 @@ public abstract class AbstractDsAssignmentStrategy { .publishEvent(new CancelTargetAssignmentEvent(target, actionId, applicationContext.getId()))); } + @SuppressWarnings("squid:S2259") JpaAction createTargetAction(final Map targetsWithActionMap, final JpaTarget target, final JpaDistributionSet set) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java index 93d3168cd..4600293e4 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java @@ -90,6 +90,9 @@ import org.springframework.transaction.support.TransactionTemplate; import org.springframework.util.CollectionUtils; import org.springframework.validation.annotation.Validated; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectWriter; import com.google.common.base.Joiner; import com.google.common.collect.Lists; @@ -100,6 +103,7 @@ import com.google.common.collect.Lists; @Transactional(readOnly = true) @Validated public class JpaDeploymentManagement implements DeploymentManagement { + private static final Logger LOG = LoggerFactory.getLogger(JpaDeploymentManagement.class); /** @@ -321,8 +325,10 @@ public class JpaDeploymentManagement implements DeploymentManagement { assignmentStrategy.updateTargetStatus(set, targetIds, currentUser); final Map targetIdsToActions = targets.stream() - .map(t -> actionRepository.save(assignmentStrategy.createTargetAction(targetsWithActionMap, t, set))) - .collect(Collectors.toMap(a -> a.getTarget().getControllerId(), Function.identity())); + .map(trg -> createTargetAction(assignmentStrategy, targetsWithActionType, controllerIDs, targets, + targetsWithActionMap, trg, set)) + .map(actionRepository::save) + .collect(Collectors.toMap(action -> action.getTarget().getControllerId(), Function.identity())); // create initial action status when action is created so we remember // the initial running status because we will change the status @@ -578,7 +584,6 @@ public class JpaDeploymentManagement implements DeploymentManagement { @Override public Page findActiveActionsByTarget(final Pageable pageable, final String controllerId) { throwExceptionIfTargetDoesNotExist(controllerId); - return actionRepository.findByActiveAndTarget(pageable, controllerId, true); } @@ -592,14 +597,12 @@ public class JpaDeploymentManagement implements DeploymentManagement { @Override public long countActionsByTarget(final String controllerId) { throwExceptionIfTargetDoesNotExist(controllerId); - return actionRepository.countByTargetControllerId(controllerId); } @Override public long countActionsByTarget(final String rsqlParam, final String controllerId) { throwExceptionIfTargetDoesNotExist(controllerId); - return actionRepository.count(createSpecificationFor(controllerId, rsqlParam)); } @@ -684,7 +687,6 @@ public class JpaDeploymentManagement implements DeploymentManagement { @Override public Slice findActionsByDistributionSet(final Pageable pageable, final long dsId) { throwExceptionIfDistributionSetDoesNotExist(dsId); - return actionRepository.findByDistributionSetId(pageable, dsId); } @@ -696,14 +698,12 @@ public class JpaDeploymentManagement implements DeploymentManagement { @Override public Optional getAssignedDistributionSet(final String controllerId) { throwExceptionIfTargetDoesNotExist(controllerId); - return distributionSetRepository.findAssignedToTarget(controllerId); } @Override public Optional getInstalledDistributionSet(final String controllerId) { throwExceptionIfTargetDoesNotExist(controllerId); - return distributionSetRepository.findInstalledAtTarget(controllerId); } @@ -736,7 +736,7 @@ public class JpaDeploymentManagement implements DeploymentManagement { return deleteQuery.executeUpdate(); } - private static String getQueryForDeleteActionsByStatusAndLastModifiedBeforeString(Database database) { + private static String getQueryForDeleteActionsByStatusAndLastModifiedBeforeString(final Database database) { return QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED.getOrDefault(database, QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED_DEFAULT); } @@ -749,4 +749,52 @@ public class JpaDeploymentManagement implements DeploymentManagement { return "#" + Joiner.on(",#").join(elements); } + @SuppressWarnings({ "squid:S1695", "squid:S1696" }) + private JpaAction createTargetAction(final AbstractDsAssignmentStrategy assignmentStrategy, + final Collection targetsWithActionType, final List controllerIDs, + final List targets, final Map targetsWithActionMap, + final JpaTarget target, final JpaDistributionSet set) { + try { + return assignmentStrategy.createTargetAction(targetsWithActionMap, target, set); + } catch (final NullPointerException e) { + dumpDistributionSetAssignment(target, set, targetsWithActionType, controllerIDs, targets, + targetsWithActionMap); + throw e; + } + } + + private void dumpDistributionSetAssignment(final JpaTarget target, final JpaDistributionSet set, + final Collection targetsWithActionType, final List controllerIDs, + final List targets, final Map targetsWithActionMap) { + final ObjectWriter writer = new ObjectMapper().writer(); + final String correlationID = "DistributionSetAssignmentDump[" + Thread.currentThread().getId() + "]"; + dump(correlationID, "Target", Objects.toString(target)); + dump(correlationID, "DistributionSet", Objects.toString(set)); + if (target != null) { + dump(correlationID, "ControllerID", Objects.toString(target.getControllerId())); + if (set != null) { + dump(correlationID, "Actions", toString(writer, + actionRepository.findByTargetAndDistributionSet(new PageRequest(0, 500), target, set))); + } + } + dump(correlationID, "ControllerIDs", toString(writer, controllerIDs)); + dump(correlationID, "Targets", toString(writer, targets)); + dump(correlationID, "TargetsWithActionType", toString(writer, targetsWithActionType)); + dump(correlationID, "TargetsWithActionMap", toString(writer, targetsWithActionMap)); + } + + private static void dump(final String prefix, final String key, final String value) { + LOG.error("{} >>> {}: {}", prefix, key, value); + } + + @SuppressWarnings("squid:S1166") + private static String toString(final ObjectWriter writer, final Object o) { + try { + return writer.writeValueAsString(o); + } catch (final JsonProcessingException e) { + LOG.error("Object serialization failed", e); + return e.getMessage(); + } + } + } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaAction.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaAction.java index 313c275c3..9bfc2c197 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaAction.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaAction.java @@ -209,7 +209,8 @@ public class JpaAction extends AbstractJpaTenantAwareBaseEntity implements Actio @Override public String toString() { return "JpaAction [distributionSet=" + distributionSet.getId() + ", version=" + getOptLockRevision() + ", id=" - + getId() + "]"; + + getId() + ", actionType=" + getActionType() + ", isActive=" + isActive() + ", createdAt=" + + getCreatedAt() + ", lastModifiedAt=" + getLastModifiedAt() + "]"; } @Override diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java index 8f727b0cc..ec61a3069 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java @@ -8,12 +8,12 @@ */ package org.eclipse.hawkbit.ui.artifacts.upload; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.Serializable; +import java.util.Optional; +import java.util.concurrent.ExecutorService; import org.apache.commons.lang3.StringUtils; import org.eclipse.hawkbit.repository.ArtifactManagement; @@ -27,13 +27,14 @@ import org.eclipse.hawkbit.ui.artifacts.event.SoftwareModuleEvent.SoftwareModule import org.eclipse.hawkbit.ui.artifacts.state.ArtifactUploadState; import org.eclipse.hawkbit.ui.artifacts.upload.FileUploadProgress.FileUploadStatus; import org.eclipse.hawkbit.ui.utils.SpringContextHelper; +import org.eclipse.hawkbit.ui.utils.UINotification; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.vaadin.spring.events.EventBus; import org.vaadin.spring.events.EventBus.UIEventBus; -import com.google.common.io.ByteStreams; +import com.vaadin.ui.UI; /** * Abstract base class for transferring files from the browser to the @@ -49,8 +50,6 @@ public abstract class AbstractFileTransferHandler implements Serializable { private volatile boolean uploadInterrupted; - private volatile String tempFilePath; - private volatile String failureReason; private final ArtifactUploadState artifactUploadState; @@ -61,11 +60,14 @@ public abstract class AbstractFileTransferHandler implements Serializable { private final VaadinMessageSource i18n; + protected final UINotification uiNotification; + AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n) { this.artifactManagement = artifactManagement; this.i18n = i18n; this.eventBus = SpringContextHelper.getBean(EventBus.UIEventBus.class); this.artifactUploadState = SpringContextHelper.getBean(ArtifactUploadState.class); + this.uiNotification = SpringContextHelper.getBean(UINotification.class); } protected boolean isDuplicateFile() { @@ -99,6 +101,13 @@ public abstract class AbstractFileTransferHandler implements Serializable { return i18n; } + protected void startTransferToRepositoryThread(final InputStream inputStream, final FileUploadId fileUploadId, + final String mimeType) { + SpringContextHelper.getBean("asyncExecutor", ExecutorService.class) + .execute( + new TransferArtifactToRepositoryRunnable(inputStream, fileUploadId, mimeType, UI.getCurrent())); + } + private void setFailureReason(final String failureReason) { this.failureReason = failureReason; } @@ -131,22 +140,13 @@ public abstract class AbstractFileTransferHandler implements Serializable { } protected void publishUploadProgressEvent(final FileUploadId fileUploadId, final long bytesReceived, - final long fileSize, final String tempFilePath) { + final long fileSize) { if (LOG.isTraceEnabled()) { LOG.trace("Upload in progress for file {} - {}%", fileUploadId, String.format("%.0f", (double) bytesReceived / (double) fileSize * 100)); } final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, - FileUploadStatus.UPLOAD_IN_PROGRESS, bytesReceived, fileSize, tempFilePath); - artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress); - eventBus.publish(this, fileUploadProgress); - } - - private void publishUploadSucceeded(final FileUploadId fileUploadId, final long fileSize, - final String tempFilePath) { - LOG.info("Upload succeeded for file {}", fileUploadId); - final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, - FileUploadStatus.UPLOAD_SUCCESSFUL, fileSize, fileSize, tempFilePath); + FileUploadStatus.UPLOAD_IN_PROGRESS, bytesReceived, fileSize); artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress); eventBus.publish(this, fileUploadProgress); } @@ -158,6 +158,19 @@ public abstract class AbstractFileTransferHandler implements Serializable { eventBus.publish(this, fileUploadProgress); } + protected void publishUploadSucceeded(final FileUploadId fileUploadId, final long fileSize) { + LOG.info("Upload succeeded for file {}", fileUploadId); + final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, + FileUploadStatus.UPLOAD_SUCCESSFUL, fileSize, fileSize); + artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress); + eventBus.publish(this, fileUploadProgress); + } + + protected void publishArtifactsChanged(final FileUploadId fileUploadId) { + eventBus.publish(this, + new SoftwareModuleEvent(SoftwareModuleEventType.ARTIFACTS_CHANGED, fileUploadId.getSoftwareModuleId())); + } + protected void publishUploadFailedEvent(final FileUploadId fileUploadId, final String failureReason, final Exception uploadException) { LOG.info("Upload failed for file {} due to {}", fileUploadId, @@ -180,59 +193,94 @@ public abstract class AbstractFileTransferHandler implements Serializable { } } - protected OutputStream createOutputStreamForTempFile() throws IOException { - - if (isUploadInterrupted()) { - return ByteStreams.nullOutputStream(); - } - - final File tempFile = File.createTempFile("spUiArtifactUpload", null); - - // we return the outputstream so we cannot close it here - @SuppressWarnings("squid:S2095") - final OutputStream out = new FileOutputStream(tempFile); - - tempFilePath = tempFile.getAbsolutePath(); - - return out; - } - - protected String getTempFilePath() { - return tempFilePath; - } - - // Exception squid:S3655 - Optional access is checked in - // checkIfArtifactDetailsDispalyed subroutine - @SuppressWarnings("squid:S3655") - protected void transferArtifactToRepository(final FileUploadId fileUploadId, final long fileSize, - final String mimeType, final String tempFilePath) { - - final File newFile = new File(tempFilePath); - - final String filename = fileUploadId.getFilename(); - LOG.info("Transfering tempfile {} - {} to repository", filename, tempFilePath); - try (FileInputStream fis = new FileInputStream(newFile)) { - - artifactManagement.create(fis, fileUploadId.getSoftwareModuleId(), filename, null, null, true, mimeType, - fileSize); - - publishUploadSucceeded(fileUploadId, fileSize, tempFilePath); - - eventBus.publish(this, new SoftwareModuleEvent(SoftwareModuleEventType.ARTIFACTS_CHANGED, - fileUploadId.getSoftwareModuleId())); - - } catch (final ArtifactUploadFailedException | InvalidSHA1HashException | InvalidMD5HashException - | IOException e) { - publishUploadFailedEvent(fileUploadId, i18n.getMessage("message.upload.failed"), e); - LOG.error("Failed to transfer file to repository", e); - } finally { - LOG.debug("Deleting tempfile {} - {}", filename, newFile.getAbsolutePath()); - if (newFile.exists() && !newFile.delete()) { - LOG.error("Could not delete temporary file: {}", newFile.getAbsolutePath()); + protected static void tryToCloseIOStream(final OutputStream outputStream) { + if (outputStream != null) { + try { + outputStream.close(); + } catch (final IOException e1) { + LOG.error("Closing output stream caused an exception {}", e1); } } - publishUploadFinishedEvent(fileUploadId); + } + + protected static void tryToCloseIOStream(final InputStream inputStream) { + if (inputStream != null) { + try { + inputStream.close(); + } catch (final IOException e1) { + LOG.error("Closing input stream caused an exception {}", e1); + } + } + } + + private final class TransferArtifactToRepositoryRunnable implements Runnable { + private final InputStream inputStream; + private final FileUploadId fileUploadId; + private final String mimeType; + private final UI vaadinUi; + + public TransferArtifactToRepositoryRunnable(final InputStream inputStream, final FileUploadId fileUploadId, + final String mimeType, final UI vaadinUi) { + this.inputStream = inputStream; + this.fileUploadId = fileUploadId; + this.mimeType = mimeType; + this.vaadinUi = vaadinUi; + } + + @Override + public void run() { + try { + UI.setCurrent(vaadinUi); + streamToRepository(); + } catch (final RuntimeException e) { + publishUploadFailedEvent(fileUploadId, i18n.getMessage("message.upload.failed"), e); + LOG.error("Failed to transfer file to repository", e); + } finally { + tryToCloseIOStream(inputStream); + } + } + + private void streamToRepository() { + if (fileUploadId == null) { + throw new ArtifactUploadFailedException(); + } + final String filename = fileUploadId.getFilename(); + LOG.info("Transfering file {} directly to repository", filename); + final Artifact artifact = uploadArtifact(filename).orElseThrow(ArtifactUploadFailedException::new); + if (isUploadInterrupted()) { + handleUploadFailure(artifact); + return; + } + publishUploadSucceeded(fileUploadId, artifact.getSize()); + publishUploadFinishedEvent(fileUploadId); + publishArtifactsChanged(fileUploadId); + } + + private Optional uploadArtifact(final String filename) { + try { + return Optional.ofNullable(artifactManagement.create(inputStream, fileUploadId.getSoftwareModuleId(), + filename, null, null, true, mimeType, -1)); + } catch (final ArtifactUploadFailedException | InvalidSHA1HashException | InvalidMD5HashException e) { + LOG.error("Failed to transfer file to repository", e); + return Optional.empty(); + } + } + + private void handleUploadFailure(final Artifact artifact) { + Exception exception; + int tries = 0; + do { + try { + artifactManagement.delete(artifact.getId()); + return; + } catch (final RuntimeException e) { + exception = e; + tries++; + } + } while (tries < 5); + LOG.error("Failed to delete artifact from repository after upload was interrupted", exception); + } } } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java index af8dc425c..a871c31ae 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java @@ -10,6 +10,8 @@ package org.eclipse.hawkbit.ui.artifacts.upload; import java.io.IOException; import java.io.OutputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.model.SoftwareModule; @@ -72,26 +74,28 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl @Override public final OutputStream getOutputStream() { + if (isUploadInterrupted()) { + return ByteStreams.nullOutputStream(); + } // we return the outputstream so we cannot close it here @SuppressWarnings("squid:S2095") - OutputStream outputStream = ByteStreams.nullOutputStream(); + final PipedOutputStream outputStream = new PipedOutputStream(); + PipedInputStream inputStream = null; try { - outputStream = createOutputStreamForTempFile(); - publishUploadProgressEvent(fileUploadId, 0, fileSize, getTempFilePath()); - + inputStream = new PipedInputStream(outputStream); + publishUploadProgressEvent(fileUploadId, 0, fileSize); + startTransferToRepositoryThread(inputStream, fileUploadId, mimeType); } catch (final IOException e) { - LOG.error("Creating temp file for upload failed {}.", e); - try { - outputStream.close(); - } catch (final IOException e1) { - LOG.error("Closing output stream caused an exception {}", e1); - } - + LOG.error("Creating piped Stream failed {}.", e); + tryToCloseIOStream(outputStream); + tryToCloseIOStream(inputStream); setFailureReasonUploadFailed(); setUploadInterrupted(); + getUploadState().updateFileUploadProgress(fileUploadId, + new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); + return ByteStreams.nullOutputStream(); } - return outputStream; } @@ -124,7 +128,7 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl return; } - publishUploadProgressEvent(fileUploadId, event.getBytesReceived(), event.getContentLength(), getTempFilePath()); + publishUploadProgressEvent(fileUploadId, event.getBytesReceived(), event.getContentLength()); } /** @@ -137,8 +141,6 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl @Override public void streamingFinished(final StreamingEndEvent event) { assertStateConsistency(fileUploadId, event.getFileName()); - - transferArtifactToRepository(fileUploadId, event.getContentLength(), mimeType, getTempFilePath()); } /** @@ -157,6 +159,7 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl } else { publishUploadFailedEvent(fileUploadId, event.getException()); } + setUploadInterrupted(); publishUploadFinishedEvent(fileUploadId); } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java index 08589c5eb..f6fe169c7 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java @@ -10,6 +10,8 @@ package org.eclipse.hawkbit.ui.artifacts.upload; import java.io.IOException; import java.io.OutputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; import java.util.Optional; import org.eclipse.hawkbit.repository.ArtifactManagement; @@ -51,7 +53,6 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler private final transient SoftwareModuleManagement softwareModuleManagement; private final long maxSize; - private volatile String mimeType; private volatile FileUploadId fileUploadId; FileTransferHandlerVaadinUpload(final long maxSize, final SoftwareModuleManagement softwareManagement, @@ -70,12 +71,12 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler public void uploadStarted(final StartedEvent event) { // reset internal state here because instance is reused for next upload! resetState(); - this.mimeType = null; this.fileUploadId = null; assertThatOneSoftwareModuleIsSelected(); - // selected software module at the time of this callback is considered + // selected software module at the time of this callback is + // considered SoftwareModule softwareModule = null; final Optional selectedSoftwareModuleId = getUploadState().getSelectedBaseSwModuleId(); final Long softwareModuleId = selectedSoftwareModuleId.orElse(null); @@ -84,7 +85,6 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler } this.fileUploadId = new FileUploadId(event.getFilename(), softwareModule); - this.mimeType = event.getMIMEType(); if (getUploadState().isFileInUploadState(this.fileUploadId)) { setFailureReasonUploadFailed(); @@ -131,26 +131,23 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler // we return the outputstream so we cannot close it here @SuppressWarnings("squid:S2095") - OutputStream outputStream = ByteStreams.nullOutputStream(); + final PipedOutputStream outputStream = new PipedOutputStream(); + PipedInputStream inputStream = null; try { - outputStream = createOutputStreamForTempFile(); - this.mimeType = mimeType; - publishUploadProgressEvent(fileUploadId, 0, 0, getTempFilePath()); - + inputStream = new PipedInputStream(outputStream); + publishUploadProgressEvent(fileUploadId, 0, 0); + startTransferToRepositoryThread(inputStream, fileUploadId, mimeType); } catch (final IOException e) { - LOG.error("Creating temp file for upload failed {}.", e); - if (outputStream != null) { - try { - outputStream.close(); - } catch (final IOException e1) { - LOG.error("Closing output stream caused an exception {}", e1); - } - } - + LOG.error("Creating piped Stream failed {}.", e); + tryToCloseIOStream(outputStream); + tryToCloseIOStream(inputStream); setFailureReasonUploadFailed(); setUploadInterrupted(); + getUploadState().updateFileUploadProgress(fileUploadId, + new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); + uiNotification.displayWarning("try again"); + return ByteStreams.nullOutputStream(); } - return outputStream; } @@ -173,7 +170,7 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler return; } - publishUploadProgressEvent(fileUploadId, readBytes, contentLength, getTempFilePath()); + publishUploadProgressEvent(fileUploadId, readBytes, contentLength); } /** @@ -190,8 +187,6 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler return; } assertStateConsistency(fileUploadId, event.getFilename()); - - transferArtifactToRepository(fileUploadId, event.getLength(), mimeType, getTempFilePath()); } /** @@ -220,6 +215,7 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler } else { publishUploadFailedEvent(fileUploadId, event.getReason()); } + setUploadInterrupted(); publishUploadFinishedEvent(fileUploadId); } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileUploadProgress.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileUploadProgress.java index 2f1034a58..f0217974c 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileUploadProgress.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileUploadProgress.java @@ -75,6 +75,27 @@ public class FileUploadProgress implements Serializable { this.fileUploadStatus = fileUploadStatus; } + /** + * Creates a new {@link FileUploadProgress} instance. + * + * @param fileUploadId + * the {@link FileUploadId} to which this progress information + * belongs. + * @param fileUploadStatus + * the {@link FileUploadStatus} of this progress + * @param bytesRead + * number of bytes read + * @param contentLength + * size of the file in bytes + */ + FileUploadProgress(final FileUploadId fileUploadId, final FileUploadStatus fileUploadStatus, + final long bytesRead, final long contentLength) { + this.fileUploadId = fileUploadId; + this.fileUploadStatus = fileUploadStatus; + this.contentLength = contentLength; + this.bytesRead = bytesRead; + } + /** * Creates a new {@link FileUploadProgress} instance. * diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java index 530cb1df0..1b5c1d745 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java @@ -10,7 +10,6 @@ package org.eclipse.hawkbit.ui.artifacts.upload; import javax.servlet.MultipartConfigElement; -import org.apache.commons.lang3.StringUtils; import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.SoftwareModuleManagement; import org.eclipse.hawkbit.repository.model.SoftwareModule; @@ -179,39 +178,28 @@ public class UploadDropAreaLayout extends AbstractComponent { private void uploadFilesForSoftwareModule(final Html5File[] files, final Long softwareModuleId) { final SoftwareModule softwareModule = softwareManagement.get(softwareModuleId).orElse(null); - boolean isDirectory = false; - boolean isDuplicate = false; + boolean duplicateFound = false; for (final Html5File file : files) { - - isDirectory = isDirectory(file); - isDuplicate = artifactUploadState.isFileInUploadState(file.getFileName(), softwareModule); - - if (!isDirectory && !isDuplicate) { + if (artifactUploadState.isFileInUploadState(file.getFileName(), softwareModule)) { + duplicateFound = true; + } else { file.setStreamVariable(new FileTransferHandlerStreamVariable(file.getFileName(), file.getFileSize(), multipartConfigElement.getMaxFileSize(), file.getType(), softwareModule, artifactManagement, i18n)); } } - if (isDirectory && isDuplicate) { - uiNotification.displayValidationError(i18n.getMessage("message.no.duplicateFiles") + "
" - + i18n.getMessage("message.no.directory.upload")); - } else if (isDirectory) { - uiNotification.displayValidationError(i18n.getMessage("message.no.directory.upload")); - } else if (isDuplicate) { - uiNotification.displayValidationError(i18n.getMessage("message.no.duplicateFiles")); + if (duplicateFound) { + uiNotification.displayValidationError( + i18n.getMessage("message.no.duplicateFiles")); } } - private boolean isDirectory(final Html5File file) { - return StringUtils.isBlank(file.getType()) && file.getFileSize() % 4096 == 0; - } - private boolean validate(final DragAndDropEvent event) { // check if drop is valid.If valid ,check if software module is // selected. if (!isFilesDropped(event)) { - uiNotification.displayValidationError(i18n.getMessage(UIMessageIdProvider.MESSAGE_ACTION_NOT_ALLOWED)); + uiNotification.displayValidationError(i18n.getMessage("message.drop.onlyFiles")); return false; } return validateSoftwareModuleSelection(); diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/SpringContextHelper.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/SpringContextHelper.java index 6e3fa4db1..d268f8aca 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/SpringContextHelper.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/SpringContextHelper.java @@ -55,4 +55,19 @@ public final class SpringContextHelper { return context.getBean(beanClazz); } + /** + * method to return a certain bean by its class and name. + * + * @param beanName + * name of the beand which should be returned from the + * application context + * @param beanClazz + * class of the bean which should be returned from the + * application context + * @return the requested bean + */ + public static T getBean(final String beanName, final Class beanClazz) { + return context.getBean(beanName, beanClazz); + } + } diff --git a/hawkbit-ui/src/main/resources/messages.properties b/hawkbit-ui/src/main/resources/messages.properties index 4635cf32e..704831eaf 100644 --- a/hawkbit-ui/src/main/resources/messages.properties +++ b/hawkbit-ui/src/main/resources/messages.properties @@ -393,6 +393,7 @@ message.error.missing.tagname = Please select tag name message.type.delete = Please unclick the distribution type {0}, then try to delete message.error.dist.set.type.update= Distribution set type is already assigned to set(s) and cannot be changed message.no.directory.upload = Directory upload is not supported +message.drop.onlyFiles = Please drop files only message.delete.filter.confirm = Are you sure you want to delete custom filter? message.delete.filter.success = Custom filter {0} deleted Successfully! message.create.filter.success = Custom filter {0} created Successfully!