Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtractArchiveJob: Fixes job not stopping when cancelled by user, also changes state to use "cancelled" instead of "terminated" #2008

Merged
merged 8 commits into from
Jun 18, 2024
7 changes: 5 additions & 2 deletions src/main/java/sirius/biz/process/Processes.java
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,11 @@ protected boolean markCompleted(String processId,
return modify(processId, process -> process.getState() != ProcessState.TERMINATED, process -> {
if (process.getState() != ProcessState.STANDBY) {
process.setErrorneous(process.isErrorneous() || !TaskContext.get().isActive());
process.setState(ProcessState.TERMINATED);
process.setCompleted(LocalDateTime.now());
//Do not alter the job state if the job was previously cancelled by the user
if (process.getState() != ProcessState.CANCELED) {
Copy link
Contributor

@idlira idlira Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on a second thought, it will be a bit more robust if we specifically check for cancelations.
If the job was cancelled, both date and status will be already set.

if (process.getCancelled() == null) {
  process.setState(ProcessState.TERMINATED);
  process.setCompleted(LocalDateTime.now());
}

OR

if (process.getState() != ProcessState.CANCELED) {
  process.setState(ProcessState.TERMINATED);
  process.setCompleted(LocalDateTime.now());
}

process.setState(ProcessState.TERMINATED);
process.setCompleted(LocalDateTime.now());
}
process.setComputationTime(process.getComputationTime() + computationTimeInSeconds);
process.setExpires(process.getPersistencePeriod().plus(LocalDate.now()));
}
Expand Down
30 changes: 16 additions & 14 deletions src/main/java/sirius/biz/storage/layer3/ExtractArchiveJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public class ExtractArchiveJob extends SimpleBatchProcessJobFactory {

private static final String FILE_SKIPPED_COUNTER = "ExtractArchiveJob.fileSkipped";
private static final String FILE_EMPTY_COUNTER = "ExtractArchiveJob.fileEmpty";
private static final String ARCHIVE_JOB_EMPTY_FILE = "ExtractArchiveJob.emptyFile";
private static final String FILENAME = "filename";

/**
* Creates the job factory so that it can be invoked by the framework.
Expand Down Expand Up @@ -137,15 +139,15 @@ protected void execute(ProcessContext process) throws Exception {
.withContext("size", NLS.formatSize(sourceFile.size())));

try (FileHandle archive = sourceFile.download()) {
extractor.extractAll(sourceFile.name(),
archive.getFile(),
null,
file -> handleExtractedFile(file,
process,
overrideMode,
targetDirectory,
flattenDirs));
process.forceUpdateState(NLS.get("ExtractArchiveJob.completed"));
extractor.extractAll(sourceFile.name(), archive.getFile(), null, file -> {
if (!process.isActive()) {
return;
}
handleExtractedFile(file, process, overrideMode, targetDirectory, flattenDirs);
});
if(processes.fetchProcessForUser(process.getProcessId()).orElse(null).getCanceled() == null){
process.forceUpdateState(NLS.get("ExtractArchiveJob.completed"));
}
} catch (Exception exception) {
process.log(ProcessLog.error().withMessage(exception.getMessage()));
}
Expand All @@ -167,7 +169,7 @@ private void handleExtractedFile(ExtractedFile extractedFile,

if (extractedFile == null) {
// if this happens we don't know the file name of this entry, we just log with an empty name
process.log(ProcessLog.warn().withNLSKey("ExtractArchiveJob.emptyFile").withContext("filename", ""));
process.log(ProcessLog.warn().withNLSKey(ARCHIVE_JOB_EMPTY_FILE).withContext(FILENAME, ""));
process.addTiming(FILE_EMPTY_COUNTER, watch.elapsedMillis());
return;
}
Expand All @@ -178,8 +180,8 @@ private void handleExtractedFile(ExtractedFile extractedFile,

if (extractedFile.size() == 0) {
process.log(ProcessLog.warn()
.withNLSKey("ExtractArchiveJob.emptyFile")
.withContext("filename", extractedFile.getFilePath()));
.withNLSKey(ARCHIVE_JOB_EMPTY_FILE)
.withContext(FILENAME, extractedFile.getFilePath()));
process.addTiming(FILE_EMPTY_COUNTER, watch.elapsedMillis());
return;
}
Expand All @@ -188,8 +190,8 @@ private void handleExtractedFile(ExtractedFile extractedFile,
VirtualFile targetFile = fetchTargetFile(targetDirectory, targetPath);
if (targetFile == null) {
process.log(ProcessLog.warn()
.withNLSKey("ExtractArchiveJob.emptyFile")
.withContext("filename", extractedFile.getFilePath()));
.withNLSKey(ARCHIVE_JOB_EMPTY_FILE)
.withContext(FILENAME, extractedFile.getFilePath()));
process.addTiming(FILE_SKIPPED_COUNTER, watch.elapsedMillis());
return;
}
Expand Down