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 <stefan.klotz@bosch-si.com>
Signed-off-by: Stefan Behl <stefan.behl@bosch-si.com>
This commit is contained in:
Stefan Klotz
2018-11-12 08:57:10 +01:00
committed by Stefan Behl
parent 49d064df18
commit d4c1e82090
11 changed files with 266 additions and 137 deletions

View File

@@ -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<Artifact> 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);
}
}
}

View File

@@ -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);
}

View File

@@ -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<Long> 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);
}

View File

@@ -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.
*

View File

@@ -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") + "<br>"
+ 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();

View File

@@ -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> T getBean(final String beanName, final Class<T> beanClazz) {
return context.getBean(beanName, beanClazz);
}
}

View File

@@ -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!