Fixed error message not shown when exceeding filesize (#906)

* Fixed message not showing when exceeding filesize

Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com>

* Also use fixed max filesize approach during drag and drop

Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com>

* Moved log messages and adjusted upload log levels

Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com>

* Further adjustments of logging

Signed-off-by: Alexander Dobler <alexander.dobler3@bosch-si.com>
This commit is contained in:
Alexander Dobler
2019-11-08 11:11:32 +01:00
committed by Dominic Schabel
parent 9cb5d31396
commit 51982a77e3
4 changed files with 57 additions and 56 deletions

View File

@@ -12,7 +12,6 @@ import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
import java.io.Serializable; import java.io.Serializable;
import java.util.Optional;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.Lock;
@@ -21,10 +20,10 @@ import org.eclipse.hawkbit.repository.ArtifactManagement;
import org.eclipse.hawkbit.repository.RegexCharacterCollection; import org.eclipse.hawkbit.repository.RegexCharacterCollection;
import org.eclipse.hawkbit.repository.RegexCharacterCollection.RegexChar; import org.eclipse.hawkbit.repository.RegexCharacterCollection.RegexChar;
import org.eclipse.hawkbit.repository.exception.ArtifactUploadFailedException; import org.eclipse.hawkbit.repository.exception.ArtifactUploadFailedException;
import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException;
import org.eclipse.hawkbit.repository.exception.FileSizeQuotaExceededException; import org.eclipse.hawkbit.repository.exception.FileSizeQuotaExceededException;
import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException; import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException;
import org.eclipse.hawkbit.repository.exception.InvalidSHA1HashException; 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.exception.StorageQuotaExceededException;
import org.eclipse.hawkbit.repository.model.Artifact; import org.eclipse.hawkbit.repository.model.Artifact;
import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.ArtifactUpload;
@@ -40,6 +39,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.vaadin.spring.events.EventBus; import org.vaadin.spring.events.EventBus;
import org.vaadin.spring.events.EventBus.UIEventBus; import org.vaadin.spring.events.EventBus.UIEventBus;
import com.vaadin.ui.UI; import com.vaadin.ui.UI;
/** /**
@@ -72,7 +72,8 @@ public abstract class AbstractFileTransferHandler implements Serializable {
protected static final RegexCharacterCollection ILLEGAL_FILENAME_CHARACTERS = new RegexCharacterCollection( protected static final RegexCharacterCollection ILLEGAL_FILENAME_CHARACTERS = new RegexCharacterCollection(
RegexChar.GREATER_THAN, RegexChar.LESS_THAN, RegexChar.SLASHES); RegexChar.GREATER_THAN, RegexChar.LESS_THAN, RegexChar.SLASHES);
AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n, final Lock uploadLock) { AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n,
final Lock uploadLock) {
this.artifactManagement = artifactManagement; this.artifactManagement = artifactManagement;
this.i18n = i18n; this.i18n = i18n;
this.eventBus = SpringContextHelper.getBean(EventBus.UIEventBus.class); this.eventBus = SpringContextHelper.getBean(EventBus.UIEventBus.class);
@@ -100,8 +101,9 @@ public abstract class AbstractFileTransferHandler implements Serializable {
protected void startTransferToRepositoryThread(final InputStream inputStream, final FileUploadId fileUploadId, protected void startTransferToRepositoryThread(final InputStream inputStream, final FileUploadId fileUploadId,
final String mimeType) { final String mimeType) {
SpringContextHelper.getBean("asyncExecutor", ExecutorService.class).execute( SpringContextHelper.getBean("asyncExecutor", ExecutorService.class)
new TransferArtifactToRepositoryRunnable(inputStream, fileUploadId, mimeType, UI.getCurrent(), uploadLock)); .execute(new TransferArtifactToRepositoryRunnable(inputStream, fileUploadId, mimeType, UI.getCurrent(),
uploadLock));
} }
private void interruptUploadAndSetReason(final String failureReason) { private void interruptUploadAndSetReason(final String failureReason) {
@@ -129,10 +131,6 @@ public abstract class AbstractFileTransferHandler implements Serializable {
interruptUploadAndSetReason(i18n.getMessage("message.no.duplicateFiles")); interruptUploadAndSetReason(i18n.getMessage("message.no.duplicateFiles"));
} }
protected void interruptUploadDueToFileSizeExceeded(final long maxSize) {
interruptUploadAndSetReason(i18n.getMessage("message.uploadedfile.size.exceeded", maxSize));
}
protected void interruptUploadDueToIllegalFilename() { protected void interruptUploadDueToIllegalFilename() {
interruptUploadAndSetReason(i18n.getMessage("message.uploadedfile.illegalFilename")); interruptUploadAndSetReason(i18n.getMessage("message.uploadedfile.illegalFilename"));
} }
@@ -170,7 +168,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
} }
protected void publishUploadFinishedEvent(final FileUploadId fileUploadId) { protected void publishUploadFinishedEvent(final FileUploadId fileUploadId) {
LOG.info("Upload finished for file {}", fileUploadId); LOG.debug("Upload finished for file {}", fileUploadId);
final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId,
FileUploadStatus.UPLOAD_FINISHED); FileUploadStatus.UPLOAD_FINISHED);
eventBus.publish(this, fileUploadProgress); eventBus.publish(this, fileUploadProgress);
@@ -184,21 +182,22 @@ public abstract class AbstractFileTransferHandler implements Serializable {
eventBus.publish(this, fileUploadProgress); eventBus.publish(this, fileUploadProgress);
} }
protected void publishArtifactsChanged(final FileUploadId fileUploadId) { protected void publishUploadFailedEvent(final FileUploadId fileUploadId) {
eventBus.publish(this, LOG.info("Upload failed for file {} due to reason: {}", fileUploadId, failureReason);
new SoftwareModuleEvent(SoftwareModuleEventType.ARTIFACTS_CHANGED, fileUploadId.getSoftwareModuleId()));
}
protected void publishUploadFailedAndFinishedEvent(final FileUploadId fileUploadId,
final Exception uploadException) {
LOG.info("Upload failed for file {} due to reason: {}, exception: {}", fileUploadId, failureReason,
uploadException.getMessage());
final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId,
FileUploadStatus.UPLOAD_FAILED, 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); artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress);
eventBus.publish(this, fileUploadProgress); eventBus.publish(this, fileUploadProgress);
}
protected void publishArtifactsChanged(final FileUploadId fileUploadId) {
eventBus.publish(this,
new SoftwareModuleEvent(SoftwareModuleEventType.ARTIFACTS_CHANGED, fileUploadId.getSoftwareModuleId()));
}
protected void publishUploadFailedAndFinishedEvent(final FileUploadId fileUploadId) {
publishUploadFailedEvent(fileUploadId);
publishUploadFinishedEvent(fileUploadId); publishUploadFinishedEvent(fileUploadId);
} }
@@ -214,7 +213,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
try { try {
outputStream.close(); outputStream.close();
} catch (final IOException e1) { } catch (final IOException e1) {
LOG.error("Closing output stream caused an exception {}", e1); LOG.warn("Closing output stream caused an exception {}", e1);
} }
} }
@@ -225,7 +224,7 @@ public abstract class AbstractFileTransferHandler implements Serializable {
try { try {
inputStream.close(); inputStream.close();
} catch (final IOException e1) { } catch (final IOException e1) {
LOG.error("Closing input stream caused an exception {}", e1); LOG.warn("Closing input stream caused an exception {}", e1);
} }
} }
} }
@@ -248,9 +247,10 @@ public abstract class AbstractFileTransferHandler implements Serializable {
/** /**
* a lock object is used here that is propagated down from * 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 * {@link org.eclipse.hawkbit.ui.artifacts.UploadArtifactView}. It
* all uploads are executed sequentially to avoid issues that occur when multiple files are processed at the * ensures that from within the same UI instance all uploads are
* same time (e.g. regarding quota checks) * executed sequentially to avoid issues that occur when multiple files
* are processed at the same time (e.g. regarding quota checks)
*/ */
@Override @Override
public void run() { public void run() {
@@ -269,8 +269,8 @@ public abstract class AbstractFileTransferHandler implements Serializable {
LOG.debug("Upload failed due to assignment quota exceeded:", e); LOG.debug("Upload failed due to assignment quota exceeded:", e);
} catch (final RuntimeException e) { } catch (final RuntimeException e) {
interruptUploadDueToUploadFailed(); interruptUploadDueToUploadFailed();
publishUploadFailedAndFinishedEvent(fileUploadId, e); publishUploadFailedAndFinishedEvent(fileUploadId);
LOG.error("Failed to transfer file to repository", e); LOG.warn("Failed to transfer file to repository", e);
} finally { } finally {
tryToCloseIOStream(inputStream); tryToCloseIOStream(inputStream);
uploadLock.unlock(); uploadLock.unlock();
@@ -282,8 +282,8 @@ public abstract class AbstractFileTransferHandler implements Serializable {
throw new ArtifactUploadFailedException(); throw new ArtifactUploadFailedException();
} }
final String filename = fileUploadId.getFilename(); final String filename = fileUploadId.getFilename();
LOG.info("Transfering file {} directly to repository", filename); LOG.debug("Transfering file {} directly to repository", filename);
final Artifact artifact = uploadArtifact(filename).orElseThrow(ArtifactUploadFailedException::new); final Artifact artifact = uploadArtifact(filename);
if (isUploadInterrupted()) { if (isUploadInterrupted()) {
handleUploadFailure(artifact); handleUploadFailure(artifact);
publishUploadFinishedEvent(fileUploadId); publishUploadFinishedEvent(fileUploadId);
@@ -294,13 +294,12 @@ public abstract class AbstractFileTransferHandler implements Serializable {
publishArtifactsChanged(fileUploadId); publishArtifactsChanged(fileUploadId);
} }
private Optional<Artifact> uploadArtifact(final String filename) { private Artifact uploadArtifact(final String filename) {
try { try {
return Optional.ofNullable(artifactManagement.create(new ArtifactUpload(inputStream, return artifactManagement.create(new ArtifactUpload(inputStream, fileUploadId.getSoftwareModuleId(),
fileUploadId.getSoftwareModuleId(), filename, null, null, true, mimeType, -1))); filename, null, null, true, mimeType, -1));
} catch (final ArtifactUploadFailedException | InvalidSHA1HashException | InvalidMD5HashException e) { } catch (final ArtifactUploadFailedException | InvalidSHA1HashException | InvalidMD5HashException e) {
LOG.error("Failed to transfer file to repository", e); throw new ArtifactUploadFailedException(e);
return Optional.empty();
} }
} }

View File

@@ -16,6 +16,7 @@ import java.util.concurrent.locks.Lock;
import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.ArtifactManagement;
import org.eclipse.hawkbit.repository.RegexCharacterCollection; import org.eclipse.hawkbit.repository.RegexCharacterCollection;
import org.eclipse.hawkbit.repository.SizeConversionHelper;
import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource;
import org.slf4j.Logger; import org.slf4j.Logger;
@@ -65,10 +66,10 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl
assertStateConsistency(fileUploadId, event.getFileName()); assertStateConsistency(fileUploadId, event.getFileName());
if (RegexCharacterCollection.stringContainsCharacter(event.getFileName(), ILLEGAL_FILENAME_CHARACTERS)) { if (RegexCharacterCollection.stringContainsCharacter(event.getFileName(), ILLEGAL_FILENAME_CHARACTERS)) {
LOG.info("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(), fileUploadId); LOG.debug("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(), fileUploadId);
interruptUploadDueToIllegalFilename(); interruptUploadDueToIllegalFilename();
} else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, selectedSoftwareModule)) { } else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, selectedSoftwareModule)) {
LOG.info("File {} already contained in Software Module {}", fileUploadId.getFilename(), LOG.debug("File {} already contained in Software Module {}", fileUploadId.getFilename(),
selectedSoftwareModule); selectedSoftwareModule);
interruptUploadDueToDuplicateFile(); interruptUploadDueToDuplicateFile();
} }
@@ -89,11 +90,11 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl
publishUploadProgressEvent(fileUploadId, 0, fileSize); publishUploadProgressEvent(fileUploadId, 0, fileSize);
startTransferToRepositoryThread(inputStream, fileUploadId, mimeType); startTransferToRepositoryThread(inputStream, fileUploadId, mimeType);
} catch (final IOException e) { } catch (final IOException e) {
LOG.error("Creating piped Stream failed {}.", e); LOG.warn("Creating piped Stream failed {}.", e);
tryToCloseIOStream(outputStream); tryToCloseIOStream(outputStream);
tryToCloseIOStream(inputStream); tryToCloseIOStream(inputStream);
interruptUploadDueToUploadFailed(); interruptUploadDueToUploadFailed();
publishUploadFailedAndFinishedEvent(fileUploadId, e); publishUploadFailedAndFinishedEvent(fileUploadId);
return ByteStreams.nullOutputStream(); return ByteStreams.nullOutputStream();
} }
return outputStream; return outputStream;
@@ -118,12 +119,13 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl
public void onProgress(final StreamingProgressEvent event) { public void onProgress(final StreamingProgressEvent event) {
assertStateConsistency(fileUploadId, event.getFileName()); assertStateConsistency(fileUploadId, event.getFileName());
if (event.getBytesReceived() > maxSize || event.getContentLength() > maxSize) { if (isUploadInterrupted()) {
LOG.error("User tried to upload more than was allowed ({}).", maxSize); publishUploadFailedAndFinishedEvent(fileUploadId);
interruptUploadDueToFileSizeExceeded(maxSize);
return; return;
} }
if (isUploadInterrupted()) {
if (event.getBytesReceived() > maxSize || event.getContentLength() > maxSize) {
interruptUploadDueToFileSizeQuotaExceeded(SizeConversionHelper.byteValueToReadableString(maxSize));
return; return;
} }
@@ -155,7 +157,8 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl
if (!isUploadInterrupted()) { if (!isUploadInterrupted()) {
interruptUploadDueToUploadFailed(); interruptUploadDueToUploadFailed();
} }
publishUploadFailedAndFinishedEvent(fileUploadId, event.getException()); LOG.debug("Streaming of file {} failed due to following exception: {}", fileUploadId, event.getException());
publishUploadFailedAndFinishedEvent(fileUploadId);
} }
@Override @Override

View File

@@ -16,6 +16,7 @@ import java.util.concurrent.locks.Lock;
import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.ArtifactManagement;
import org.eclipse.hawkbit.repository.RegexCharacterCollection; import org.eclipse.hawkbit.repository.RegexCharacterCollection;
import org.eclipse.hawkbit.repository.SizeConversionHelper;
import org.eclipse.hawkbit.repository.SoftwareModuleManagement; import org.eclipse.hawkbit.repository.SoftwareModuleManagement;
import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource;
@@ -82,16 +83,15 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler
interruptUploadDueToDuplicateFile(); interruptUploadDueToDuplicateFile();
event.getUpload().interruptUpload(); event.getUpload().interruptUpload();
} else { } else {
LOG.info("Uploading file {}", fileUploadId);
publishUploadStarted(fileUploadId); publishUploadStarted(fileUploadId);
if (RegexCharacterCollection.stringContainsCharacter(event.getFilename(), ILLEGAL_FILENAME_CHARACTERS)) { if (RegexCharacterCollection.stringContainsCharacter(event.getFilename(), ILLEGAL_FILENAME_CHARACTERS)) {
LOG.info("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(), LOG.debug("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(),
fileUploadId); fileUploadId);
interruptUploadDueToIllegalFilename(); interruptUploadDueToIllegalFilename();
event.getUpload().interruptUpload(); event.getUpload().interruptUpload();
} else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, softwareModule)) { } else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, softwareModule)) {
LOG.info("File {} already contained in Software Module {}", fileUploadId.getFilename(), softwareModule); LOG.debug("File {} already contained in Software Module {}", fileUploadId.getFilename(), softwareModule);
interruptUploadDueToDuplicateFile(); interruptUploadDueToDuplicateFile();
event.getUpload().interruptUpload(); event.getUpload().interruptUpload();
} }
@@ -130,11 +130,11 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler
publishUploadProgressEvent(fileUploadId, 0, 0); publishUploadProgressEvent(fileUploadId, 0, 0);
startTransferToRepositoryThread(inputStream, fileUploadId, mimeType); startTransferToRepositoryThread(inputStream, fileUploadId, mimeType);
} catch (final IOException e) { } catch (final IOException e) {
LOG.error("Creating piped Stream failed {}.", e); LOG.warn("Creating piped Stream failed {}.", e);
tryToCloseIOStream(outputStream); tryToCloseIOStream(outputStream);
tryToCloseIOStream(inputStream); tryToCloseIOStream(inputStream);
interruptUploadDueToUploadFailed(); interruptUploadDueToUploadFailed();
publishUploadFailedAndFinishedEvent(fileUploadId, e); publishUploadFailedAndFinishedEvent(fileUploadId);
return ByteStreams.nullOutputStream(); return ByteStreams.nullOutputStream();
} }
return outputStream; return outputStream;
@@ -147,14 +147,13 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler
*/ */
@Override @Override
public void updateProgress(final long readBytes, final long contentLength) { public void updateProgress(final long readBytes, final long contentLength) {
if (readBytes > maxSize || contentLength > maxSize) { if (isUploadInterrupted()) {
LOG.error("User tried to upload more than was allowed ({}).", maxSize); publishUploadFailedAndFinishedEvent(fileUploadId);
interruptUploadDueToFileSizeExceeded(maxSize);
return; return;
} }
if (isUploadInterrupted()) {
// Upload interruption is delayed maybe another event is fired if (readBytes > maxSize || contentLength > maxSize) {
// before interruptUploadDueToFileSizeQuotaExceeded(SizeConversionHelper.byteValueToReadableString(maxSize));
return; return;
} }
@@ -200,7 +199,8 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler
if (!isUploadInterrupted()) { if (!isUploadInterrupted()) {
interruptUploadDueToUploadFailed(); interruptUploadDueToUploadFailed();
} }
publishUploadFailedAndFinishedEvent(fileUploadId, event.getReason()); LOG.debug("Upload of file {} failed due to following exception: {}", fileUploadId, event.getReason());
publishUploadFailedAndFinishedEvent(fileUploadId);
} }
@Override @Override

View File

@@ -469,7 +469,6 @@ message.upload.failed = Upload failed
message.upload.assignmentQuota = Max assignments per software module exceeded message.upload.assignmentQuota = Max assignments per software module exceeded
message.upload.fileSizeQuota = Maximum artifact size ({0}) exceeded message.upload.fileSizeQuota = Maximum artifact size ({0}) exceeded
message.upload.storageQuota = Storage quota exceeded, {0} left 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.uploadedfile.illegalFilename = Filename contains illegal characters
message.artifact.deleted = Artifact with file {0} deleted successfully message.artifact.deleted = Artifact with file {0} deleted successfully