Fix upload quota check and provide better error message (#893)
* Adjusted upload quota check to include file size and show proper error message Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Fixed failing upload quota tests Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Moved quota check to stream, fixed review findings Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Added missing license header to QuotaInputStream Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Reworked uploadLock, ensured error messages may be translated, review fixes Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Added local artifactrepo to gitignore Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Fixed sonar issues and assignment quota message Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * PR review fixes Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Split quota exceptions, PR fixes Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Removed left over conversion method in quota helper Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com> * Made conversion helper class final Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com>
This commit is contained in:
committed by
Dominic Schabel
parent
9b8ab40c55
commit
09f2d8a481
@@ -14,14 +14,18 @@ import java.io.OutputStream;
|
||||
import java.io.Serializable;
|
||||
import java.util.Optional;
|
||||
import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.locks.Lock;
|
||||
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.eclipse.hawkbit.repository.ArtifactManagement;
|
||||
import org.eclipse.hawkbit.repository.RegexCharacterCollection;
|
||||
import org.eclipse.hawkbit.repository.RegexCharacterCollection.RegexChar;
|
||||
import org.eclipse.hawkbit.repository.exception.ArtifactUploadFailedException;
|
||||
import org.eclipse.hawkbit.repository.exception.FileSizeQuotaExceededException;
|
||||
import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException;
|
||||
import org.eclipse.hawkbit.repository.exception.InvalidSHA1HashException;
|
||||
import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException;
|
||||
import org.eclipse.hawkbit.repository.exception.StorageQuotaExceededException;
|
||||
import org.eclipse.hawkbit.repository.model.Artifact;
|
||||
import org.eclipse.hawkbit.repository.model.ArtifactUpload;
|
||||
import org.eclipse.hawkbit.repository.model.SoftwareModule;
|
||||
@@ -36,7 +40,6 @@ import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.vaadin.spring.events.EventBus;
|
||||
import org.vaadin.spring.events.EventBus.UIEventBus;
|
||||
|
||||
import com.vaadin.ui.UI;
|
||||
|
||||
/**
|
||||
@@ -48,6 +51,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
private static final Logger LOG = LoggerFactory.getLogger(AbstractFileTransferHandler.class);
|
||||
private static final String MESSAGE_UPLOAD_FAILED = "message.upload.failed";
|
||||
|
||||
private volatile boolean uploadInterrupted;
|
||||
|
||||
@@ -63,15 +67,18 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
|
||||
protected final UINotification uiNotification;
|
||||
|
||||
private final transient Lock uploadLock;
|
||||
|
||||
protected static final RegexCharacterCollection ILLEGAL_FILENAME_CHARACTERS = new RegexCharacterCollection(
|
||||
RegexChar.GREATER_THAN, RegexChar.LESS_THAN, RegexChar.SLASHES);
|
||||
|
||||
AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n) {
|
||||
AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n, final Lock uploadLock) {
|
||||
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);
|
||||
this.uploadLock = uploadLock;
|
||||
}
|
||||
|
||||
protected boolean isUploadInterrupted() {
|
||||
@@ -94,7 +101,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
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()));
|
||||
new TransferArtifactToRepositoryRunnable(inputStream, fileUploadId, mimeType, UI.getCurrent(), uploadLock));
|
||||
}
|
||||
|
||||
private void interruptUploadAndSetReason(final String failureReason) {
|
||||
@@ -103,7 +110,19 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
}
|
||||
|
||||
protected void interruptUploadDueToUploadFailed() {
|
||||
interruptUploadAndSetReason(i18n.getMessage("message.upload.failed"));
|
||||
interruptUploadAndSetReason(i18n.getMessage(MESSAGE_UPLOAD_FAILED));
|
||||
}
|
||||
|
||||
protected void interruptUploadDueToAssignmentQuotaExceeded() {
|
||||
interruptUploadAndSetReason(i18n.getMessage("message.upload.assignmentQuota"));
|
||||
}
|
||||
|
||||
protected void interruptUploadDueToFileSizeQuotaExceeded(final String exceededValue) {
|
||||
interruptUploadAndSetReason(i18n.getMessage("message.upload.fileSizeQuota", exceededValue));
|
||||
}
|
||||
|
||||
protected void interruptUploadDueToStorageQuotaExceeded(final String exceededValue) {
|
||||
interruptUploadAndSetReason(i18n.getMessage("message.upload.storageQuota", exceededValue));
|
||||
}
|
||||
|
||||
protected void interruptUploadDueToDuplicateFile() {
|
||||
@@ -177,7 +196,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
|
||||
final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId,
|
||||
FileUploadStatus.UPLOAD_FAILED,
|
||||
StringUtils.isBlank(failureReason) ? i18n.getMessage("message.upload.failed") : failureReason);
|
||||
StringUtils.isBlank(failureReason) ? i18n.getMessage(MESSAGE_UPLOAD_FAILED) : failureReason);
|
||||
artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress);
|
||||
eventBus.publish(this, fileUploadProgress);
|
||||
publishUploadFinishedEvent(fileUploadId);
|
||||
@@ -216,26 +235,45 @@ public abstract class AbstractFileTransferHandler implements Serializable {
|
||||
private final FileUploadId fileUploadId;
|
||||
private final String mimeType;
|
||||
private final UI vaadinUi;
|
||||
private final Lock uploadLock;
|
||||
|
||||
public TransferArtifactToRepositoryRunnable(final InputStream inputStream, final FileUploadId fileUploadId,
|
||||
final String mimeType, final UI vaadinUi) {
|
||||
final String mimeType, final UI vaadinUi, final Lock uploadLock) {
|
||||
this.inputStream = inputStream;
|
||||
this.fileUploadId = fileUploadId;
|
||||
this.mimeType = mimeType;
|
||||
this.vaadinUi = vaadinUi;
|
||||
this.uploadLock = uploadLock;
|
||||
}
|
||||
|
||||
/**
|
||||
* a lock object is used here that is propagated down from
|
||||
* {@link org.eclipse.hawkbit.ui.artifacts.UploadArtifactView}. It ensures that from within the same UI instance
|
||||
* all uploads are executed sequentially to avoid issues that occur when multiple files are processed at the
|
||||
* same time (e.g. regarding quota checks)
|
||||
*/
|
||||
@Override
|
||||
public void run() {
|
||||
try {
|
||||
UI.setCurrent(vaadinUi);
|
||||
uploadLock.lock();
|
||||
streamToRepository();
|
||||
} catch (final FileSizeQuotaExceededException e) {
|
||||
interruptUploadDueToFileSizeQuotaExceeded(e.getExceededQuotaValueString());
|
||||
LOG.debug("Upload failed due to file size quota exceeded:", e);
|
||||
} catch (final StorageQuotaExceededException e) {
|
||||
interruptUploadDueToStorageQuotaExceeded(e.getExceededQuotaValueString());
|
||||
LOG.debug("Upload failed due to storage quota exceeded:", e);
|
||||
} catch (final AssignmentQuotaExceededException e) {
|
||||
interruptUploadDueToAssignmentQuotaExceeded();
|
||||
LOG.debug("Upload failed due to assignment quota exceeded:", e);
|
||||
} catch (final RuntimeException e) {
|
||||
interruptUploadDueToUploadFailed();
|
||||
publishUploadFailedAndFinishedEvent(fileUploadId, e);
|
||||
LOG.error("Failed to transfer file to repository", e);
|
||||
} finally {
|
||||
tryToCloseIOStream(inputStream);
|
||||
uploadLock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ import java.io.IOException;
|
||||
import java.io.OutputStream;
|
||||
import java.io.PipedInputStream;
|
||||
import java.io.PipedOutputStream;
|
||||
import java.util.concurrent.locks.Lock;
|
||||
|
||||
import org.eclipse.hawkbit.repository.ArtifactManagement;
|
||||
import org.eclipse.hawkbit.repository.RegexCharacterCollection;
|
||||
@@ -48,8 +49,8 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl
|
||||
|
||||
FileTransferHandlerStreamVariable(final String fileName, final long fileSize, final long maxSize,
|
||||
final String mimeType, final SoftwareModule selectedSw, final ArtifactManagement artifactManagement,
|
||||
final VaadinMessageSource i18n) {
|
||||
super(artifactManagement, i18n);
|
||||
final VaadinMessageSource i18n, final Lock uploadLock) {
|
||||
super(artifactManagement, i18n, uploadLock);
|
||||
this.fileSize = fileSize;
|
||||
this.maxSize = maxSize;
|
||||
this.mimeType = mimeType;
|
||||
|
||||
@@ -12,6 +12,7 @@ import java.io.IOException;
|
||||
import java.io.OutputStream;
|
||||
import java.io.PipedInputStream;
|
||||
import java.io.PipedOutputStream;
|
||||
import java.util.concurrent.locks.Lock;
|
||||
|
||||
import org.eclipse.hawkbit.repository.ArtifactManagement;
|
||||
import org.eclipse.hawkbit.repository.RegexCharacterCollection;
|
||||
@@ -55,8 +56,8 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler
|
||||
private volatile FileUploadId fileUploadId;
|
||||
|
||||
FileTransferHandlerVaadinUpload(final long maxSize, final SoftwareModuleManagement softwareManagement,
|
||||
final ArtifactManagement artifactManagement, final VaadinMessageSource i18n) {
|
||||
super(artifactManagement, i18n);
|
||||
final ArtifactManagement artifactManagement, final VaadinMessageSource i18n, final Lock uploadLock) {
|
||||
super(artifactManagement, i18n, uploadLock);
|
||||
this.maxSize = maxSize;
|
||||
this.softwareModuleManagement = softwareManagement;
|
||||
}
|
||||
|
||||
@@ -42,6 +42,9 @@ import com.vaadin.ui.Label;
|
||||
import com.vaadin.ui.UI;
|
||||
import com.vaadin.ui.VerticalLayout;
|
||||
|
||||
import java.util.concurrent.locks.Lock;
|
||||
import java.util.concurrent.locks.ReentrantLock;
|
||||
|
||||
/**
|
||||
* Container for drag and drop area in the upload view.
|
||||
*/
|
||||
@@ -68,6 +71,8 @@ public class UploadDropAreaLayout extends AbstractComponent {
|
||||
|
||||
private final UploadProgressButtonLayout uploadButtonLayout;
|
||||
|
||||
private final transient Lock uploadLock = new ReentrantLock();
|
||||
|
||||
/**
|
||||
* Creates a new {@link UploadDropAreaLayout} instance.
|
||||
*
|
||||
@@ -99,7 +104,7 @@ public class UploadDropAreaLayout extends AbstractComponent {
|
||||
this.softwareManagement = softwareManagement;
|
||||
this.artifactManagement = artifactManagement;
|
||||
this.uploadButtonLayout = new UploadProgressButtonLayout(i18n, eventBus, artifactUploadState,
|
||||
multipartConfigElement, artifactManagement, softwareManagement);
|
||||
multipartConfigElement, artifactManagement, softwareManagement, uploadLock);
|
||||
|
||||
buildLayout();
|
||||
|
||||
@@ -186,7 +191,7 @@ public class UploadDropAreaLayout extends AbstractComponent {
|
||||
} else {
|
||||
file.setStreamVariable(new FileTransferHandlerStreamVariable(file.getFileName(), file.getFileSize(),
|
||||
multipartConfigElement.getMaxFileSize(), file.getType(), softwareModule, artifactManagement,
|
||||
i18n));
|
||||
i18n, uploadLock));
|
||||
}
|
||||
}
|
||||
if (duplicateFound) {
|
||||
|
||||
@@ -29,6 +29,8 @@ import com.vaadin.ui.Button;
|
||||
import com.vaadin.ui.UI;
|
||||
import com.vaadin.ui.VerticalLayout;
|
||||
|
||||
import java.util.concurrent.locks.Lock;
|
||||
|
||||
/**
|
||||
* Container for upload and progress button.
|
||||
*/
|
||||
@@ -54,6 +56,8 @@ public class UploadProgressButtonLayout extends VerticalLayout {
|
||||
|
||||
private final ArtifactUploadState artifactUploadState;
|
||||
|
||||
private final transient Lock uploadLock;
|
||||
|
||||
/**
|
||||
* Creates a new {@link UploadProgressButtonLayout} instance.
|
||||
*
|
||||
@@ -71,10 +75,12 @@ public class UploadProgressButtonLayout extends VerticalLayout {
|
||||
* @param artifactManagement
|
||||
* the {@link ArtifactManagement} for storing the uploaded
|
||||
* artifacts
|
||||
* @param uploadLock
|
||||
* A common upload lock that enforced sequential upload within an UI instance
|
||||
*/
|
||||
public UploadProgressButtonLayout(final VaadinMessageSource i18n, final UIEventBus eventBus,
|
||||
final ArtifactUploadState artifactUploadState, final MultipartConfigElement multipartConfigElement,
|
||||
final ArtifactManagement artifactManagement, final SoftwareModuleManagement softwareManagement) {
|
||||
final ArtifactManagement artifactManagement, final SoftwareModuleManagement softwareManagement, final Lock uploadLock) {
|
||||
this.artifactUploadState = artifactUploadState;
|
||||
this.artifactManagement = artifactManagement;
|
||||
this.uploadInfoWindow = new UploadProgressInfoWindow(eventBus, artifactUploadState, i18n);
|
||||
@@ -89,6 +95,7 @@ public class UploadProgressButtonLayout extends VerticalLayout {
|
||||
this.multipartConfigElement = multipartConfigElement;
|
||||
this.softwareModuleManagement = softwareManagement;
|
||||
this.upload = new UploadFixed();
|
||||
this.uploadLock = uploadLock;
|
||||
|
||||
createComponents();
|
||||
buildLayout();
|
||||
@@ -144,7 +151,7 @@ public class UploadProgressButtonLayout extends VerticalLayout {
|
||||
|
||||
private void buildLayout() {
|
||||
final FileTransferHandlerVaadinUpload uploadHandler = new FileTransferHandlerVaadinUpload(
|
||||
multipartConfigElement.getMaxFileSize(), softwareModuleManagement, artifactManagement, i18n);
|
||||
multipartConfigElement.getMaxFileSize(), softwareModuleManagement, artifactManagement, i18n, uploadLock);
|
||||
upload.setButtonCaption(i18n.getMessage("upload.file"));
|
||||
upload.setImmediate(true);
|
||||
upload.setReceiver(uploadHandler);
|
||||
|
||||
@@ -25,6 +25,7 @@ import com.vaadin.server.ClientConnector.ConnectorErrorEvent;
|
||||
import com.vaadin.server.DefaultErrorHandler;
|
||||
import com.vaadin.server.ErrorEvent;
|
||||
import com.vaadin.server.Page;
|
||||
import com.vaadin.server.UploadException;
|
||||
import com.vaadin.shared.Connector;
|
||||
import com.vaadin.ui.Component;
|
||||
import com.vaadin.ui.Notification.Type;
|
||||
@@ -45,6 +46,11 @@ public class HawkbitUIErrorHandler extends DefaultErrorHandler {
|
||||
@Override
|
||||
public void error(final ErrorEvent event) {
|
||||
|
||||
// filter upload exceptions
|
||||
if (event.getThrowable() instanceof UploadException) {
|
||||
return;
|
||||
}
|
||||
|
||||
final HawkbitErrorNotificationMessage message = buildNotification(getRootExceptionFrom(event));
|
||||
if (event instanceof ConnectorErrorEvent) {
|
||||
final Connector connector = ((ConnectorErrorEvent) event).getConnector();
|
||||
|
||||
@@ -465,7 +465,10 @@ message.no.duplicateFiles = Duplicate files selected
|
||||
message.delete.artifact = Are you sure you want to delete artifact {0} ?
|
||||
message.swModule.deleted = {0} Software Module(s) deleted
|
||||
message.error.swModule.notDeleted = Upload is running for Software Module(s)
|
||||
message.upload.failed = Streaming Failed
|
||||
message.upload.failed = Upload failed
|
||||
message.upload.assignmentQuota = Max assignments per software module exceeded
|
||||
message.upload.fileSizeQuota = Maximum artifact size ({0}) exceeded
|
||||
message.upload.storageQuota = Storage quota exceeded, {0} left
|
||||
message.uploadedfile.size.exceeded = File size exceeded. Allowed size {0} bytes
|
||||
message.uploadedfile.illegalFilename = Filename contains illegal characters
|
||||
message.artifact.deleted = Artifact with file {0} deleted successfully
|
||||
|
||||
Reference in New Issue
Block a user