From 7a5dd2db3a3523b3874cbbae9aac5c92d83ded64 Mon Sep 17 00:00:00 2001 From: Sven F <9976560+sven1103@users.noreply.github.com> Date: Fri, 31 May 2024 14:53:58 +0200 Subject: [PATCH] Enhanced logging and process configuration checks (#20) Working and target dir(s) are now evaluated on application startup and let the application start fail, if any access right precondition fails. Thus, no surprising runtime exceptions are thrown, when the actually processing starts. --- .../processing/AccessRightsEvaluation.java | 71 +++++++++++++++++++ .../life/qbic/data/processing/AppConfig.java | 18 +++-- .../qbic/data/processing/Application.java | 27 ++++--- .../qbic/data/processing/GlobalConfig.java | 3 +- .../life/qbic/data/processing/Provenance.java | 1 - .../config/EvaluationWorkersConfig.java | 1 - .../config/RegistrationWorkersConfig.java | 3 +- .../processing/config/RoundRobinDraw.java | 7 +- .../evaluation/EvaluationConfiguration.java | 18 ++--- .../evaluation/EvaluationRequest.java | 22 +++--- .../processing/ProcessingConfiguration.java | 14 ++-- .../processing/ProcessingRequest.java | 4 +- .../ProcessRegistrationRequest.java | 2 +- .../RegistrationConfiguration.java | 34 ++++----- .../registration/RegistrationRequest.java | 3 +- .../scanner/ScannerConfiguration.java | 17 +++-- 16 files changed, 171 insertions(+), 74 deletions(-) create mode 100644 src/main/java/life/qbic/data/processing/AccessRightsEvaluation.java diff --git a/src/main/java/life/qbic/data/processing/AccessRightsEvaluation.java b/src/main/java/life/qbic/data/processing/AccessRightsEvaluation.java new file mode 100644 index 0000000..a9e2cc1 --- /dev/null +++ b/src/main/java/life/qbic/data/processing/AccessRightsEvaluation.java @@ -0,0 +1,71 @@ +package life.qbic.data.processing; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; + +/** + * Access and File Evaluation helper class + * + * @since 1.0.0 + */ +public class AccessRightsEvaluation { + + /** + * Convenience method that checks if the application has write and executable permission for the + * given file. + * + * @param file the path of the file of interest to evaluate + * @throws IOException if one or both of the evaluated conditions are failing + * @since 1.0.0 + */ + public static void evaluateWriteAndExecutablePermission(Path file) throws IOException { + evaluateWriteAndExecutablePermission(file.toFile()); + } + + /** + * Convenience method that checks if the application has write and executable permission for the + * given file. + * + * @param file the file of interest to evaluate + * @throws IOException if one or both of the evaluated conditions are failing + * @since 1.0.0 + */ + public static void evaluateWriteAndExecutablePermission(File file) throws IOException { + if (!file.canWrite()) { + throw new IOException("Cannot write to file " + file); + } + if (!file.canExecute()) { + throw new IOException("Cannot execute file " + file); + } + } + + /** + * Convenience method that checks if file exists and is a directory. + * + * @param file the path of the file of interest to evaluate + * @throws IOException if neither of the evaluated conditions are failing + * @since 1.0.0 + */ + public static void evaluateExistenceAndDirectory(Path file) throws IOException { + evaluateExistenceAndDirectory(file.toFile()); + } + + /** + * Convenience method that checks if file exists and is a directory. + * + * @param file the path of the file of interest to evaluate + * @throws IOException if neither of the evaluated conditions are failing + * @since 1.0.0 + */ + public static void evaluateExistenceAndDirectory(File file) throws IOException { + if (!file.exists()) { + throw new IOException("File does not exist " + file); + } + if (!file.isDirectory()) { + throw new IOException("File is not a directory " + file); + } + } + + +} diff --git a/src/main/java/life/qbic/data/processing/AppConfig.java b/src/main/java/life/qbic/data/processing/AppConfig.java index d5f357e..8f82250 100644 --- a/src/main/java/life/qbic/data/processing/AppConfig.java +++ b/src/main/java/life/qbic/data/processing/AppConfig.java @@ -1,5 +1,6 @@ package life.qbic.data.processing; +import java.io.IOException; import java.nio.file.Path; import java.util.Arrays; import life.qbic.data.processing.config.EvaluationWorkersConfig; @@ -28,7 +29,8 @@ class AppConfig { @Bean ScannerConfiguration scannerConfiguration( @Value("${scanner.directory}") String scannerDirectory, - @Value("${scanner.interval}") int interval, @Value("${scanner.ignore}") String[] ignore) { + @Value("${scanner.interval}") int interval, @Value("${scanner.ignore}") String[] ignore) + throws IOException { return new ScannerConfiguration(scannerDirectory, interval, ignore); } @@ -44,7 +46,7 @@ RegistrationWorkersConfig registrationWorkersConfig( @Bean RegistrationConfiguration registrationConfiguration( - RegistrationWorkersConfig registrationWorkersConfig) { + RegistrationWorkersConfig registrationWorkersConfig) throws IOException { return new RegistrationConfiguration(registrationWorkersConfig.workingDirectory().toString(), registrationWorkersConfig.targetDirectory().toString(), registrationWorkersConfig.metadataFileName()); @@ -55,12 +57,13 @@ EvaluationWorkersConfig evaluationWorkersConfig( @Value("${evaluation.threads}") int amountOfWorkers, @Value("${evaluation.working.dir}") String workingDirectory, @Value("${evaluation.target.dirs}") String[] targetDirectory) { - return new EvaluationWorkersConfig(amountOfWorkers, workingDirectory, Arrays.stream(targetDirectory).toList()); + return new EvaluationWorkersConfig(amountOfWorkers, workingDirectory, + Arrays.stream(targetDirectory).toList()); } @Bean EvaluationConfiguration evaluationConfiguration(EvaluationWorkersConfig evaluationWorkersConfig, - GlobalConfig globalConfig) { + GlobalConfig globalConfig) throws IOException { return new EvaluationConfiguration(evaluationWorkersConfig.workingDirectory().toString(), evaluationWorkersConfig.targetDirectories(), globalConfig); } @@ -75,7 +78,8 @@ ProcessingWorkersConfig processingWorkersConfig( } @Bean - ProcessingConfiguration processingConfiguration(ProcessingWorkersConfig processingWorkersConfig) { + ProcessingConfiguration processingConfiguration(ProcessingWorkersConfig processingWorkersConfig) + throws IOException { return new ProcessingConfiguration(processingWorkersConfig.workingDirectory(), processingWorkersConfig.targetDirectory()); } @@ -85,7 +89,7 @@ GlobalConfig globalConfig( @Value("${users.error.directory.name}") String usersErrorDirectoryName, @Value("${users.registration.directory.name}") String usersRegistrationDirectoryName, @Value("${qbic.measurement-id.pattern}") String measurementIdPattern) { - return new GlobalConfig(usersErrorDirectoryName, usersRegistrationDirectoryName, measurementIdPattern); + return new GlobalConfig(usersErrorDirectoryName, usersRegistrationDirectoryName, + measurementIdPattern); } - } diff --git a/src/main/java/life/qbic/data/processing/Application.java b/src/main/java/life/qbic/data/processing/Application.java index 0f7dd51..2d9a0a6 100644 --- a/src/main/java/life/qbic/data/processing/Application.java +++ b/src/main/java/life/qbic/data/processing/Application.java @@ -31,12 +31,18 @@ public static void main(String[] args) { AppConfig.class); ScannerConfiguration scannerConfiguration = context.getBean(ScannerConfiguration.class); - RegistrationWorkersConfig registrationWorkersConfig = context.getBean(RegistrationWorkersConfig.class); - RegistrationConfiguration registrationConfiguration = context.getBean(RegistrationConfiguration.class); - ProcessingWorkersConfig processingWorkersConfig = context.getBean(ProcessingWorkersConfig.class); - ProcessingConfiguration processingConfiguration = context.getBean(ProcessingConfiguration.class); - EvaluationWorkersConfig evaluationWorkersConfig = context.getBean(EvaluationWorkersConfig.class); - EvaluationConfiguration evaluationConfiguration = context.getBean(EvaluationConfiguration.class); + RegistrationWorkersConfig registrationWorkersConfig = context.getBean( + RegistrationWorkersConfig.class); + RegistrationConfiguration registrationConfiguration = context.getBean( + RegistrationConfiguration.class); + ProcessingWorkersConfig processingWorkersConfig = context.getBean( + ProcessingWorkersConfig.class); + ProcessingConfiguration processingConfiguration = context.getBean( + ProcessingConfiguration.class); + EvaluationWorkersConfig evaluationWorkersConfig = context.getBean( + EvaluationWorkersConfig.class); + EvaluationConfiguration evaluationConfiguration = context.getBean( + EvaluationConfiguration.class); GlobalConfig globalConfig = context.getBean(GlobalConfig.class); var requestQueue = new ConcurrentRegistrationQueue(); @@ -45,21 +51,22 @@ public static void main(String[] args) { log.info("Registering {} registration workers...", registrationWorkersConfig.amountOfWorkers()); List registrationWorkers = new LinkedList<>(); - for (int i=0; i processingWorkers = new LinkedList<>(); - for (int i=0; i evaluationWorkers = new LinkedList<>(); - for (int i=0; i items) { } /** - * Creates an instance of {@link RoundRobinDraw} based on the type {@link T} of the collection provided + * Creates an instance of {@link RoundRobinDraw} based on the type {@link T} of the collection + * provided + * * @param items a collection of items the round robin method shall be applied. * @return an instance of this class - * @throws IllegalArgumentException if an empty collection is provided or the collection is null + * @throws IllegalArgumentException if an empty collection is provided or the collection is + * null * @since 1.0.0 */ public static RoundRobinDraw create(Collection items) throws IllegalArgumentException { diff --git a/src/main/java/life/qbic/data/processing/evaluation/EvaluationConfiguration.java b/src/main/java/life/qbic/data/processing/evaluation/EvaluationConfiguration.java index 6ccfdbb..5424f21 100644 --- a/src/main/java/life/qbic/data/processing/evaluation/EvaluationConfiguration.java +++ b/src/main/java/life/qbic/data/processing/evaluation/EvaluationConfiguration.java @@ -1,9 +1,10 @@ package life.qbic.data.processing.evaluation; +import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collection; -import java.util.regex.Pattern; +import life.qbic.data.processing.AccessRightsEvaluation; import life.qbic.data.processing.GlobalConfig; import life.qbic.data.processing.config.RoundRobinDraw; @@ -22,16 +23,15 @@ public class EvaluationConfiguration { private final RoundRobinDraw targetDirectoriesRoundRobinDraw; public EvaluationConfiguration(String workingDirectory, Collection targetDirectories, - GlobalConfig globalConfig) { + GlobalConfig globalConfig) throws IOException { this.workingDirectory = Paths.get(workingDirectory); - if (!this.workingDirectory.toFile().exists()) { - throw new IllegalArgumentException("Evaluation worker directory does not exist"); - } + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.workingDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.workingDirectory); this.targetDirectories = targetDirectories.stream().toList(); - this.targetDirectories.stream().filter(path -> !path.toFile().exists()).forEach(path -> { - throw new IllegalArgumentException( - "Evaluation target directory '%s' does not exist".formatted(path)); - }); + for (Path targetDirectory : this.targetDirectories) { + AccessRightsEvaluation.evaluateExistenceAndDirectory(targetDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(targetDirectory); + } this.targetDirectoriesRoundRobinDraw = RoundRobinDraw.create(targetDirectories); this.usersErrorDirectory = globalConfig.usersErrorDirectory(); } diff --git a/src/main/java/life/qbic/data/processing/evaluation/EvaluationRequest.java b/src/main/java/life/qbic/data/processing/evaluation/EvaluationRequest.java index f75254d..6601027 100644 --- a/src/main/java/life/qbic/data/processing/evaluation/EvaluationRequest.java +++ b/src/main/java/life/qbic/data/processing/evaluation/EvaluationRequest.java @@ -15,10 +15,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantLock; -import java.util.regex.MatchResult; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; import life.qbic.data.processing.ErrorSummary; import life.qbic.data.processing.Provenance; import life.qbic.data.processing.Provenance.ProvenanceException; @@ -55,7 +51,7 @@ public class EvaluationRequest extends Thread { private Path assignedTargetDirectory; public EvaluationRequest(Path workingDirectory, RoundRobinDraw targetDirectories, - Path usersErrorDirectory) { + Path usersErrorDirectory) { this.setName(THREAD_NAME.formatted(nextThreadNumber())); this.workingDirectory = workingDirectory; this.targetDirectories = targetDirectories; @@ -144,7 +140,9 @@ private void evaluateDirectory(File taskDir) { return; } - var measurementIdResult = provenance.qbicMeasurementID == null || provenance.qbicMeasurementID.isBlank() ? Optional.empty() : Optional.of(provenance.qbicMeasurementID); + var measurementIdResult = + provenance.qbicMeasurementID == null || provenance.qbicMeasurementID.isBlank() + ? Optional.empty() : Optional.of(provenance.qbicMeasurementID); if (measurementIdResult.isPresent()) { provenance.addToHistory(taskDir.getAbsolutePath()); try { @@ -157,7 +155,7 @@ private void evaluateDirectory(File taskDir) { try { createMarkerFile(assignedTargetDirectory, taskDir.getName()); } catch (IOException e) { - LOG.error("Could not create marker file: {}", taskDir.getAbsolutePath(), e); + LOG.error("Could not create marker file in: {}", assignedTargetDirectory, e); moveToSystemIntervention(taskDir, e.getMessage()); } return; @@ -205,20 +203,22 @@ private void moveBackToOrigin(File taskDir, Provenance provenance, String reason Paths.get(provenance.userWorkDirectoryPath).resolve(usersErrorDirectory) .resolve(taskDir.getName())); } catch (IOException e) { - LOG.error("Cannot move task to user intervention: %s".formatted(Paths.get(provenance.userWorkDirectoryPath).resolve(usersErrorDirectory)), e); + LOG.error("Cannot move task to user intervention: %s".formatted( + Paths.get(provenance.userWorkDirectoryPath).resolve(usersErrorDirectory)), e); moveToSystemIntervention(taskDir, e.getMessage()); } } private void moveToTargetDir(File taskDir) { LOG.info( - "Moving %s to target directory %s".formatted(taskDir.getAbsolutePath(), assignedTargetDirectory)); + "Moving %s to target directory %s".formatted(taskDir.getAbsolutePath(), + assignedTargetDirectory)); try { Files.move(taskDir.toPath(), assignedTargetDirectory.resolve(taskDir.getName())); } catch (IOException e) { - LOG.error("Cannot move task to target directory: %s".formatted(targetDirectories), e); + LOG.error("Cannot move task to target directory: %s".formatted(assignedTargetDirectory), e); moveToSystemIntervention(taskDir, - "Cannot move task to target directory: %s".formatted(targetDirectories)); + "Cannot move task to target directory: %s".formatted(assignedTargetDirectory)); } } diff --git a/src/main/java/life/qbic/data/processing/processing/ProcessingConfiguration.java b/src/main/java/life/qbic/data/processing/processing/ProcessingConfiguration.java index 6084645..5f28e03 100644 --- a/src/main/java/life/qbic/data/processing/processing/ProcessingConfiguration.java +++ b/src/main/java/life/qbic/data/processing/processing/ProcessingConfiguration.java @@ -1,6 +1,8 @@ package life.qbic.data.processing.processing; +import java.io.IOException; import java.nio.file.Path; +import life.qbic.data.processing.AccessRightsEvaluation; /** * Processing Configuration @@ -17,15 +19,13 @@ public class ProcessingConfiguration { private final Path targetDirectory; - public ProcessingConfiguration(Path workingDirectory, Path targetDirectory) { + public ProcessingConfiguration(Path workingDirectory, Path targetDirectory) throws IOException { this.workingDirectory = workingDirectory; - if (!workingDirectory.toFile().exists()) { - throw new IllegalArgumentException("Working directory does not exist: " + workingDirectory); - } + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.workingDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.workingDirectory); this.targetDirectory = targetDirectory; - if (!targetDirectory.toFile().exists()) { - throw new IllegalArgumentException("Target directory does not exist: " + targetDirectory); - } + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.targetDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.workingDirectory); } public Path getWorkingDirectory() { diff --git a/src/main/java/life/qbic/data/processing/processing/ProcessingRequest.java b/src/main/java/life/qbic/data/processing/processing/ProcessingRequest.java index fcfff17..16d5e02 100644 --- a/src/main/java/life/qbic/data/processing/processing/ProcessingRequest.java +++ b/src/main/java/life/qbic/data/processing/processing/ProcessingRequest.java @@ -107,7 +107,9 @@ public void run() { * @since */ private void processFile(File taskDir) { - var taskDirContent = Arrays.stream(Objects.requireNonNull(taskDir.listFiles(), "Task dir must not be null: " + taskDir)).toList(); + var taskDirContent = Arrays.stream( + Objects.requireNonNull(taskDir.listFiles(), "Task dir must not be null: " + taskDir)) + .toList(); if (checkForEmpty(taskDir, taskDirContent)) { return; diff --git a/src/main/java/life/qbic/data/processing/registration/ProcessRegistrationRequest.java b/src/main/java/life/qbic/data/processing/registration/ProcessRegistrationRequest.java index b214e19..2609f57 100644 --- a/src/main/java/life/qbic/data/processing/registration/ProcessRegistrationRequest.java +++ b/src/main/java/life/qbic/data/processing/registration/ProcessRegistrationRequest.java @@ -55,7 +55,7 @@ public class ProcessRegistrationRequest extends Thread { private final String metadataFileName; private final Path userErrorDirectory; private final Pattern measurementIdPattern; - private AtomicBoolean active = new AtomicBoolean(false); + private final AtomicBoolean active = new AtomicBoolean(false); public ProcessRegistrationRequest(@NonNull ConcurrentRegistrationQueue registrationQueue, @NonNull RegistrationConfiguration configuration, @NonNull GlobalConfig globalConfig) { diff --git a/src/main/java/life/qbic/data/processing/registration/RegistrationConfiguration.java b/src/main/java/life/qbic/data/processing/registration/RegistrationConfiguration.java index 67c8693..f0a5577 100644 --- a/src/main/java/life/qbic/data/processing/registration/RegistrationConfiguration.java +++ b/src/main/java/life/qbic/data/processing/registration/RegistrationConfiguration.java @@ -1,8 +1,10 @@ package life.qbic.data.processing.registration; +import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Objects; +import life.qbic.data.processing.AccessRightsEvaluation; public class RegistrationConfiguration { @@ -11,24 +13,22 @@ public class RegistrationConfiguration { private final Path targetDirectory; private final String metadataFileName; - public RegistrationConfiguration(String workingDirectory, String targetDirectory, String metadataFileName) { - this.workingDirectory = Paths.get(Objects.requireNonNull(workingDirectory, "workingDirectory must not be null")); - if (!workingDirectory().toFile().exists()) { - throw new IllegalArgumentException(targetDirectory + " does not exist"); - } - if (!workingDirectory().toFile().isDirectory()) { - throw new IllegalArgumentException(targetDirectory + " is not a directory"); - } - this.targetDirectory = Paths.get(Objects.requireNonNull(targetDirectory, "targetDirectories must not be null")); - if (!targetDirectory().toFile().exists()) { - throw new IllegalArgumentException(targetDirectory + " does not exist"); - } - if (!targetDirectory().toFile().isDirectory()) { - throw new IllegalArgumentException(targetDirectory + " is not a directory"); - } + public RegistrationConfiguration(String workingDirectory, String targetDirectory, + String metadataFileName) + throws IOException { + this.workingDirectory = Paths.get( + Objects.requireNonNull(workingDirectory, "workingDirectory must not be null")); + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.workingDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.workingDirectory); + this.targetDirectory = Paths.get( + Objects.requireNonNull(targetDirectory, "targetDirectories must not be null")); + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.targetDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.targetDirectory); + if (metadataFileName == null || metadataFileName.isEmpty()) { throw new IllegalArgumentException("metadataFileName must not be null or empty"); } + this.metadataFileName = metadataFileName; } @@ -40,5 +40,7 @@ public Path targetDirectory() { return targetDirectory; } - public String metadataFileName() { return metadataFileName; } + public String metadataFileName() { + return metadataFileName; + } } diff --git a/src/main/java/life/qbic/data/processing/registration/RegistrationRequest.java b/src/main/java/life/qbic/data/processing/registration/RegistrationRequest.java index 3c586d0..7a8ccf3 100644 --- a/src/main/java/life/qbic/data/processing/registration/RegistrationRequest.java +++ b/src/main/java/life/qbic/data/processing/registration/RegistrationRequest.java @@ -4,7 +4,8 @@ import java.time.Instant; import java.util.Objects; -public record RegistrationRequest(Instant timestamp, long lastModified, Path origin, Path target, Path userPath) { +public record RegistrationRequest(Instant timestamp, long lastModified, Path origin, Path target, + Path userPath) { @Override public boolean equals(Object o) { diff --git a/src/main/java/life/qbic/data/processing/scanner/ScannerConfiguration.java b/src/main/java/life/qbic/data/processing/scanner/ScannerConfiguration.java index ac558b1..47c55bb 100644 --- a/src/main/java/life/qbic/data/processing/scanner/ScannerConfiguration.java +++ b/src/main/java/life/qbic/data/processing/scanner/ScannerConfiguration.java @@ -1,33 +1,40 @@ package life.qbic.data.processing.scanner; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; import java.util.Collection; import java.util.Objects; +import life.qbic.data.processing.AccessRightsEvaluation; public class ScannerConfiguration { - private final String scannerDirectory; + private final Path scannerDirectory; private final int scanInterval; private final String[] ignore; - public ScannerConfiguration(String scannerDirectory, int interval, String[] ignore) { - this.scannerDirectory = scannerDirectory; + public ScannerConfiguration(String scannerDirectory, int interval, String[] ignore) + throws IOException { + this.scannerDirectory = Paths.get(scannerDirectory); if (interval <= 0) { throw new IllegalArgumentException("Interval must be greater than 0"); } + AccessRightsEvaluation.evaluateExistenceAndDirectory(this.scannerDirectory); + AccessRightsEvaluation.evaluateWriteAndExecutablePermission(this.scannerDirectory); this.scanInterval = interval; this.ignore = Arrays.copyOf(Objects.requireNonNull(ignore), ignore.length); } public String scannerDirectory() { - return scannerDirectory; + return scannerDirectory.toString(); } public int scanInterval() { return scanInterval; } - public Collection ignore () { + public Collection ignore() { return Arrays.stream(ignore).toList(); } }