From ecc1fef27349f69f919136a7cf5e2c95273f5129 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Thu, 8 Feb 2024 16:53:17 +0100 Subject: [PATCH 01/25] DD-1492 dd-manage-deposits improvemnets fixes for DD-1419 - Part 2 directory search depth limit --- .../managedeposit/core/service/IngestPathMonitor.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java index 179b49e..ea2aa5e 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java @@ -47,13 +47,13 @@ public IngestPathMonitor(List depositBoxesPaths, DepositStatusUpdater depo } private void startMonitors() throws Exception { - IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(), HiddenFileFilter.VISIBLE); - IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); - IOFileFilter filter = FileFilterUtils.or(directories, files); - - log.info("Starting 'IngestPathMonitor', file filter: deposit.properties"); + log.info("Starting 'IngestPathMonitor', file filter: deposit.properties, directory depth: only first child of the base folder"); for (Path folder : toMonitorPaths) { + IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(), new DepthFileFilter(folder, 1)); + IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); + IOFileFilter filter = FileFilterUtils.or(directories, files); + FileAlterationObserver observer = new FileAlterationObserver(folder.toFile(), filter); observer.addListener(this); From 90eb06ad430b697b3e356b554a6dcc09484d9c51 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Thu, 8 Feb 2024 17:00:36 +0100 Subject: [PATCH 02/25] DD-1492 dd-manage-deposits improvemnets fixes for DD-1419 - Part 2 directory search depth limit --- .../core/service/DepthFileFilter.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java new file mode 100644 index 0000000..2d3a1b9 --- /dev/null +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit.core.service; + +import org.apache.commons.io.filefilter.AbstractFileFilter; + +import java.io.File; +import java.nio.file.Path; + +public class DepthFileFilter extends AbstractFileFilter { + private final Path baseFolder; + private final int depthLimit; + + public DepthFileFilter(Path baseFolder, int depthLimit) { + this.baseFolder = baseFolder; + this.depthLimit = depthLimit; + + } + + public boolean accept(File file) { + Path parent = file.toPath(); + for (int depth = 0; depth < this.depthLimit; depth++) + parent = parent.getParent(); + if (parent.endsWith(this.baseFolder)) + return true; + + return false; + } +} From 684bcd5043beaf0ff23b41ed6734f5ca91aeab67 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Sat, 9 Mar 2024 16:55:56 +0100 Subject: [PATCH 03/25] DD-1492 dd-manage-deposits improvemnet and fixes for DD-1419 --- pom.xml | 3 +- src/main/assembly/dist/bin/dd-manage-deposit | 2 +- src/main/assembly/dist/cfg/config.yml | 2 +- .../DdManageDepositApplication.java | 27 ++++---- .../DdManageDepositConfiguration.java | 5 +- .../core/CsvMessageBodyWriter.java | 8 +-- .../managedeposit/core/DepositProperties.java | 25 ++++---- .../service/DepositPropertiesAssembler.java | 41 ++++++------ .../service/DepositPropertiesFileReader.java | 6 +- .../core/service/DepositStatusUpdater.java | 62 ++++++++++--------- .../core/service/DepthFileFilter.java | 21 ++++--- .../core/service/IngestPathMonitor.java | 30 +++++---- .../core/service/TextTruncation.java | 5 +- .../db/DepositPropertiesDAO.java | 26 +------- .../health/InboxHealthCheck.java | 9 +-- .../DepositPropertiesDeleteResource.java | 6 +- .../DepositPropertiesReportResource.java | 8 +-- .../resources/DepositPropertiesResource.java | 40 ++++++------ 18 files changed, 148 insertions(+), 178 deletions(-) diff --git a/pom.xml b/pom.xml index 54031e0..85b98db 100644 --- a/pom.xml +++ b/pom.xml @@ -150,7 +150,8 @@ false - /usr/local/bin/rpm + /opt/local/bin/rpm + diff --git a/src/main/assembly/dist/bin/dd-manage-deposit b/src/main/assembly/dist/bin/dd-manage-deposit index fe00670..0625e07 100644 --- a/src/main/assembly/dist/bin/dd-manage-deposit +++ b/src/main/assembly/dist/bin/dd-manage-deposit @@ -5,4 +5,4 @@ BINPATH=$(command readlink -f $0 2> /dev/null || command grealpath $0 2> /dev/nu APPHOME=$(dirname $(dirname $BINPATH)) CONFIG_PATH=/etc/opt/dans.knaw.nl/$SCRIPTNAME/config.yml -java -Ddans.default.config=$CONFIG_PATH -jar $APPHOME/bin/$SCRIPTNAME.jar "$@" +java $DANS_JAVA_OPTS -Ddans.default.config=$CONFIG_PATH -jar $APPHOME/bin/$SCRIPTNAME.jar "$@" diff --git a/src/main/assembly/dist/cfg/config.yml b/src/main/assembly/dist/cfg/config.yml index e941205..cec3712 100644 --- a/src/main/assembly/dist/cfg/config.yml +++ b/src/main/assembly/dist/cfg/config.yml @@ -18,7 +18,7 @@ depositBoxes: - /var/opt/dans.knaw.nl/tmp/auto-ingest/outbox/failed - /var/opt/dans.knaw.nl/tmp/sword2-uploads -pollingInterval: 5000 +pollingInterval: 500 depositPropertiesDatabase: driverClass: org.postgresql.Driver diff --git a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java index e5428f2..c6de424 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java +++ b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java @@ -17,11 +17,11 @@ package nl.knaw.dans.managedeposit; import io.dropwizard.core.Application; +import io.dropwizard.core.setup.Bootstrap; +import io.dropwizard.core.setup.Environment; import io.dropwizard.db.DataSourceFactory; import io.dropwizard.hibernate.HibernateBundle; import io.dropwizard.hibernate.UnitOfWorkAwareProxyFactory; -import io.dropwizard.core.setup.Bootstrap; -import io.dropwizard.core.setup.Environment; import nl.knaw.dans.managedeposit.core.CsvMessageBodyWriter; import nl.knaw.dans.managedeposit.core.DepositProperties; import nl.knaw.dans.managedeposit.core.service.DepositStatusUpdater; @@ -34,19 +34,19 @@ public class DdManageDepositApplication extends Application { + private final HibernateBundle depositPropertiesHibernate = + new HibernateBundle<>(DepositProperties.class) { + + @Override + public DataSourceFactory getDataSourceFactory(DdManageDepositConfiguration configuration) { + return configuration.getDepositPropertiesDatabase(); + } + }; + public static void main(final String[] args) throws Exception { new DdManageDepositApplication().run(args); } - private final HibernateBundle depositPropertiesHibernate = - new HibernateBundle<>(DepositProperties.class) { - - @Override - public DataSourceFactory getDataSourceFactory(DdManageDepositConfiguration configuration) { - return configuration.getDepositPropertiesDatabase(); - } - }; - @Override public String getName() { return "Dd Manage Deposit"; @@ -70,13 +70,10 @@ public void run(final DdManageDepositConfiguration configuration, final Environm final UnitOfWorkAwareProxyFactory proxyFactory = new UnitOfWorkAwareProxyFactory(depositPropertiesHibernate); DepositStatusUpdater depositStatusUpdater = proxyFactory.create( - DepositStatusUpdater.class, - new Class[] { DepositPropertiesDAO.class }, - new Object[] { depositPropertiesDAO }); + DepositStatusUpdater.class, DepositPropertiesDAO.class, depositPropertiesDAO); final IngestPathMonitor ingestPathMonitor = new IngestPathMonitor(configuration.getDepositBoxes(), depositStatusUpdater, configuration.getPollingInterval()); environment.lifecycle().manage(ingestPathMonitor); - } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java index 70fc225..e492e40 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java +++ b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java @@ -18,7 +18,6 @@ import io.dropwizard.core.Configuration; import io.dropwizard.db.DataSourceFactory; -import nl.knaw.dans.managedeposit.core.service.TextTruncation; import javax.validation.Valid; import javax.validation.constraints.NotNull; @@ -26,7 +25,9 @@ import java.util.ArrayList; import java.util.List; +@SuppressWarnings("unused") public class DdManageDepositConfiguration extends Configuration { + private static final long DEFAULT_POLLING_INTERVAL = 500; @Valid @NotNull private DataSourceFactory database = new DataSourceFactory(); @@ -52,7 +53,7 @@ public void setDepositPropertiesDatabase(DataSourceFactory database) { } public long getPollingInterval() { - return pollingInterval > 0 ? pollingInterval : TextTruncation.pollingInterval; + return pollingInterval > 0 ? pollingInterval : DEFAULT_POLLING_INTERVAL; } public void setPollingInterval(long pollingInterval) { diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java index 3236949..26ce908 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java @@ -44,14 +44,14 @@ public boolean isWriteable(Class type, Type genericType, Annotation[] annotation @Override public void writeTo(List data, Class aClass, Type type, Annotation[] annotations, MediaType mediaType, MultivaluedMap multivaluedMap, OutputStream outputStream) throws - IOException, WebApplicationException { - if (data != null && data.size() > 0) { + IOException, WebApplicationException { + if (data != null && !data.isEmpty()) { // TODO: pass the mapper in at configuration time CsvMapper mapper = new CsvMapper(); Object o = data.get(0); CsvSchema schema = mapper.schemaFor(o.getClass()) - .withHeader() - .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") + .withHeader() + .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") .rebuild().build(); mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index 177416a..f7bb28d 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -17,19 +17,17 @@ import nl.knaw.dans.managedeposit.core.service.TextTruncation; -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.NamedQuery; -import javax.persistence.Table; +import javax.persistence.*; import java.time.OffsetDateTime; @Entity @Table(name = "deposit_properties") @NamedQuery( - name = "showAll", - query = "SELECT dp FROM DepositProperties dp" + name = "showAll", + query = "SELECT dp FROM DepositProperties dp" ) + +@SuppressWarnings("unused") public class DepositProperties { @Column(name = "depositor", nullable = false) // depositor.userId @@ -49,10 +47,10 @@ public class DepositProperties { @Column(name = "deposit_update_timestamp") // modified timestamp of deposit.properties private OffsetDateTime depositUpdateTimestamp; - @Column(name = "description", length = TextTruncation.maxDescriptionLength) // state.description + @Column(name = "description", length = TextTruncation.MAX_DESCRIPTION_LENGTH) // state.description private String description; - @Column(name = "location", length = TextTruncation.maxDirectoryLength) // full parent-path on disk + @Column(name = "location", length = TextTruncation.MAX_DIRECTORY_LENGTH) // full parent-path on disk private String location; @Column(name = "storage_in_bytes") // Total storage of deposit directory @@ -60,15 +58,12 @@ public class DepositProperties { @Column(name = "deleted") // deposit is deleted from inbox - archived private boolean deleted; - public String getDepositId() { - return depositId; - } public DepositProperties() { } public DepositProperties(String depositId, String depositor, String bagName, String depositState, - String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes) { + String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes) { this.depositId = depositId; this.depositor = depositor; this.bagName = bagName; @@ -79,6 +74,10 @@ public DepositProperties(String depositId, String depositor, String bagName, Str this.storageInBytes = storageInBytes; } + public String getDepositId() { + return depositId; + } + public String getDepositor() { return depositor; } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java index 1a18fb7..aac3ae1 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java @@ -34,7 +34,7 @@ class DepositPropertiesAssembler { DepositPropertiesAssembler() { } - Optional assembleObject(File depositPropertiesFile, boolean updateModificationDateTime) { + Optional assembleObject(File depositPropertiesFile, boolean updateModificationDateTime, long CalculatedFolderSize) { Path depositPath = depositPropertiesFile.getParentFile().toPath(); log.debug("assembleObject(depositPropertiesPath:Path): '{}'", depositPropertiesFile.getAbsolutePath()); @@ -44,23 +44,21 @@ Optional assembleObject(File depositPropertiesFile, boolean configuration = DepositPropertiesFileReader.readDepositProperties(depositPropertiesFile); dp = new DepositProperties(depositPath.getFileName().toString(), - configuration.getString("depositor.userId", ""), - configuration.getString("bag-store.bag-name", ""), - configuration.getString("state.label", ""), - TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.maxDescriptionLength), - OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), - TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.maxDirectoryLength), - calculateFolderSize(depositPath)); + configuration.getString("depositor.userId", ""), + configuration.getString("bag-store.bag-name", ""), + configuration.getString("state.label", ""), + TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), + OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), + TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), + CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); if (updateModificationDateTime) { dp.setDepositUpdateTimestamp(OffsetDateTime.now()); - } - else { + } else { dp.setDepositUpdateTimestamp(dp.getDepositCreationTimestamp()); } - } - catch (ConfigurationException e) { + } catch (ConfigurationException e) { log.error(e.getMessage(), e); throw new RuntimeException(e); } @@ -68,15 +66,16 @@ Optional assembleObject(File depositPropertiesFile, boolean } private long calculateFolderSize(Path path) { - long size; - try (var pathStream = Files.walk(path)) { - size = pathStream - .filter(p -> p.toFile().isFile()) - .mapToLong(p -> p.toFile().length()) - .sum(); - } - catch (IOException e) { - throw new RuntimeException(e); + long size = 0; + if (Files.exists(path)) { + try (var pathStream = Files.walk(path)) { + size = pathStream + .filter(p -> p.toFile().isFile()) + .mapToLong(p -> p.toFile().length()) + .sum(); + } catch (IOException e) { + throw new RuntimeException(e); + } } return size; diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java index cfe8402..9ed9a3c 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java @@ -26,7 +26,7 @@ import java.io.File; class DepositPropertiesFileReader { - private static final Logger log = LoggerFactory.getLogger(DepositPropertiesAssembler.class); + private static final Logger log = LoggerFactory.getLogger(DepositPropertiesFileReader.class); static public Configuration readDepositProperties(File propertiesFile) throws ConfigurationException { log.debug("readDepositProperties: '{}'", propertiesFile.toString()); @@ -34,8 +34,8 @@ static public Configuration readDepositProperties(File propertiesFile) throws Co var paramConfig = params.properties().setFileName(propertiesFile.getAbsolutePath()); FileBasedConfigurationBuilder builder = new FileBasedConfigurationBuilder<> - (PropertiesConfiguration.class, null, true) - .configure(paramConfig); + (PropertiesConfiguration.class, null, true) + .configure(paramConfig); return builder.getConfiguration(); } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java index 936fa79..3096540 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java @@ -28,7 +28,7 @@ import java.util.Optional; public class DepositStatusUpdater { - private static final Logger log = LoggerFactory.getLogger(DepositPropertiesAssembler.class); + private static final Logger log = LoggerFactory.getLogger(DepositStatusUpdater.class); private final DepositPropertiesDAO depositPropertiesDAO; private final DepositPropertiesAssembler depositPropertiesAssembler; @@ -38,47 +38,49 @@ public DepositStatusUpdater(DepositPropertiesDAO depositPropertiesDAO) { } @UnitOfWork - public void onCreateDeposit(File depositPropertiesFile) { - Optional dp = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + public void onDepositCreate(File depositPropertiesFile) { + // Note: the 'move deposit' action is processed by the system in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); - if (dp.isPresent()) { - // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. - // Update the location column. - Path depositLocationFolder = Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath()); - Optional updatedNumber = depositPropertiesDAO.updateDepositLocation(dp.get().getDepositId(), depositLocationFolder); - if (updatedNumber.isPresent()) - log.debug("onCreateDeposit - `location` of deposit '{}' has been updated to '{}' ", dp.get().getDepositId(), depositLocationFolder); - } - else { - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false); + if (record.isPresent()) { + // Update the location column and other fields + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, record.get().getStorageInBytes()); + dpObject.ifPresent(depositPropertiesDAO::merge); + log.debug("onDepositCreate - The deposit '{}' location and/or state has been updated to '{}' ", record.get().getDepositId(), depositPropertiesFile.getParentFile().getAbsolutePath()); + } else { + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false, 0); dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + log.debug("onDepositCreate: A new deposit has been registered '{}'", depositPropertiesFile.getParentFile().getAbsolutePath());//log.info("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); } } @UnitOfWork - public void onChangeDeposit(File depositPropertiesFile) { - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true); - dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onChangeDeposit: deposit.properties has been changed `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + public void onDepositChange(File depositPropertiesFile) { + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + long folder_size = record.isPresent() ? record.get().getStorageInBytes() : 0; + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, folder_size); + dpObject.ifPresent(depositPropertiesDAO::merge); + log.debug("onDepositChange: deposit.properties has been changed '{}'", depositPropertiesFile.getParentFile().getAbsolutePath()); } @UnitOfWork - public void onDeleteDeposit(File depositPropertiesFile) { - // At this stage, the deposit.properties file's handle is present but the content is null (impossible to read data of the file) - Optional dp = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + public void onDepositDelete(File depositPropertiesFile) { + // Note: if delete notify is part of a folder moving, then at this stage, the deposit.properties file's handle is present but the content is null (impossible to read data of the file) + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); - try { - // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step - if (dp.isPresent() && Files.isSameFile(Path.of(dp.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath()))) { - String depositId = depositPropertiesFile.getParentFile().getName(); - Optional deletedNumber = depositPropertiesDAO.updateDeleteFlag(depositId, true); - log.debug("onDeleteDeposit - 'deleted' mark has been set to '{}' for deposit.properties from '{}' ", deletedNumber.isPresent(), depositId); + // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step + if (record.isPresent()) { + log.debug("OnDepositDelete: \n record.getLocation(): {} \n deposit-path-argument: {}\n depositPropertiesFile.exists() : {}\n", record.get().getLocation(), depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), depositPropertiesFile.exists()); + try { + if (Files.isSameFile(Path.of(record.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath())) && !depositPropertiesFile.exists()) { + String depositId = depositPropertiesFile.getParentFile().getName(); + Optional deletedNumber = depositPropertiesDAO.updateDeleteFlag(depositId, true); + log.debug("onDepositDelete - 'deleted' mark has been set to '{}' for deposit.properties from '{}' ", deletedNumber.isPresent() && deletedNumber.get() > 0, depositId); + } + } catch (IOException e) { + throw new RuntimeException(e); } } - catch (IOException e) { - log.debug("The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step for {}", depositPropertiesFile.getParentFile().getAbsolutePath()); - } } } \ No newline at end of file diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java index 2d3a1b9..f67d5fa 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java @@ -24,19 +24,26 @@ public class DepthFileFilter extends AbstractFileFilter { private final Path baseFolder; private final int depthLimit; - public DepthFileFilter(Path baseFolder, int depthLimit) { - this.baseFolder = baseFolder; + public DepthFileFilter(Path baserFolder, int depthLimit) { + this.baseFolder = baserFolder; this.depthLimit = depthLimit; - } - public boolean accept(File file) { + private boolean confirmParent(File file) { Path parent = file.toPath(); for (int depth = 0; depth < this.depthLimit; depth++) parent = parent.getParent(); - if (parent.endsWith(this.baseFolder)) - return true; - return false; + return parent.endsWith(this.baseFolder); + } + + @Override + public boolean accept(File file) { + return confirmParent(file); + } + + @Override + public boolean accept(File dir, String name) { + return confirmParent(dir); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java index ea2aa5e..94324ae 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java @@ -18,7 +18,6 @@ import io.dropwizard.lifecycle.Managed; import org.apache.commons.io.IOCase; import org.apache.commons.io.filefilter.FileFilterUtils; -import org.apache.commons.io.filefilter.HiddenFileFilter; import org.apache.commons.io.filefilter.IOFileFilter; import org.apache.commons.io.monitor.FileAlterationListenerAdaptor; import org.apache.commons.io.monitor.FileAlterationMonitor; @@ -47,31 +46,31 @@ public IngestPathMonitor(List depositBoxesPaths, DepositStatusUpdater depo } private void startMonitors() throws Exception { - log.info("Starting 'IngestPathMonitor', file filter: deposit.properties, directory depth: only first child of the base folder"); + log.info("Starting 'IngestPathMonitor', with file filter: deposit.properties"); + List observers = new ArrayList<>(); for (Path folder : toMonitorPaths) { - IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(), new DepthFileFilter(folder, 1)); + IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(),/*HiddenFileFilter.VISIBLE*/new DepthFileFilter(folder, 1)); IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); IOFileFilter filter = FileFilterUtils.or(directories, files); FileAlterationObserver observer = new FileAlterationObserver(folder.toFile(), filter); - observer.addListener(this); + observers.add(observer); - FileAlterationMonitor monitor = new FileAlterationMonitor(this.pollingInterval, observer); - fileAlterationMonitors.add(monitor); - - monitor.start(); - log.debug("'IngestPathMonitor' is going to monitor the folder '{}'", folder); } + + FileAlterationMonitor monitor = new FileAlterationMonitor(this.pollingInterval, observers.toArray(new FileAlterationObserver[0])); + fileAlterationMonitors.add(monitor); + log.debug("'IngestPathMonitor' is going to monitor the folders\n '{}'", toMonitorPaths); + monitor.start(); } @Override public void start() throws Exception { try { startMonitors(); - } - catch (IOException | InterruptedException e) { + } catch (IOException | InterruptedException e) { log.error(e.getMessage(), e); throw new InvalidTransferItemException(e.getMessage()); } @@ -84,8 +83,7 @@ public void stop() throws Exception { fileAlterationMonitors.forEach(monitor -> { try { monitor.stop(); - } - catch (Exception e) { + } catch (Exception e) { throw new RuntimeException(e); } }); @@ -94,19 +92,19 @@ public void stop() throws Exception { @Override public void onFileCreate(File file) { log.debug("onFileCreate: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onCreateDeposit(file); + depositStatusUpdater.onDepositCreate(file); } @Override public void onFileDelete(File file) { log.debug("onFileDelete: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onDeleteDeposit(file); + depositStatusUpdater.onDepositDelete(file); } @Override public void onFileChange(File file) { log.debug("onFileChange: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onChangeDeposit(file); + depositStatusUpdater.onDepositChange(file); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java index 4192693..74e79e8 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java @@ -16,9 +16,8 @@ package nl.knaw.dans.managedeposit.core.service; public class TextTruncation { - public static final long pollingInterval = 5 * 1000; - public static final int maxDescriptionLength = 1024; - public static final int maxDirectoryLength = 512; + public static final int MAX_DESCRIPTION_LENGTH = 1024; + public static final int MAX_DIRECTORY_LENGTH = 512; public static String stripEnd(String text, int maxLength) { return text.length() > maxLength ? text.substring(0, maxLength) : text; diff --git a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java index 1db4751..5647061 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -20,13 +20,7 @@ import org.hibernate.SessionFactory; import org.hibernate.query.Query; -import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.CriteriaDelete; -import javax.persistence.criteria.CriteriaQuery; -import javax.persistence.criteria.CriteriaUpdate; -import javax.persistence.criteria.Predicate; -import javax.persistence.criteria.Root; -import java.nio.file.Path; +import javax.persistence.criteria.*; import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.ZoneOffset; @@ -70,7 +64,7 @@ public List findAll() { public List findSelection(Map> queryParameters) { CriteriaBuilder criteriaBuilder = currentSession().getCriteriaBuilder(); - if (queryParameters.size() == 0) + if (queryParameters.isEmpty()) return findAll(); CriteriaQuery criteriaQuery = criteriaBuilder.createQuery(DepositProperties.class); @@ -83,7 +77,7 @@ public List findSelection(Map> queryPara public Optional deleteSelection(Map> queryParameters) { var criteriaBuilder = currentSession().getCriteriaBuilder(); - if (queryParameters.size() == 0) // Note: all records will be deleted (accidentally) without any specified query parameter + if (queryParameters.isEmpty()) // Note: all records will be deleted (accidentally) without any specified query parameter return Optional.of(0); CriteriaDelete deleteQuery = criteriaBuilder.createCriteriaDelete(DepositProperties.class); @@ -169,18 +163,4 @@ public Optional updateDeleteFlag(String depositId, boolean deleted) { return Optional.of(query.executeUpdate()); } - public Optional updateDepositLocation(String depositId, Path currentParentPath) { - CriteriaBuilder criteriaBuilder = currentSession().getCriteriaBuilder(); - CriteriaUpdate criteriaUpdate = criteriaBuilder.createCriteriaUpdate(DepositProperties.class); - Root root = criteriaUpdate.from(DepositProperties.class); - - Predicate predicate = buildQueryCriteria(Map.of("depositId", List.of(depositId)), criteriaBuilder, root); - criteriaUpdate.where(predicate); - - criteriaUpdate.set("location", currentParentPath.toString()); - - var query = currentSession().createQuery(criteriaUpdate); - return Optional.of(query.executeUpdate()); - } - } diff --git a/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java b/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java index a64b39b..1519215 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java +++ b/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java @@ -42,14 +42,12 @@ protected Result check() throws Exception { if (exists && canRead) { log.debug("Inbox path '{}' exists and is readable", folder); - } - else { + } else { valid = false; if (!exists) { log.debug("Inbox path '{}' does not exist", folder); - } - else { + } else { log.debug("Inbox path '{}' is not readable", folder); } } @@ -57,8 +55,7 @@ protected Result check() throws Exception { if (valid) { return Result.healthy(); - } - else { + } else { return Result.unhealthy("InboxPaths are not accessible"); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java index 1e008bc..e78832a 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java @@ -18,11 +18,7 @@ import io.dropwizard.hibernate.UnitOfWork; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; -import javax.ws.rs.Consumes; -import javax.ws.rs.NotFoundException; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.Produces; +import javax.ws.rs.*; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.UriInfo; diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java index 46035d7..5c725e3 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java @@ -19,11 +19,7 @@ import nl.knaw.dans.managedeposit.core.DepositProperties; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; -import javax.ws.rs.GET; -import javax.ws.rs.NotFoundException; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; +import javax.ws.rs.*; import javax.ws.rs.core.Context; import javax.ws.rs.core.UriInfo; import java.util.List; @@ -39,7 +35,7 @@ public DepositPropertiesReportResource(DepositPropertiesDAO depositPropertiesDAO @GET @UnitOfWork - @Produces({ "application/json", "text/csv" }) + @Produces({"application/json", "text/csv"}) public List listDepositProperties(@Context UriInfo uriInfo) { return depositPropertiesDAO.findSelection(uriInfo.getQueryParameters()); } diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java index aef3290..f6809a5 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -20,11 +20,7 @@ import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; import javax.validation.Valid; -import javax.ws.rs.Consumes; -import javax.ws.rs.GET; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.Produces; +import javax.ws.rs.*; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -40,18 +36,20 @@ public DepositPropertiesResource(DepositPropertiesDAO depositPropertiesDAO) { private String writeHelpInfoText() { return - "DD Manage Deposit is running. \n" + - "Usage: \n" + - " - Create reports: GET basePath/report \n" + - " - Clean database: POST basePath/delete-deposit \n" + - " Query string parameters: user, state, startdate, enddate \n" + - " 'startdate'/'enddate' format: yyyy-MM-dd \n" + - " Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED \n" + - " Examples: \n" + - " curl -i -X GET basePath/report?startdate=yyyy-MM-dd \n" + - " curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED \n" + - " curl -i -X POST basePath/delete-deposit?user=XXX \n" + - " curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"; + """ + This is DD-1438-Fix-Problems-found-DD1419 + DD Manage Deposit is running.\s + Usage:\s + - Create reports: GET basePath/report\s + - Clean database: POST basePath/delete-deposit\s + Query string parameters: user, state, startdate, enddate\s + 'startdate'/'enddate' format: yyyy-MM-dd\s + Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s + Examples:\s + curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s + curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s + curl -i -X POST basePath/delete-deposit?user=XXX\s + curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; } @GET @@ -60,10 +58,10 @@ public Response getApiInformation() { return Response - .status(Response.Status.OK) - .entity(this.helpInfo) - .type(MediaType.TEXT_PLAIN) - .build(); + .status(Response.Status.OK) + .entity(this.helpInfo) + .type(MediaType.TEXT_PLAIN) + .build(); } @POST From 765eb2f2a0ea6d166237e536aeb1f5b79ee155e1 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Sat, 9 Mar 2024 21:20:28 +0100 Subject: [PATCH 04/25] DD-1492 dd-manage-deposits improvemnet and fixes for DD-1419 --- .../managedeposit/resources/DepositPropertiesResource.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java index f6809a5..ff1c50c 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -37,7 +37,6 @@ public DepositPropertiesResource(DepositPropertiesDAO depositPropertiesDAO) { private String writeHelpInfoText() { return """ - This is DD-1438-Fix-Problems-found-DD1419 DD Manage Deposit is running.\s Usage:\s - Create reports: GET basePath/report\s @@ -47,7 +46,7 @@ private String writeHelpInfoText() { Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s Examples:\s curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s - curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s + curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s curl -i -X POST basePath/delete-deposit?user=XXX\s curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; } From 5c928ea0c24597367ed8276425462af6c8ba0104 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Mon, 11 Mar 2024 17:35:00 +0100 Subject: [PATCH 05/25] Update RPM link was not correct --- pom.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 85ab7b5..50017c3 100644 --- a/pom.xml +++ b/pom.xml @@ -150,8 +150,7 @@ false - /opt/local/bin/rpm - + /usr/local/bin/rpm From b7c910df4bb2296ede9f1c75287ef7b6d6d24797 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:15:14 +0200 Subject: [PATCH 06/25] Update DepositProperties.java (expand `import *`) --- .../knaw/dans/managedeposit/core/DepositProperties.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index f7bb28d..7078c87 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -17,7 +17,11 @@ import nl.knaw.dans.managedeposit.core.service.TextTruncation; -import javax.persistence.*; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.NamedQuery; +import javax.persistence.Table; import java.time.OffsetDateTime; @Entity @@ -168,4 +172,4 @@ public boolean equals(Object o) { public int hashCode() { return 31 * depositId.hashCode() + depositor.hashCode(); } -} \ No newline at end of file +} From e5c3763f27d94edb07fbaa4f5743db76c5225cb4 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Wed, 10 Apr 2024 11:27:54 +0200 Subject: [PATCH 07/25] changed `List` to `var` IngestPathMonitor.java --- .../knaw/dans/managedeposit/core/service/IngestPathMonitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java index 94324ae..a144982 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java @@ -48,7 +48,7 @@ public IngestPathMonitor(List depositBoxesPaths, DepositStatusUpdater depo private void startMonitors() throws Exception { log.info("Starting 'IngestPathMonitor', with file filter: deposit.properties"); - List observers = new ArrayList<>(); + var observers = new ArrayList<>(); for (Path folder : toMonitorPaths) { IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(),/*HiddenFileFilter.VISIBLE*/new DepthFileFilter(folder, 1)); IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); From cc846d6428dcf1bad05c5776236d31988966e078 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Fri, 12 Apr 2024 11:13:48 +0200 Subject: [PATCH 08/25] testing IngestPathMonitor --- pom.xml | 17 ++++ .../AbstractTestWithTestDir.java | 16 +++ .../core/service/IngestPathMonitorTest.java | 99 ++++++++++++++++++- 3 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java diff --git a/pom.xml b/pom.xml index bb37321..f76bb6b 100644 --- a/pom.xml +++ b/pom.xml @@ -87,6 +87,23 @@ nl.knaw.dans dans-java-utils + + junit + junit + RELEASE + test + + + org.mockito + mockito-core + 5.8.0 + test + + + org.junit.jupiter + junit-jupiter + test + diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java new file mode 100644 index 0000000..9f86d02 --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java @@ -0,0 +1,16 @@ +package nl.knaw.dans.managedeposit; + +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.BeforeEach; + +import java.nio.file.Path; + +public class AbstractTestWithTestDir { + protected final Path testDir = Path.of("target/test") + .resolve(getClass().getSimpleName()); + + @BeforeEach + public void setUp() throws Exception { + FileUtils.deleteDirectory(testDir.toFile()); + } +} diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index 27a320a..390c8c9 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -15,6 +15,99 @@ */ package nl.knaw.dans.managedeposit.core.service; -public class IngestPathMonitorTest { - // Stub -} +import nl.knaw.dans.managedeposit.AbstractTestWithTestDir; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.nio.file.Files; + +import static java.nio.file.Files.createDirectories; +import static java.util.Collections.singletonList; + +public class IngestPathMonitorTest extends AbstractTestWithTestDir { + + private IngestPathMonitor startMonitor(DepositStatusUpdater mockUpdater, int pollingInterval) throws Exception { + createDirectories(testDir); + + var monitor = new IngestPathMonitor(singletonList(testDir), mockUpdater, pollingInterval); + monitor.start(); + Thread.sleep(60); // wait for the monitor to get ready + return monitor; + } + + @Test + public void should_ignore_properties_in_root() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var tempFile = Files.createFile(testDir.resolve("deposit.properties")); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(tempFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_pick_up_properties_in_bag() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(1)).onCreateDeposit(propertiesFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_ignore_properties_in_bag_content() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var propertiesFile = testDir.resolve("bag/content/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(propertiesFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_pick_up_a_bag() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var bagDir = testDir.resolve("bag"); + createDirectories(bagDir); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(1)).onCreateDeposit(bagDir.toFile()); + + monitor.stop(); + } + + @Test + public void should_ignore_bag_content() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var dirInBag = testDir.resolve("bag/content"); + createDirectories(dirInBag); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(dirInBag.toFile()); + + monitor.stop(); + } + + // TODO: Add more tests for onModifyDeposit and onDeleteDeposit and more complex scenarios, such as: + // events while the monitor was down + // not existing dir when starting the monitor + // disappearing dir while monitoring, etc. +} \ No newline at end of file From 1f19305b1737b586f98ea1a328b2c3899c53ba0f Mon Sep 17 00:00:00 2001 From: jo-pol Date: Mon, 15 Apr 2024 10:37:41 +0200 Subject: [PATCH 09/25] follow rename in PR, license header --- .../managedeposit/AbstractTestWithTestDir.java | 15 +++++++++++++++ .../core/service/IngestPathMonitorTest.java | 10 +++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java index 9f86d02..822e91a 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java @@ -1,3 +1,18 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package nl.knaw.dans.managedeposit; import org.apache.commons.io.FileUtils; diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index 390c8c9..9799626 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -43,7 +43,7 @@ public void should_ignore_properties_in_root() throws Exception { var tempFile = Files.createFile(testDir.resolve("deposit.properties")); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(tempFile.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(tempFile.toFile()); monitor.stop(); } @@ -58,7 +58,7 @@ public void should_pick_up_properties_in_bag() throws Exception { Files.createFile(propertiesFile); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(1)).onCreateDeposit(propertiesFile.toFile()); + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(propertiesFile.toFile()); monitor.stop(); } @@ -73,7 +73,7 @@ public void should_ignore_properties_in_bag_content() throws Exception { Files.createFile(propertiesFile); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(propertiesFile.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(propertiesFile.toFile()); monitor.stop(); } @@ -87,7 +87,7 @@ public void should_pick_up_a_bag() throws Exception { createDirectories(bagDir); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(1)).onCreateDeposit(bagDir.toFile()); + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(bagDir.toFile()); monitor.stop(); } @@ -101,7 +101,7 @@ public void should_ignore_bag_content() throws Exception { createDirectories(dirInBag); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(0)).onCreateDeposit(dirInBag.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(dirInBag.toFile()); monitor.stop(); } From 1f2332032351658c36bfccebfe59097d89f578ea Mon Sep 17 00:00:00 2001 From: jo-pol Date: Fri, 12 Apr 2024 15:54:50 +0200 Subject: [PATCH 10/25] test DepositStausUpdater --- pom.xml | 22 ++++++ .../managedeposit/AbstractDatabaseTest.java | 37 +++++++++ ...positStatusUpdaterOnDepositUpdateTest.java | 78 +++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java create mode 100644 src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java diff --git a/pom.xml b/pom.xml index f76bb6b..c81ba1f 100644 --- a/pom.xml +++ b/pom.xml @@ -104,6 +104,28 @@ junit-jupiter test + + io.dropwizard + dropwizard-testing + 3.0.5 + test + + + com.h2database + h2 + test + + + org.assertj + assertj-core + test + + + org.mockito + mockito-junit-jupiter + 5.4.0 + test + diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java new file mode 100644 index 0000000..c9e88bb --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit; + +import io.dropwizard.testing.junit5.DAOTestExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import nl.knaw.dans.managedeposit.core.DepositProperties; +import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(DropwizardExtensionsSupport.class) +public abstract class AbstractDatabaseTest extends AbstractTestWithTestDir { + protected final DAOTestExtension daoTestExtension = DAOTestExtension.newBuilder() + .addEntityClass(DepositProperties.class) + .build(); + protected DepositPropertiesDAO dao; + + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + dao = new DepositPropertiesDAO(daoTestExtension.getSessionFactory()); + } +} diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java new file mode 100644 index 0000000..24379cf --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit.core.service; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import nl.knaw.dans.managedeposit.AbstractDatabaseTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; + +import static java.nio.file.Files.createDirectories; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +public class DepositStatusUpdaterOnDepositUpdateTest extends AbstractDatabaseTest { + private ListAppender listAppender; + + @BeforeEach + public void setup() throws Exception { + super.setUp(); + var logger = (Logger) LoggerFactory.getLogger(DepositStatusUpdater.class); + listAppender = new ListAppender<>(); + listAppender.start(); + logger.setLevel(Level.DEBUG); + logger.addAppender(listAppender); + } + + @Test + public void onCreateDeposit_should_add_a_db_record() throws IOException { + var depositStatusUpdater = new DepositStatusUpdater(dao); + + // Prepare test data + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.writeString(propertiesFile, """ + bag-store.bag-id = a78c2cad-d473-401d-8751-a447ef573b7a + dataverse.bag-id = urn:uuid:a78c2cad-d473-401d-8751-a447ef573b7a + creation.timestamp = 2023-08-16T17:40:41.390209+02:00 + deposit.origin = SWORD2 + depositor.userId = user001 + bag-store.bag-name = revision03 + dataverse.sword-token = sword:a78c2cad-d473-401d-8751-a447ef573b7a + """); + + // Call the method under test + depositStatusUpdater.onDepositCreate(propertiesFile.toFile()); + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("onDepositCreate: A new deposit has been registered '"); + assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); + + // Check the database + assertThat(dao.findById("bag")).isNotEmpty().get() + .isEqualTo("revision03"); + assertThat(dao.findAll()).isNotEmpty(); + } + + // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit +} \ No newline at end of file From 3ed4ec8920e1158f730ae6eacee72eb98319a6e3 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Mon, 15 Apr 2024 14:42:35 +0200 Subject: [PATCH 11/25] fix test with false assumptions --- .../core/service/IngestPathMonitorTest.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index 9799626..0868dcf 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -19,10 +19,12 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import java.io.File; import java.nio.file.Files; import static java.nio.file.Files.createDirectories; import static java.util.Collections.singletonList; +import static org.mockito.ArgumentMatchers.any; public class IngestPathMonitorTest extends AbstractTestWithTestDir { @@ -43,7 +45,8 @@ public void should_ignore_properties_in_root() throws Exception { var tempFile = Files.createFile(testDir.resolve("deposit.properties")); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(tempFile.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)) + .onDepositCreate(tempFile.toFile()); monitor.stop(); } @@ -79,7 +82,7 @@ public void should_ignore_properties_in_bag_content() throws Exception { } @Test - public void should_pick_up_a_bag() throws Exception { + public void should_ignore_a_bag_without_deposit_properties() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class); var monitor = startMonitor(mockUpdater, 50); @@ -87,7 +90,8 @@ public void should_pick_up_a_bag() throws Exception { createDirectories(bagDir); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(bagDir.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)) + .onDepositCreate(any(File.class)); monitor.stop(); } @@ -101,7 +105,8 @@ public void should_ignore_bag_content() throws Exception { createDirectories(dirInBag); Thread.sleep(70);// Wait for the monitor to pick up the new file - Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(dirInBag.toFile()); + Mockito.verify(mockUpdater, Mockito.times(0)) + .onDepositCreate(dirInBag.toFile()); monitor.stop(); } From 3ba9b4c0ad4e4717307500db837a8282e6dcf706 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Mon, 15 Apr 2024 15:56:09 +0200 Subject: [PATCH 12/25] fix depthfilter --- .../core/service/IngestPathMonitor.java | 17 +++++++++++++---- ...DepositStatusUpdaterOnDepositUpdateTest.java | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java index a144982..b0cfbd2 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java @@ -50,8 +50,15 @@ private void startMonitors() throws Exception { var observers = new ArrayList<>(); for (Path folder : toMonitorPaths) { - IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(),/*HiddenFileFilter.VISIBLE*/new DepthFileFilter(folder, 1)); - IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); + IOFileFilter directories = FileFilterUtils.and( + FileFilterUtils.directoryFileFilter(), + new DepthFileFilter(folder, 1) + ); + IOFileFilter files = FileFilterUtils.and( + FileFilterUtils.fileFileFilter(), + FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE), + new DepthFileFilter(folder, 2) + ); IOFileFilter filter = FileFilterUtils.or(directories, files); FileAlterationObserver observer = new FileAlterationObserver(folder.toFile(), filter); @@ -70,7 +77,8 @@ private void startMonitors() throws Exception { public void start() throws Exception { try { startMonitors(); - } catch (IOException | InterruptedException e) { + } + catch (IOException | InterruptedException e) { log.error(e.getMessage(), e); throw new InvalidTransferItemException(e.getMessage()); } @@ -83,7 +91,8 @@ public void stop() throws Exception { fileAlterationMonitors.forEach(monitor -> { try { monitor.stop(); - } catch (Exception e) { + } + catch (Exception e) { throw new RuntimeException(e); } }); diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index 24379cf..a5ad7db 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -70,6 +70,7 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { // Check the database assertThat(dao.findById("bag")).isNotEmpty().get() + .extracting("bagName") .isEqualTo("revision03"); assertThat(dao.findAll()).isNotEmpty(); } From b2818e38ea698e76cb9091501f7ab5246945575a Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 16 Apr 2024 11:47:14 +0200 Subject: [PATCH 13/25] maximized coverage of IngestPathMonitor --- .../AbstractTestWithTestDir.java | 11 +++ .../core/service/IngestPathMonitorTest.java | 76 ++++++++++++++----- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java index 822e91a..950d084 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java @@ -20,6 +20,8 @@ import java.nio.file.Path; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + public class AbstractTestWithTestDir { protected final Path testDir = Path.of("target/test") .resolve(getClass().getSimpleName()); @@ -28,4 +30,13 @@ public class AbstractTestWithTestDir { public void setUp() throws Exception { FileUtils.deleteDirectory(testDir.toFile()); } + + /** + * Assume that a bug is not yet fixed. This allows to skip assertions while still showing the code covered by the test. + * + * @param message the message to display + */ + public void assumeNotYetFixed (String message) { + assumeTrue(false, message); + } } diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index 0868dcf..d923ce4 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -16,21 +16,19 @@ package nl.knaw.dans.managedeposit.core.service; import nl.knaw.dans.managedeposit.AbstractTestWithTestDir; +import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.File; import java.nio.file.Files; import static java.nio.file.Files.createDirectories; import static java.util.Collections.singletonList; -import static org.mockito.ArgumentMatchers.any; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class IngestPathMonitorTest extends AbstractTestWithTestDir { private IngestPathMonitor startMonitor(DepositStatusUpdater mockUpdater, int pollingInterval) throws Exception { - createDirectories(testDir); - var monitor = new IngestPathMonitor(singletonList(testDir), mockUpdater, pollingInterval); monitor.start(); Thread.sleep(60); // wait for the monitor to get ready @@ -42,17 +40,19 @@ public void should_ignore_properties_in_root() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class); var monitor = startMonitor(mockUpdater, 50); - var tempFile = Files.createFile(testDir.resolve("deposit.properties")); + createDirectories(testDir); + var propertiesFile = Files.createFile(testDir.resolve("deposit.properties")); Thread.sleep(70);// Wait for the monitor to pick up the new file Mockito.verify(mockUpdater, Mockito.times(0)) - .onDepositCreate(tempFile.toFile()); + .onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); monitor.stop(); } @Test - public void should_pick_up_properties_in_bag() throws Exception { + public void should_pick_up_new_properties_in_bag() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class); var monitor = startMonitor(mockUpdater, 50); @@ -62,6 +62,7 @@ public void should_pick_up_properties_in_bag() throws Exception { Thread.sleep(70);// Wait for the monitor to pick up the new file Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); monitor.stop(); } @@ -77,38 +78,73 @@ public void should_ignore_properties_in_bag_content() throws Exception { Thread.sleep(70);// Wait for the monitor to pick up the new file Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); monitor.stop(); } + @Test - public void should_ignore_a_bag_without_deposit_properties() throws Exception { + public void should_pick_up_deleted_bag() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class); var monitor = startMonitor(mockUpdater, 50); - var bagDir = testDir.resolve("bag"); - createDirectories(bagDir); - Thread.sleep(70);// Wait for the monitor to pick up the new file + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70); + FileUtils.deleteDirectory(propertiesFile.getParent().toFile()); + Thread.sleep(70); - Mockito.verify(mockUpdater, Mockito.times(0)) - .onDepositCreate(any(File.class)); + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositDelete(propertiesFile.toFile()); monitor.stop(); } @Test - public void should_ignore_bag_content() throws Exception { + public void should_pick_up_changed_properties() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class); - var monitor = startMonitor(mockUpdater, 50); + var monitor = startMonitor(mockUpdater, 20); - var dirInBag = testDir.resolve("bag/content"); - createDirectories(dirInBag); - Thread.sleep(70);// Wait for the monitor to pick up the new file + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(30); + Files.writeString(propertiesFile, "just some garbage"); + Thread.sleep(30); - Mockito.verify(mockUpdater, Mockito.times(0)) - .onDepositCreate(dirInBag.toFile()); + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositChange(propertiesFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_pick_up_deleted_root() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 20); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(30); + FileUtils.deleteDirectory(testDir.toFile()); + Thread.sleep(30); + + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); + assumeNotYetFixed("The monitor should pick up the deletion of the root folder, it might imply deletion of many bags"); + monitor.stop(); + } + @Test + public void should_throw_when_stopping_a_stopped_monitor() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 20); monitor.stop(); + + assertThatThrownBy(monitor::stop) + .isInstanceOf(RuntimeException.class) + .hasRootCauseInstanceOf(IllegalStateException.class); } // TODO: Add more tests for onModifyDeposit and onDeleteDeposit and more complex scenarios, such as: From c6bf1771d578c25682aae087cc1254a7f7d82782 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 16 Apr 2024 12:59:21 +0200 Subject: [PATCH 14/25] detailed expectations of onDepositUpdate --- ...epositStatusUpdaterOnDepositUpdateTest.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index a5ad7db..9348af9 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.nio.file.Files; +import java.time.OffsetDateTime; import static java.nio.file.Files.createDirectories; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; @@ -51,13 +52,9 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { var propertiesFile = testDir.resolve("bag/deposit.properties"); createDirectories(propertiesFile.getParent()); Files.writeString(propertiesFile, """ - bag-store.bag-id = a78c2cad-d473-401d-8751-a447ef573b7a - dataverse.bag-id = urn:uuid:a78c2cad-d473-401d-8751-a447ef573b7a creation.timestamp = 2023-08-16T17:40:41.390209+02:00 - deposit.origin = SWORD2 depositor.userId = user001 bag-store.bag-name = revision03 - dataverse.sword-token = sword:a78c2cad-d473-401d-8751-a447ef573b7a """); // Call the method under test @@ -70,9 +67,16 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { // Check the database assertThat(dao.findById("bag")).isNotEmpty().get() - .extracting("bagName") - .isEqualTo("revision03"); - assertThat(dao.findAll()).isNotEmpty(); + .hasFieldOrPropertyWithValue("depositId", "bag") + .hasFieldOrPropertyWithValue("depositor", "user001") + .hasFieldOrPropertyWithValue("bagName", "revision03") + .hasFieldOrPropertyWithValue("depositState", "") + .hasFieldOrPropertyWithValue("depositCreationTimestamp", OffsetDateTime.parse("2023-08-16T17:40:41.390209+02:00")) + .hasFieldOrPropertyWithValue("location", testDir.toAbsolutePath().toString()) + .hasFieldOrPropertyWithValue("storageInBytes", 113L); + + assertThat(dao.findAll()).isEmpty(); + assumeNotYetFixed("findAll should also return the record"); } // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit From 7ec43882c4f2b75f2b04a460cbe849380752474c Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 16 Apr 2024 13:00:21 +0200 Subject: [PATCH 15/25] abused assumeNotYetImplemented --- .../core/service/DepositStatusUpdaterOnDepositUpdateTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index 9348af9..d9d7ea9 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -75,8 +75,7 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { .hasFieldOrPropertyWithValue("location", testDir.toAbsolutePath().toString()) .hasFieldOrPropertyWithValue("storageInBytes", 113L); - assertThat(dao.findAll()).isEmpty(); - assumeNotYetFixed("findAll should also return the record"); + assertThat(dao.findAll()).isNotEmpty(); } // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit From 713cb47b8eaca5666d455646b54b54fbee9a672a Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 16 Apr 2024 13:10:59 +0200 Subject: [PATCH 16/25] obsolete todo --- .../managedeposit/core/service/IngestPathMonitorTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index d923ce4..b7d2a39 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -146,9 +146,4 @@ public void should_throw_when_stopping_a_stopped_monitor() throws Exception { .isInstanceOf(RuntimeException.class) .hasRootCauseInstanceOf(IllegalStateException.class); } - - // TODO: Add more tests for onModifyDeposit and onDeleteDeposit and more complex scenarios, such as: - // events while the monitor was down - // not existing dir when starting the monitor - // disappearing dir while monitoring, etc. } \ No newline at end of file From 26c174e2770ce60e670a1198a65aef0a433f2529 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Thu, 18 Apr 2024 11:34:39 +0200 Subject: [PATCH 17/25] Apply suggestions from code review use dao.findAll() in a transaction process Co-authored-by: Jo Pol --- .../DepositStatusUpdaterOnDepositUpdateTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index d9d7ea9..c809f2a 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -66,7 +66,10 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); // Check the database - assertThat(dao.findById("bag")).isNotEmpty().get() + var maybyDepositProperties = daoTestExtension.inTransaction(() -> + dao.findById("bag") + ); + assertThat(maybeDepositProperties).isNotEmpty().get() .hasFieldOrPropertyWithValue("depositId", "bag") .hasFieldOrPropertyWithValue("depositor", "user001") .hasFieldOrPropertyWithValue("bagName", "revision03") @@ -75,7 +78,10 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { .hasFieldOrPropertyWithValue("location", testDir.toAbsolutePath().toString()) .hasFieldOrPropertyWithValue("storageInBytes", 113L); - assertThat(dao.findAll()).isNotEmpty(); + var depositPropertiesList = daoTestExtension.inTransaction(() -> + dao.findAll() + ); + assertThat(depositPropertiesList).hasSize(1); } // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit From 777712bf019975bc515e936735aec3c996929c93 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Thu, 18 Apr 2024 11:53:28 +0200 Subject: [PATCH 18/25] Apply suggestions from code review Typo correction Co-authored-by: Jo Pol --- .../core/service/DepositStatusUpdaterOnDepositUpdateTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index c809f2a..4cc3322 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -66,7 +66,7 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); // Check the database - var maybyDepositProperties = daoTestExtension.inTransaction(() -> + var maybeDepositProperties = daoTestExtension.inTransaction(() -> dao.findById("bag") ); assertThat(maybeDepositProperties).isNotEmpty().get() From 79d369a360e3ebbbc50c43f63195b5ceb005b7b8 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Thu, 18 Apr 2024 17:44:58 +0200 Subject: [PATCH 19/25] DD-1438 resolve reviews --- .../DdManageDepositApplication.java | 14 +++---- .../core/CsvMessageBodyWriter.java | 8 ++-- .../managedeposit/core/DepositProperties.java | 6 +-- .../service/DepositPropertiesAssembler.java | 29 +++++++------ .../service/DepositPropertiesFileReader.java | 4 +- .../core/service/DepositStatusUpdater.java | 12 ++++-- .../core/service/DepthFileFilter.java | 23 ++++++----- .../core/service/TextTruncation.java | 1 - .../db/DepositPropertiesDAO.java | 7 +++- .../health/InboxHealthCheck.java | 9 ++-- .../DepositPropertiesDeleteResource.java | 6 ++- .../DepositPropertiesReportResource.java | 8 +++- .../resources/DepositPropertiesResource.java | 41 ++++++++++--------- 13 files changed, 98 insertions(+), 70 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java index c6de424..9326387 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java +++ b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java @@ -35,13 +35,13 @@ public class DdManageDepositApplication extends Application { private final HibernateBundle depositPropertiesHibernate = - new HibernateBundle<>(DepositProperties.class) { + new HibernateBundle<>(DepositProperties.class) { - @Override - public DataSourceFactory getDataSourceFactory(DdManageDepositConfiguration configuration) { - return configuration.getDepositPropertiesDatabase(); - } - }; + @Override + public DataSourceFactory getDataSourceFactory(DdManageDepositConfiguration configuration) { + return configuration.getDepositPropertiesDatabase(); + } + }; public static void main(final String[] args) throws Exception { new DdManageDepositApplication().run(args); @@ -70,7 +70,7 @@ public void run(final DdManageDepositConfiguration configuration, final Environm final UnitOfWorkAwareProxyFactory proxyFactory = new UnitOfWorkAwareProxyFactory(depositPropertiesHibernate); DepositStatusUpdater depositStatusUpdater = proxyFactory.create( - DepositStatusUpdater.class, DepositPropertiesDAO.class, depositPropertiesDAO); + DepositStatusUpdater.class, DepositPropertiesDAO.class, depositPropertiesDAO); final IngestPathMonitor ingestPathMonitor = new IngestPathMonitor(configuration.getDepositBoxes(), depositStatusUpdater, configuration.getPollingInterval()); environment.lifecycle().manage(ingestPathMonitor); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java index 26ce908..4bf5d27 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java @@ -44,15 +44,15 @@ public boolean isWriteable(Class type, Type genericType, Annotation[] annotation @Override public void writeTo(List data, Class aClass, Type type, Annotation[] annotations, MediaType mediaType, MultivaluedMap multivaluedMap, OutputStream outputStream) throws - IOException, WebApplicationException { + IOException, WebApplicationException { if (data != null && !data.isEmpty()) { // TODO: pass the mapper in at configuration time CsvMapper mapper = new CsvMapper(); Object o = data.get(0); CsvSchema schema = mapper.schemaFor(o.getClass()) - .withHeader() - .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") - .rebuild().build(); + .withHeader() + .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") + .rebuild().build(); mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); mapper.registerModule(new JavaTimeModule()); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index 7078c87..03bb270 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -27,8 +27,8 @@ @Entity @Table(name = "deposit_properties") @NamedQuery( - name = "showAll", - query = "SELECT dp FROM DepositProperties dp" + name = "showAll", + query = "SELECT dp FROM DepositProperties dp" ) @SuppressWarnings("unused") @@ -67,7 +67,7 @@ public DepositProperties() { } public DepositProperties(String depositId, String depositor, String bagName, String depositState, - String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes) { + String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes) { this.depositId = depositId; this.depositor = depositor; this.bagName = bagName; diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java index aac3ae1..c59c27d 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java @@ -44,21 +44,23 @@ Optional assembleObject(File depositPropertiesFile, boolean u configuration = DepositPropertiesFileReader.readDepositProperties(depositPropertiesFile); dp = new DepositProperties(depositPath.getFileName().toString(), - configuration.getString("depositor.userId", ""), - configuration.getString("bag-store.bag-name", ""), - configuration.getString("state.label", ""), - TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), - OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), - TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), - CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); + configuration.getString("depositor.userId", ""), + configuration.getString("bag-store.bag-name", ""), + configuration.getString("state.label", ""), + TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), + OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), + TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), + CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); if (updateModificationDateTime) { dp.setDepositUpdateTimestamp(OffsetDateTime.now()); - } else { + } + else { dp.setDepositUpdateTimestamp(dp.getDepositCreationTimestamp()); } - } catch (ConfigurationException e) { + } + catch (ConfigurationException e) { log.error(e.getMessage(), e); throw new RuntimeException(e); } @@ -70,10 +72,11 @@ private long calculateFolderSize(Path path) { if (Files.exists(path)) { try (var pathStream = Files.walk(path)) { size = pathStream - .filter(p -> p.toFile().isFile()) - .mapToLong(p -> p.toFile().length()) - .sum(); - } catch (IOException e) { + .filter(p -> p.toFile().isFile()) + .mapToLong(p -> p.toFile().length()) + .sum(); + } + catch (IOException e) { throw new RuntimeException(e); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java index 9ed9a3c..14a89ee 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java @@ -34,8 +34,8 @@ static public Configuration readDepositProperties(File propertiesFile) throws Co var paramConfig = params.properties().setFileName(propertiesFile.getAbsolutePath()); FileBasedConfigurationBuilder builder = new FileBasedConfigurationBuilder<> - (PropertiesConfiguration.class, null, true) - .configure(paramConfig); + (PropertiesConfiguration.class, null, true) + .configure(paramConfig); return builder.getConfiguration(); } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java index 3096540..e66adac 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java @@ -47,10 +47,12 @@ public void onDepositCreate(File depositPropertiesFile) { Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, record.get().getStorageInBytes()); dpObject.ifPresent(depositPropertiesDAO::merge); log.debug("onDepositCreate - The deposit '{}' location and/or state has been updated to '{}' ", record.get().getDepositId(), depositPropertiesFile.getParentFile().getAbsolutePath()); - } else { + } + else { Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false, 0); dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onDepositCreate: A new deposit has been registered '{}'", depositPropertiesFile.getParentFile().getAbsolutePath());//log.info("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + log.debug("onDepositCreate: A new deposit has been registered '{}'", + depositPropertiesFile.getParentFile().getAbsolutePath());//log.info("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); } } @@ -70,14 +72,16 @@ public void onDepositDelete(File depositPropertiesFile) { // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step if (record.isPresent()) { - log.debug("OnDepositDelete: \n record.getLocation(): {} \n deposit-path-argument: {}\n depositPropertiesFile.exists() : {}\n", record.get().getLocation(), depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), depositPropertiesFile.exists()); + log.debug("OnDepositDelete: \n record.getLocation(): {} \n deposit-path-argument: {}\n depositPropertiesFile.exists() : {}\n", record.get().getLocation(), + depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), depositPropertiesFile.exists()); try { if (Files.isSameFile(Path.of(record.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath())) && !depositPropertiesFile.exists()) { String depositId = depositPropertiesFile.getParentFile().getName(); Optional deletedNumber = depositPropertiesDAO.updateDeleteFlag(depositId, true); log.debug("onDepositDelete - 'deleted' mark has been set to '{}' for deposit.properties from '{}' ", deletedNumber.isPresent() && deletedNumber.get() > 0, depositId); } - } catch (IOException e) { + } + catch (IOException e) { throw new RuntimeException(e); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java index f67d5fa..492d7ee 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java @@ -16,25 +16,28 @@ package nl.knaw.dans.managedeposit.core.service; import org.apache.commons.io.filefilter.AbstractFileFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; import java.nio.file.Path; public class DepthFileFilter extends AbstractFileFilter { - private final Path baseFolder; - private final int depthLimit; + private static final Logger log = LoggerFactory.getLogger(DepthFileFilter.class); + private final Path absoluteBaseFolder; + private final int requiredNameCount; - public DepthFileFilter(Path baserFolder, int depthLimit) { - this.baseFolder = baserFolder; - this.depthLimit = depthLimit; + public DepthFileFilter(Path baseFolder, int depthLimit) { + this.absoluteBaseFolder = baseFolder.toAbsolutePath(); + this.requiredNameCount = this.absoluteBaseFolder.getNameCount() + depthLimit; } private boolean confirmParent(File file) { - Path parent = file.toPath(); - for (int depth = 0; depth < this.depthLimit; depth++) - parent = parent.getParent(); - - return parent.endsWith(this.baseFolder); + var path = file.getAbsoluteFile().toPath(); + if (!path.startsWith(absoluteBaseFolder)) { + log.warn(String.format("[%s] must be a child of [%s]", path, absoluteBaseFolder)); + } + return path.getNameCount() == requiredNameCount; } @Override diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java index 74e79e8..f367890 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java @@ -28,5 +28,4 @@ public static String stripBegin(String text, int maxLength) { return textLength > maxLength ? text.substring(textLength - maxLength) : text; } - } diff --git a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java index 5647061..5873dc7 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -20,7 +20,12 @@ import org.hibernate.SessionFactory; import org.hibernate.query.Query; -import javax.persistence.criteria.*; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaDelete; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.CriteriaUpdate; +import javax.persistence.criteria.Predicate; +import javax.persistence.criteria.Root; import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.ZoneOffset; diff --git a/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java b/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java index 1519215..a64b39b 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java +++ b/src/main/java/nl/knaw/dans/managedeposit/health/InboxHealthCheck.java @@ -42,12 +42,14 @@ protected Result check() throws Exception { if (exists && canRead) { log.debug("Inbox path '{}' exists and is readable", folder); - } else { + } + else { valid = false; if (!exists) { log.debug("Inbox path '{}' does not exist", folder); - } else { + } + else { log.debug("Inbox path '{}' is not readable", folder); } } @@ -55,7 +57,8 @@ protected Result check() throws Exception { if (valid) { return Result.healthy(); - } else { + } + else { return Result.unhealthy("InboxPaths are not accessible"); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java index e78832a..1e008bc 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java @@ -18,7 +18,11 @@ import io.dropwizard.hibernate.UnitOfWork; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; -import javax.ws.rs.*; +import javax.ws.rs.Consumes; +import javax.ws.rs.NotFoundException; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.UriInfo; diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java index 5c725e3..46035d7 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java @@ -19,7 +19,11 @@ import nl.knaw.dans.managedeposit.core.DepositProperties; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; -import javax.ws.rs.*; +import javax.ws.rs.GET; +import javax.ws.rs.NotFoundException; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.UriInfo; import java.util.List; @@ -35,7 +39,7 @@ public DepositPropertiesReportResource(DepositPropertiesDAO depositPropertiesDAO @GET @UnitOfWork - @Produces({"application/json", "text/csv"}) + @Produces({ "application/json", "text/csv" }) public List listDepositProperties(@Context UriInfo uriInfo) { return depositPropertiesDAO.findSelection(uriInfo.getQueryParameters()); } diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java index ff1c50c..ad13abd 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -20,7 +20,11 @@ import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; import javax.validation.Valid; -import javax.ws.rs.*; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -36,31 +40,30 @@ public DepositPropertiesResource(DepositPropertiesDAO depositPropertiesDAO) { private String writeHelpInfoText() { return - """ - DD Manage Deposit is running.\s - Usage:\s - - Create reports: GET basePath/report\s - - Clean database: POST basePath/delete-deposit\s - Query string parameters: user, state, startdate, enddate\s - 'startdate'/'enddate' format: yyyy-MM-dd\s - Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s - Examples:\s - curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s - curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s - curl -i -X POST basePath/delete-deposit?user=XXX\s - curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; + """ + DD Manage Deposit is running.\s + Usage:\s + - Create reports: GET basePath/report\s + - Clean database: POST basePath/delete-deposit\s + Query string parameters: user, state, startdate, enddate\s + 'startdate'/'enddate' format: yyyy-MM-dd\s + Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s + Examples:\s + curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s + curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s + curl -i -X POST basePath/delete-deposit?user=XXX\s + curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; } @GET @UnitOfWork public Response getApiInformation() { - return Response - .status(Response.Status.OK) - .entity(this.helpInfo) - .type(MediaType.TEXT_PLAIN) - .build(); + .status(Response.Status.OK) + .entity(this.helpInfo) + .type(MediaType.TEXT_PLAIN) + .build(); } @POST From 701ff278d71d220e722691b5ae7ae7bb486b08fc Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Sun, 21 Apr 2024 23:10:46 +0200 Subject: [PATCH 20/25] DD-1438 resolve reviews -2 --- .../nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java index 492d7ee..b200053 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java @@ -36,6 +36,7 @@ private boolean confirmParent(File file) { var path = file.getAbsoluteFile().toPath(); if (!path.startsWith(absoluteBaseFolder)) { log.warn(String.format("[%s] must be a child of [%s]", path, absoluteBaseFolder)); + return false; } return path.getNameCount() == requiredNameCount; } From 06a8a983a082a40cdc4c4aefac73dc9658311ecd Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Mon, 22 Apr 2024 15:04:41 +0200 Subject: [PATCH 21/25] DD-1438 Creation time will not be set by dd-manage-deposit --- .../service/DepositPropertiesAssembler.java | 2 +- .../db/DepositPropertiesDAO.java | 73 ++++++++++--------- 2 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java index c59c27d..947820d 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java @@ -48,7 +48,7 @@ Optional assembleObject(File depositPropertiesFile, boolean u configuration.getString("bag-store.bag-name", ""), configuration.getString("state.label", ""), TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), - OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), + OffsetDateTime.parse(configuration.getString("creation.timestamp", "")), TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); diff --git a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java index 5873dc7..88d9058 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -106,43 +106,44 @@ private Predicate buildQueryCriteria(Map> queryParameters, Predicate orPredicateItem; List orPredicatesList = new ArrayList<>(); for (String value : values) { - switch (parameter) { - case "depositid": - orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); - break; - - case "user": - orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); - break; - - case "deleted": - if (Boolean.parseBoolean(value)) - orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); - else - orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); - break; - - case "state": - orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); - break; - - case "startdate": - case "enddate": - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - LocalDate date = LocalDate.parse(value, formatter); - var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); - - if (parameter.equals("startdate")) - orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); - else - orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); - break; - - default: - orPredicateItem = criteriaBuilder.equal(root.get(key), value); + if (!value.isEmpty()) { + switch (parameter) { + case "depositid": + orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); + break; + + case "user": + orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); + break; + + case "deleted": + if (Boolean.parseBoolean(value)) + orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); + else + orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); + break; + + case "state": + orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); + break; + + case "startdate": + case "enddate": + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDate date = LocalDate.parse(value, formatter); + var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); + + if (parameter.equals("startdate")) + orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); + else + orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); + break; + + default: + orPredicateItem = criteriaBuilder.equal(root.get(key), value); + } + orPredicatesList.add(orPredicateItem); } - - orPredicatesList.add(orPredicateItem); } orPredicateItem = criteriaBuilder.or(orPredicatesList.toArray(new Predicate[0])); From 254e2fd4b3d682212dd95c51b18eee0741936df0 Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Tue, 23 Apr 2024 15:54:31 +0200 Subject: [PATCH 22/25] DD-1438 dd-manage-deposits: Fix problems found DD-1419 (#6) * DD-1492 dd-manage-deposits improvemnets fixes for DD-1419 - Part 2 directory search depth limit * DD-1492 dd-manage-deposits improvemnets fixes for DD-1419 - Part 2 directory search depth limit * DD-1492 dd-manage-deposits improvemnet and fixes for DD-1419 * DD-1492 dd-manage-deposits improvemnet and fixes for DD-1419 * Update RPM link was not correct * Update DepositProperties.java (expand `import *`) * changed `List` to `var` IngestPathMonitor.java * testing IngestPathMonitor * follow rename in PR, license header * test DepositStausUpdater * fix test with false assumptions * fix depthfilter * maximized coverage of IngestPathMonitor * detailed expectations of onDepositUpdate * abused assumeNotYetImplemented * obsolete todo * Apply suggestions from code review use dao.findAll() in a transaction process Co-authored-by: Jo Pol * Apply suggestions from code review Typo correction Co-authored-by: Jo Pol * DD-1438 resolve reviews * DD-1438 resolve reviews -2 * DD-1438 Creation time will not be set by dd-manage-deposit --------- Co-authored-by: Ali Sheikhi Co-authored-by: Jan van Mansum Co-authored-by: jo-pol Co-authored-by: Ali Sheikhi --- pom.xml | 39 +++++ src/main/assembly/dist/bin/dd-manage-deposit | 2 +- src/main/assembly/dist/cfg/config.yml | 2 +- .../DdManageDepositApplication.java | 17 +-- .../DdManageDepositConfiguration.java | 5 +- .../core/CsvMessageBodyWriter.java | 4 +- .../managedeposit/core/DepositProperties.java | 15 +- .../service/DepositPropertiesAssembler.java | 30 ++-- .../service/DepositPropertiesFileReader.java | 2 +- .../core/service/DepositStatusUpdater.java | 62 ++++---- .../core/service/DepthFileFilter.java | 53 +++++++ .../core/service/IngestPathMonitor.java | 37 +++-- .../core/service/TextTruncation.java | 6 +- .../db/DepositPropertiesDAO.java | 92 +++++------- .../resources/DepositPropertiesResource.java | 26 ++-- .../managedeposit/AbstractDatabaseTest.java | 37 +++++ .../AbstractTestWithTestDir.java | 42 ++++++ ...positStatusUpdaterOnDepositUpdateTest.java | 88 ++++++++++++ .../core/service/IngestPathMonitorTest.java | 135 +++++++++++++++++- 19 files changed, 541 insertions(+), 153 deletions(-) create mode 100644 src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java create mode 100644 src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java create mode 100644 src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java create mode 100644 src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java diff --git a/pom.xml b/pom.xml index bb37321..c81ba1f 100644 --- a/pom.xml +++ b/pom.xml @@ -87,6 +87,45 @@ nl.knaw.dans dans-java-utils + + junit + junit + RELEASE + test + + + org.mockito + mockito-core + 5.8.0 + test + + + org.junit.jupiter + junit-jupiter + test + + + io.dropwizard + dropwizard-testing + 3.0.5 + test + + + com.h2database + h2 + test + + + org.assertj + assertj-core + test + + + org.mockito + mockito-junit-jupiter + 5.4.0 + test + diff --git a/src/main/assembly/dist/bin/dd-manage-deposit b/src/main/assembly/dist/bin/dd-manage-deposit index fe00670..0625e07 100644 --- a/src/main/assembly/dist/bin/dd-manage-deposit +++ b/src/main/assembly/dist/bin/dd-manage-deposit @@ -5,4 +5,4 @@ BINPATH=$(command readlink -f $0 2> /dev/null || command grealpath $0 2> /dev/nu APPHOME=$(dirname $(dirname $BINPATH)) CONFIG_PATH=/etc/opt/dans.knaw.nl/$SCRIPTNAME/config.yml -java -Ddans.default.config=$CONFIG_PATH -jar $APPHOME/bin/$SCRIPTNAME.jar "$@" +java $DANS_JAVA_OPTS -Ddans.default.config=$CONFIG_PATH -jar $APPHOME/bin/$SCRIPTNAME.jar "$@" diff --git a/src/main/assembly/dist/cfg/config.yml b/src/main/assembly/dist/cfg/config.yml index e941205..cec3712 100644 --- a/src/main/assembly/dist/cfg/config.yml +++ b/src/main/assembly/dist/cfg/config.yml @@ -18,7 +18,7 @@ depositBoxes: - /var/opt/dans.knaw.nl/tmp/auto-ingest/outbox/failed - /var/opt/dans.knaw.nl/tmp/sword2-uploads -pollingInterval: 5000 +pollingInterval: 500 depositPropertiesDatabase: driverClass: org.postgresql.Driver diff --git a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java index e5428f2..9326387 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java +++ b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositApplication.java @@ -17,11 +17,11 @@ package nl.knaw.dans.managedeposit; import io.dropwizard.core.Application; +import io.dropwizard.core.setup.Bootstrap; +import io.dropwizard.core.setup.Environment; import io.dropwizard.db.DataSourceFactory; import io.dropwizard.hibernate.HibernateBundle; import io.dropwizard.hibernate.UnitOfWorkAwareProxyFactory; -import io.dropwizard.core.setup.Bootstrap; -import io.dropwizard.core.setup.Environment; import nl.knaw.dans.managedeposit.core.CsvMessageBodyWriter; import nl.knaw.dans.managedeposit.core.DepositProperties; import nl.knaw.dans.managedeposit.core.service.DepositStatusUpdater; @@ -34,10 +34,6 @@ public class DdManageDepositApplication extends Application { - public static void main(final String[] args) throws Exception { - new DdManageDepositApplication().run(args); - } - private final HibernateBundle depositPropertiesHibernate = new HibernateBundle<>(DepositProperties.class) { @@ -47,6 +43,10 @@ public DataSourceFactory getDataSourceFactory(DdManageDepositConfiguration confi } }; + public static void main(final String[] args) throws Exception { + new DdManageDepositApplication().run(args); + } + @Override public String getName() { return "Dd Manage Deposit"; @@ -70,13 +70,10 @@ public void run(final DdManageDepositConfiguration configuration, final Environm final UnitOfWorkAwareProxyFactory proxyFactory = new UnitOfWorkAwareProxyFactory(depositPropertiesHibernate); DepositStatusUpdater depositStatusUpdater = proxyFactory.create( - DepositStatusUpdater.class, - new Class[] { DepositPropertiesDAO.class }, - new Object[] { depositPropertiesDAO }); + DepositStatusUpdater.class, DepositPropertiesDAO.class, depositPropertiesDAO); final IngestPathMonitor ingestPathMonitor = new IngestPathMonitor(configuration.getDepositBoxes(), depositStatusUpdater, configuration.getPollingInterval()); environment.lifecycle().manage(ingestPathMonitor); - } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java index 70fc225..e492e40 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java +++ b/src/main/java/nl/knaw/dans/managedeposit/DdManageDepositConfiguration.java @@ -18,7 +18,6 @@ import io.dropwizard.core.Configuration; import io.dropwizard.db.DataSourceFactory; -import nl.knaw.dans.managedeposit.core.service.TextTruncation; import javax.validation.Valid; import javax.validation.constraints.NotNull; @@ -26,7 +25,9 @@ import java.util.ArrayList; import java.util.List; +@SuppressWarnings("unused") public class DdManageDepositConfiguration extends Configuration { + private static final long DEFAULT_POLLING_INTERVAL = 500; @Valid @NotNull private DataSourceFactory database = new DataSourceFactory(); @@ -52,7 +53,7 @@ public void setDepositPropertiesDatabase(DataSourceFactory database) { } public long getPollingInterval() { - return pollingInterval > 0 ? pollingInterval : TextTruncation.pollingInterval; + return pollingInterval > 0 ? pollingInterval : DEFAULT_POLLING_INTERVAL; } public void setPollingInterval(long pollingInterval) { diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java index 3236949..4bf5d27 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java @@ -45,14 +45,14 @@ public boolean isWriteable(Class type, Type genericType, Annotation[] annotation @Override public void writeTo(List data, Class aClass, Type type, Annotation[] annotations, MediaType mediaType, MultivaluedMap multivaluedMap, OutputStream outputStream) throws IOException, WebApplicationException { - if (data != null && data.size() > 0) { + if (data != null && !data.isEmpty()) { // TODO: pass the mapper in at configuration time CsvMapper mapper = new CsvMapper(); Object o = data.get(0); CsvSchema schema = mapper.schemaFor(o.getClass()) .withHeader() .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") - .rebuild().build(); + .rebuild().build(); mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); mapper.registerModule(new JavaTimeModule()); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index 177416a..03bb270 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -30,6 +30,8 @@ name = "showAll", query = "SELECT dp FROM DepositProperties dp" ) + +@SuppressWarnings("unused") public class DepositProperties { @Column(name = "depositor", nullable = false) // depositor.userId @@ -49,10 +51,10 @@ public class DepositProperties { @Column(name = "deposit_update_timestamp") // modified timestamp of deposit.properties private OffsetDateTime depositUpdateTimestamp; - @Column(name = "description", length = TextTruncation.maxDescriptionLength) // state.description + @Column(name = "description", length = TextTruncation.MAX_DESCRIPTION_LENGTH) // state.description private String description; - @Column(name = "location", length = TextTruncation.maxDirectoryLength) // full parent-path on disk + @Column(name = "location", length = TextTruncation.MAX_DIRECTORY_LENGTH) // full parent-path on disk private String location; @Column(name = "storage_in_bytes") // Total storage of deposit directory @@ -60,9 +62,6 @@ public class DepositProperties { @Column(name = "deleted") // deposit is deleted from inbox - archived private boolean deleted; - public String getDepositId() { - return depositId; - } public DepositProperties() { } @@ -79,6 +78,10 @@ public DepositProperties(String depositId, String depositor, String bagName, Str this.storageInBytes = storageInBytes; } + public String getDepositId() { + return depositId; + } + public String getDepositor() { return depositor; } @@ -169,4 +172,4 @@ public boolean equals(Object o) { public int hashCode() { return 31 * depositId.hashCode() + depositor.hashCode(); } -} \ No newline at end of file +} diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java index 1a18fb7..947820d 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java @@ -34,7 +34,7 @@ class DepositPropertiesAssembler { DepositPropertiesAssembler() { } - Optional assembleObject(File depositPropertiesFile, boolean updateModificationDateTime) { + Optional assembleObject(File depositPropertiesFile, boolean updateModificationDateTime, long CalculatedFolderSize) { Path depositPath = depositPropertiesFile.getParentFile().toPath(); log.debug("assembleObject(depositPropertiesPath:Path): '{}'", depositPropertiesFile.getAbsolutePath()); @@ -47,10 +47,10 @@ Optional assembleObject(File depositPropertiesFile, boolean configuration.getString("depositor.userId", ""), configuration.getString("bag-store.bag-name", ""), configuration.getString("state.label", ""), - TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.maxDescriptionLength), - OffsetDateTime.parse(configuration.getString("creation.timestamp", OffsetDateTime.now().toString())), - TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.maxDirectoryLength), - calculateFolderSize(depositPath)); + TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), + OffsetDateTime.parse(configuration.getString("creation.timestamp", "")), + TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), + CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); if (updateModificationDateTime) { dp.setDepositUpdateTimestamp(OffsetDateTime.now()); @@ -68,15 +68,17 @@ Optional assembleObject(File depositPropertiesFile, boolean } private long calculateFolderSize(Path path) { - long size; - try (var pathStream = Files.walk(path)) { - size = pathStream - .filter(p -> p.toFile().isFile()) - .mapToLong(p -> p.toFile().length()) - .sum(); - } - catch (IOException e) { - throw new RuntimeException(e); + long size = 0; + if (Files.exists(path)) { + try (var pathStream = Files.walk(path)) { + size = pathStream + .filter(p -> p.toFile().isFile()) + .mapToLong(p -> p.toFile().length()) + .sum(); + } + catch (IOException e) { + throw new RuntimeException(e); + } } return size; diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java index cfe8402..14a89ee 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesFileReader.java @@ -26,7 +26,7 @@ import java.io.File; class DepositPropertiesFileReader { - private static final Logger log = LoggerFactory.getLogger(DepositPropertiesAssembler.class); + private static final Logger log = LoggerFactory.getLogger(DepositPropertiesFileReader.class); static public Configuration readDepositProperties(File propertiesFile) throws ConfigurationException { log.debug("readDepositProperties: '{}'", propertiesFile.toString()); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java index 936fa79..e66adac 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java @@ -28,7 +28,7 @@ import java.util.Optional; public class DepositStatusUpdater { - private static final Logger log = LoggerFactory.getLogger(DepositPropertiesAssembler.class); + private static final Logger log = LoggerFactory.getLogger(DepositStatusUpdater.class); private final DepositPropertiesDAO depositPropertiesDAO; private final DepositPropertiesAssembler depositPropertiesAssembler; @@ -38,46 +38,52 @@ public DepositStatusUpdater(DepositPropertiesDAO depositPropertiesDAO) { } @UnitOfWork - public void onCreateDeposit(File depositPropertiesFile) { - Optional dp = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + public void onDepositCreate(File depositPropertiesFile) { + // Note: the 'move deposit' action is processed by the system in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); - if (dp.isPresent()) { - // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. - // Update the location column. - Path depositLocationFolder = Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath()); - Optional updatedNumber = depositPropertiesDAO.updateDepositLocation(dp.get().getDepositId(), depositLocationFolder); - if (updatedNumber.isPresent()) - log.debug("onCreateDeposit - `location` of deposit '{}' has been updated to '{}' ", dp.get().getDepositId(), depositLocationFolder); + if (record.isPresent()) { + // Update the location column and other fields + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, record.get().getStorageInBytes()); + dpObject.ifPresent(depositPropertiesDAO::merge); + log.debug("onDepositCreate - The deposit '{}' location and/or state has been updated to '{}' ", record.get().getDepositId(), depositPropertiesFile.getParentFile().getAbsolutePath()); } else { - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false); + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false, 0); dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + log.debug("onDepositCreate: A new deposit has been registered '{}'", + depositPropertiesFile.getParentFile().getAbsolutePath());//log.info("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); } } @UnitOfWork - public void onChangeDeposit(File depositPropertiesFile) { - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true); - dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onChangeDeposit: deposit.properties has been changed `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + public void onDepositChange(File depositPropertiesFile) { + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + long folder_size = record.isPresent() ? record.get().getStorageInBytes() : 0; + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, folder_size); + dpObject.ifPresent(depositPropertiesDAO::merge); + log.debug("onDepositChange: deposit.properties has been changed '{}'", depositPropertiesFile.getParentFile().getAbsolutePath()); } @UnitOfWork - public void onDeleteDeposit(File depositPropertiesFile) { - // At this stage, the deposit.properties file's handle is present but the content is null (impossible to read data of the file) - Optional dp = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); + public void onDepositDelete(File depositPropertiesFile) { + // Note: if delete notify is part of a folder moving, then at this stage, the deposit.properties file's handle is present but the content is null (impossible to read data of the file) + Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); - try { - // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step - if (dp.isPresent() && Files.isSameFile(Path.of(dp.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath()))) { - String depositId = depositPropertiesFile.getParentFile().getName(); - Optional deletedNumber = depositPropertiesDAO.updateDeleteFlag(depositId, true); - log.debug("onDeleteDeposit - 'deleted' mark has been set to '{}' for deposit.properties from '{}' ", deletedNumber.isPresent(), depositId); + // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step + if (record.isPresent()) { + log.debug("OnDepositDelete: \n record.getLocation(): {} \n deposit-path-argument: {}\n depositPropertiesFile.exists() : {}\n", record.get().getLocation(), + depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), depositPropertiesFile.exists()); + try { + if (Files.isSameFile(Path.of(record.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath())) && !depositPropertiesFile.exists()) { + String depositId = depositPropertiesFile.getParentFile().getName(); + Optional deletedNumber = depositPropertiesDAO.updateDeleteFlag(depositId, true); + log.debug("onDepositDelete - 'deleted' mark has been set to '{}' for deposit.properties from '{}' ", deletedNumber.isPresent() && deletedNumber.get() > 0, depositId); + } + } + catch (IOException e) { + throw new RuntimeException(e); } - } - catch (IOException e) { - log.debug("The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step for {}", depositPropertiesFile.getParentFile().getAbsolutePath()); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java new file mode 100644 index 0000000..b200053 --- /dev/null +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepthFileFilter.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit.core.service; + +import org.apache.commons.io.filefilter.AbstractFileFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.nio.file.Path; + +public class DepthFileFilter extends AbstractFileFilter { + private static final Logger log = LoggerFactory.getLogger(DepthFileFilter.class); + private final Path absoluteBaseFolder; + private final int requiredNameCount; + + public DepthFileFilter(Path baseFolder, int depthLimit) { + this.absoluteBaseFolder = baseFolder.toAbsolutePath(); + this.requiredNameCount = this.absoluteBaseFolder.getNameCount() + depthLimit; + } + + private boolean confirmParent(File file) { + var path = file.getAbsoluteFile().toPath(); + if (!path.startsWith(absoluteBaseFolder)) { + log.warn(String.format("[%s] must be a child of [%s]", path, absoluteBaseFolder)); + return false; + } + return path.getNameCount() == requiredNameCount; + } + + @Override + public boolean accept(File file) { + return confirmParent(file); + } + + @Override + public boolean accept(File dir, String name) { + return confirmParent(dir); + } +} diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java index 179b49e..b0cfbd2 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitor.java @@ -18,7 +18,6 @@ import io.dropwizard.lifecycle.Managed; import org.apache.commons.io.IOCase; import org.apache.commons.io.filefilter.FileFilterUtils; -import org.apache.commons.io.filefilter.HiddenFileFilter; import org.apache.commons.io.filefilter.IOFileFilter; import org.apache.commons.io.monitor.FileAlterationListenerAdaptor; import org.apache.commons.io.monitor.FileAlterationMonitor; @@ -47,23 +46,31 @@ public IngestPathMonitor(List depositBoxesPaths, DepositStatusUpdater depo } private void startMonitors() throws Exception { - IOFileFilter directories = FileFilterUtils.and(FileFilterUtils.directoryFileFilter(), HiddenFileFilter.VISIBLE); - IOFileFilter files = FileFilterUtils.and(FileFilterUtils.fileFileFilter(), FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE)); - IOFileFilter filter = FileFilterUtils.or(directories, files); - - log.info("Starting 'IngestPathMonitor', file filter: deposit.properties"); + log.info("Starting 'IngestPathMonitor', with file filter: deposit.properties"); + var observers = new ArrayList<>(); for (Path folder : toMonitorPaths) { - FileAlterationObserver observer = new FileAlterationObserver(folder.toFile(), filter); + IOFileFilter directories = FileFilterUtils.and( + FileFilterUtils.directoryFileFilter(), + new DepthFileFilter(folder, 1) + ); + IOFileFilter files = FileFilterUtils.and( + FileFilterUtils.fileFileFilter(), + FileFilterUtils.nameFileFilter("deposit.properties", IOCase.INSENSITIVE), + new DepthFileFilter(folder, 2) + ); + IOFileFilter filter = FileFilterUtils.or(directories, files); + FileAlterationObserver observer = new FileAlterationObserver(folder.toFile(), filter); observer.addListener(this); + observers.add(observer); - FileAlterationMonitor monitor = new FileAlterationMonitor(this.pollingInterval, observer); - fileAlterationMonitors.add(monitor); - - monitor.start(); - log.debug("'IngestPathMonitor' is going to monitor the folder '{}'", folder); } + + FileAlterationMonitor monitor = new FileAlterationMonitor(this.pollingInterval, observers.toArray(new FileAlterationObserver[0])); + fileAlterationMonitors.add(monitor); + log.debug("'IngestPathMonitor' is going to monitor the folders\n '{}'", toMonitorPaths); + monitor.start(); } @Override @@ -94,19 +101,19 @@ public void stop() throws Exception { @Override public void onFileCreate(File file) { log.debug("onFileCreate: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onCreateDeposit(file); + depositStatusUpdater.onDepositCreate(file); } @Override public void onFileDelete(File file) { log.debug("onFileDelete: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onDeleteDeposit(file); + depositStatusUpdater.onDepositDelete(file); } @Override public void onFileChange(File file) { log.debug("onFileChange: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onChangeDeposit(file); + depositStatusUpdater.onDepositChange(file); } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java index 4192693..f367890 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/TextTruncation.java @@ -16,9 +16,8 @@ package nl.knaw.dans.managedeposit.core.service; public class TextTruncation { - public static final long pollingInterval = 5 * 1000; - public static final int maxDescriptionLength = 1024; - public static final int maxDirectoryLength = 512; + public static final int MAX_DESCRIPTION_LENGTH = 1024; + public static final int MAX_DIRECTORY_LENGTH = 512; public static String stripEnd(String text, int maxLength) { return text.length() > maxLength ? text.substring(0, maxLength) : text; @@ -29,5 +28,4 @@ public static String stripBegin(String text, int maxLength) { return textLength > maxLength ? text.substring(textLength - maxLength) : text; } - } diff --git a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java index 1db4751..88d9058 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -26,7 +26,6 @@ import javax.persistence.criteria.CriteriaUpdate; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; -import java.nio.file.Path; import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.ZoneOffset; @@ -70,7 +69,7 @@ public List findAll() { public List findSelection(Map> queryParameters) { CriteriaBuilder criteriaBuilder = currentSession().getCriteriaBuilder(); - if (queryParameters.size() == 0) + if (queryParameters.isEmpty()) return findAll(); CriteriaQuery criteriaQuery = criteriaBuilder.createQuery(DepositProperties.class); @@ -83,7 +82,7 @@ public List findSelection(Map> queryPara public Optional deleteSelection(Map> queryParameters) { var criteriaBuilder = currentSession().getCriteriaBuilder(); - if (queryParameters.size() == 0) // Note: all records will be deleted (accidentally) without any specified query parameter + if (queryParameters.isEmpty()) // Note: all records will be deleted (accidentally) without any specified query parameter return Optional.of(0); CriteriaDelete deleteQuery = criteriaBuilder.createCriteriaDelete(DepositProperties.class); @@ -107,43 +106,44 @@ private Predicate buildQueryCriteria(Map> queryParameters, Predicate orPredicateItem; List orPredicatesList = new ArrayList<>(); for (String value : values) { - switch (parameter) { - case "depositid": - orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); - break; - - case "user": - orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); - break; - - case "deleted": - if (Boolean.parseBoolean(value)) - orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); - else - orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); - break; - - case "state": - orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); - break; - - case "startdate": - case "enddate": - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - LocalDate date = LocalDate.parse(value, formatter); - var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); - - if (parameter.equals("startdate")) - orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); - else - orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); - break; - - default: - orPredicateItem = criteriaBuilder.equal(root.get(key), value); + if (!value.isEmpty()) { + switch (parameter) { + case "depositid": + orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); + break; + + case "user": + orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); + break; + + case "deleted": + if (Boolean.parseBoolean(value)) + orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); + else + orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); + break; + + case "state": + orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); + break; + + case "startdate": + case "enddate": + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDate date = LocalDate.parse(value, formatter); + var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); + + if (parameter.equals("startdate")) + orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); + else + orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); + break; + + default: + orPredicateItem = criteriaBuilder.equal(root.get(key), value); + } + orPredicatesList.add(orPredicateItem); } - - orPredicatesList.add(orPredicateItem); } orPredicateItem = criteriaBuilder.or(orPredicatesList.toArray(new Predicate[0])); @@ -169,18 +169,4 @@ public Optional updateDeleteFlag(String depositId, boolean deleted) { return Optional.of(query.executeUpdate()); } - public Optional updateDepositLocation(String depositId, Path currentParentPath) { - CriteriaBuilder criteriaBuilder = currentSession().getCriteriaBuilder(); - CriteriaUpdate criteriaUpdate = criteriaBuilder.createCriteriaUpdate(DepositProperties.class); - Root root = criteriaUpdate.from(DepositProperties.class); - - Predicate predicate = buildQueryCriteria(Map.of("depositId", List.of(depositId)), criteriaBuilder, root); - criteriaUpdate.where(predicate); - - criteriaUpdate.set("location", currentParentPath.toString()); - - var query = currentSession().createQuery(criteriaUpdate); - return Optional.of(query.executeUpdate()); - } - } diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java index aef3290..ad13abd 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -40,25 +40,25 @@ public DepositPropertiesResource(DepositPropertiesDAO depositPropertiesDAO) { private String writeHelpInfoText() { return - "DD Manage Deposit is running. \n" + - "Usage: \n" + - " - Create reports: GET basePath/report \n" + - " - Clean database: POST basePath/delete-deposit \n" + - " Query string parameters: user, state, startdate, enddate \n" + - " 'startdate'/'enddate' format: yyyy-MM-dd \n" + - " Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED \n" + - " Examples: \n" + - " curl -i -X GET basePath/report?startdate=yyyy-MM-dd \n" + - " curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED \n" + - " curl -i -X POST basePath/delete-deposit?user=XXX \n" + - " curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"; + """ + DD Manage Deposit is running.\s + Usage:\s + - Create reports: GET basePath/report\s + - Clean database: POST basePath/delete-deposit\s + Query string parameters: user, state, startdate, enddate\s + 'startdate'/'enddate' format: yyyy-MM-dd\s + Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s + Examples:\s + curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s + curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s + curl -i -X POST basePath/delete-deposit?user=XXX\s + curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; } @GET @UnitOfWork public Response getApiInformation() { - return Response .status(Response.Status.OK) .entity(this.helpInfo) diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java new file mode 100644 index 0000000..c9e88bb --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractDatabaseTest.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit; + +import io.dropwizard.testing.junit5.DAOTestExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import nl.knaw.dans.managedeposit.core.DepositProperties; +import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(DropwizardExtensionsSupport.class) +public abstract class AbstractDatabaseTest extends AbstractTestWithTestDir { + protected final DAOTestExtension daoTestExtension = DAOTestExtension.newBuilder() + .addEntityClass(DepositProperties.class) + .build(); + protected DepositPropertiesDAO dao; + + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + dao = new DepositPropertiesDAO(daoTestExtension.getSessionFactory()); + } +} diff --git a/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java new file mode 100644 index 0000000..950d084 --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/AbstractTestWithTestDir.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit; + +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.BeforeEach; + +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +public class AbstractTestWithTestDir { + protected final Path testDir = Path.of("target/test") + .resolve(getClass().getSimpleName()); + + @BeforeEach + public void setUp() throws Exception { + FileUtils.deleteDirectory(testDir.toFile()); + } + + /** + * Assume that a bug is not yet fixed. This allows to skip assertions while still showing the code covered by the test. + * + * @param message the message to display + */ + public void assumeNotYetFixed (String message) { + assumeTrue(false, message); + } +} diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java new file mode 100644 index 0000000..4cc3322 --- /dev/null +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2023 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package nl.knaw.dans.managedeposit.core.service; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import nl.knaw.dans.managedeposit.AbstractDatabaseTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.time.OffsetDateTime; + +import static java.nio.file.Files.createDirectories; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +public class DepositStatusUpdaterOnDepositUpdateTest extends AbstractDatabaseTest { + private ListAppender listAppender; + + @BeforeEach + public void setup() throws Exception { + super.setUp(); + var logger = (Logger) LoggerFactory.getLogger(DepositStatusUpdater.class); + listAppender = new ListAppender<>(); + listAppender.start(); + logger.setLevel(Level.DEBUG); + logger.addAppender(listAppender); + } + + @Test + public void onCreateDeposit_should_add_a_db_record() throws IOException { + var depositStatusUpdater = new DepositStatusUpdater(dao); + + // Prepare test data + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.writeString(propertiesFile, """ + creation.timestamp = 2023-08-16T17:40:41.390209+02:00 + depositor.userId = user001 + bag-store.bag-name = revision03 + """); + + // Call the method under test + depositStatusUpdater.onDepositCreate(propertiesFile.toFile()); + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("onDepositCreate: A new deposit has been registered '"); + assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); + + // Check the database + var maybeDepositProperties = daoTestExtension.inTransaction(() -> + dao.findById("bag") + ); + assertThat(maybeDepositProperties).isNotEmpty().get() + .hasFieldOrPropertyWithValue("depositId", "bag") + .hasFieldOrPropertyWithValue("depositor", "user001") + .hasFieldOrPropertyWithValue("bagName", "revision03") + .hasFieldOrPropertyWithValue("depositState", "") + .hasFieldOrPropertyWithValue("depositCreationTimestamp", OffsetDateTime.parse("2023-08-16T17:40:41.390209+02:00")) + .hasFieldOrPropertyWithValue("location", testDir.toAbsolutePath().toString()) + .hasFieldOrPropertyWithValue("storageInBytes", 113L); + + var depositPropertiesList = daoTestExtension.inTransaction(() -> + dao.findAll() + ); + assertThat(depositPropertiesList).hasSize(1); + } + + // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit +} \ No newline at end of file diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java index 27a320a..b7d2a39 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/IngestPathMonitorTest.java @@ -15,6 +15,135 @@ */ package nl.knaw.dans.managedeposit.core.service; -public class IngestPathMonitorTest { - // Stub -} +import nl.knaw.dans.managedeposit.AbstractTestWithTestDir; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.nio.file.Files; + +import static java.nio.file.Files.createDirectories; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class IngestPathMonitorTest extends AbstractTestWithTestDir { + + private IngestPathMonitor startMonitor(DepositStatusUpdater mockUpdater, int pollingInterval) throws Exception { + var monitor = new IngestPathMonitor(singletonList(testDir), mockUpdater, pollingInterval); + monitor.start(); + Thread.sleep(60); // wait for the monitor to get ready + return monitor; + } + + @Test + public void should_ignore_properties_in_root() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + createDirectories(testDir); + var propertiesFile = Files.createFile(testDir.resolve("deposit.properties")); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(0)) + .onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); + + monitor.stop(); + } + + @Test + public void should_pick_up_new_properties_in_bag() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); + + monitor.stop(); + } + + @Test + public void should_ignore_properties_in_bag_content() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var propertiesFile = testDir.resolve("bag/content/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + Mockito.verify(mockUpdater, Mockito.times(0)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); + + monitor.stop(); + } + + + @Test + public void should_pick_up_deleted_bag() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 50); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(70); + FileUtils.deleteDirectory(propertiesFile.getParent().toFile()); + Thread.sleep(70); + + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositDelete(propertiesFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_pick_up_changed_properties() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 20); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(30); + Files.writeString(propertiesFile, "just some garbage"); + Thread.sleep(30); + + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositChange(propertiesFile.toFile()); + + monitor.stop(); + } + + @Test + public void should_pick_up_deleted_root() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 20); + + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.createFile(propertiesFile); + Thread.sleep(30); + FileUtils.deleteDirectory(testDir.toFile()); + Thread.sleep(30); + + Mockito.verify(mockUpdater, Mockito.times(1)).onDepositCreate(propertiesFile.toFile()); + Mockito.verifyNoMoreInteractions(mockUpdater); + assumeNotYetFixed("The monitor should pick up the deletion of the root folder, it might imply deletion of many bags"); + monitor.stop(); + } + + @Test + public void should_throw_when_stopping_a_stopped_monitor() throws Exception { + var mockUpdater = Mockito.mock(DepositStatusUpdater.class); + var monitor = startMonitor(mockUpdater, 20); + monitor.stop(); + + assertThatThrownBy(monitor::stop) + .isInstanceOf(RuntimeException.class) + .hasRootCauseInstanceOf(IllegalStateException.class); + } +} \ No newline at end of file From 515ac46f69fa5412cc35f73d16dc26262479b164 Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Fri, 26 Apr 2024 00:34:28 +0200 Subject: [PATCH 23/25] DD-1438 Deposit Creation Time might be null --- .../core/CsvMessageBodyWriter.java | 4 +- .../managedeposit/core/DepositProperties.java | 19 +++- .../service/DepositPropertiesAssembler.java | 29 +++--- .../core/service/DepositStatusUpdater.java | 44 +++++++--- .../db/DepositPropertiesDAO.java | 88 +++++++++++-------- .../resources/DepositPropertiesResource.java | 8 +- ...positStatusUpdaterOnDepositUpdateTest.java | 81 ++++++++++++++++- 7 files changed, 199 insertions(+), 74 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java index 4bf5d27..43ae7bb 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/CsvMessageBodyWriter.java @@ -52,7 +52,9 @@ public void writeTo(List data, Class aClass, Type type, Annotation[] annotations CsvSchema schema = mapper.schemaFor(o.getClass()) .withHeader() .sortedBy("depositor", "depositId", "bagName", "depositState", "depositCreationTimestamp", "depositUpdateTimestamp", "description", "location", "storageInBytes", "deleted") - .rebuild().build(); + .rebuild() + .setNullValue("undefined") + .build(); mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); mapper.registerModule(new JavaTimeModule()); diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index 03bb270..c54f8a8 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -67,7 +67,7 @@ public DepositProperties() { } public DepositProperties(String depositId, String depositor, String bagName, String depositState, - String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes) { + String description, OffsetDateTime depositCreationTimestamp, String location, long storageInBytes, OffsetDateTime depositUpdateTimestamp) { this.depositId = depositId; this.depositor = depositor; this.bagName = bagName; @@ -76,6 +76,7 @@ public DepositProperties(String depositId, String depositor, String bagName, Str this.depositCreationTimestamp = depositCreationTimestamp; this.location = location; this.storageInBytes = storageInBytes; + this.depositUpdateTimestamp = depositUpdateTimestamp; } public String getDepositId() { @@ -172,4 +173,20 @@ public boolean equals(Object o) { public int hashCode() { return 31 * depositId.hashCode() + depositor.hashCode(); } + + @Override + public String toString() { + return "DepositProperties{" + + "depositor='" + depositor + '\'' + + ", depositId='" + depositId + '\'' + + ", bagName='" + bagName + '\'' + + ", depositState='" + depositState + '\'' + + ", depositCreationTimestamp=" + depositCreationTimestamp + + ", depositUpdateTimestamp=" + depositUpdateTimestamp + + ", description='" + description + '\'' + + ", location='" + location + '\'' + + ", storageInBytes=" + storageInBytes + + ", deleted=" + deleted + + '}'; + } } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java index 947820d..ad4adc7 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositPropertiesAssembler.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.DateTimeException; import java.time.OffsetDateTime; import java.util.Optional; @@ -34,37 +35,33 @@ class DepositPropertiesAssembler { DepositPropertiesAssembler() { } - Optional assembleObject(File depositPropertiesFile, boolean updateModificationDateTime, long CalculatedFolderSize) { + Optional assembleObject(File depositPropertiesFile, long CalculatedFolderSize) { Path depositPath = depositPropertiesFile.getParentFile().toPath(); log.debug("assembleObject(depositPropertiesPath:Path): '{}'", depositPropertiesFile.getAbsolutePath()); - DepositProperties dp; // = null + DepositProperties dp = null; Configuration configuration; try { configuration = DepositPropertiesFileReader.readDepositProperties(depositPropertiesFile); + String creationTime = configuration.getString("creation.timestamp"); dp = new DepositProperties(depositPath.getFileName().toString(), configuration.getString("depositor.userId", ""), configuration.getString("bag-store.bag-name", ""), configuration.getString("state.label", ""), TextTruncation.stripEnd(configuration.getString("state.description", ""), TextTruncation.MAX_DESCRIPTION_LENGTH), - OffsetDateTime.parse(configuration.getString("creation.timestamp", "")), + (creationTime == null || creationTime.isEmpty()) ? null : OffsetDateTime.parse(creationTime), TextTruncation.stripBegin(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), TextTruncation.MAX_DIRECTORY_LENGTH), - CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize); - - if (updateModificationDateTime) { - dp.setDepositUpdateTimestamp(OffsetDateTime.now()); - } - else { - dp.setDepositUpdateTimestamp(dp.getDepositCreationTimestamp()); - } - + CalculatedFolderSize == 0 ? calculateFolderSize(depositPath) : CalculatedFolderSize, + OffsetDateTime.now()); + } + catch (ConfigurationException | DateTimeException e) { + log.error("Error reading deposit.properties file: {}", e.getMessage()); } - catch (ConfigurationException e) { - log.error(e.getMessage(), e); - throw new RuntimeException(e); + catch (RuntimeException e) { + log.error("Error accessing deposit files : {}", e.getMessage()); } - return Optional.of(dp); + return Optional.ofNullable(dp); } private long calculateFolderSize(Path path) { diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java index e66adac..aeb5807 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdater.java @@ -44,25 +44,39 @@ public void onDepositCreate(File depositPropertiesFile) { if (record.isPresent()) { // Update the location column and other fields - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, record.get().getStorageInBytes()); - dpObject.ifPresent(depositPropertiesDAO::merge); - log.debug("onDepositCreate - The deposit '{}' location and/or state has been updated to '{}' ", record.get().getDepositId(), depositPropertiesFile.getParentFile().getAbsolutePath()); + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, record.get().getStorageInBytes()); + if (dpObject.isPresent()) { + depositPropertiesDAO.merge(dpObject.get()); + log.debug("onDepositCreate - The deposit '{}' location and/or state has been updated to '{}' ", record.get().getDepositId(), depositPropertiesFile.getParentFile().getAbsolutePath()); + } + else { + writeErrorMsg(depositPropertiesFile.getParentFile().getAbsolutePath()); + } } else { - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, false, 0); - dpObject.ifPresent(depositPropertiesDAO::save); - log.debug("onDepositCreate: A new deposit has been registered '{}'", - depositPropertiesFile.getParentFile().getAbsolutePath());//log.info("onCreateDeposit: A new deposit has been registered `{}`", depositPropertiesFile.getParentFile().getAbsolutePath()); + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, 0); + if (dpObject.isPresent()) { + depositPropertiesDAO.save(dpObject.get()); + log.debug("onDepositCreate: A new deposit has been registered '{}'", depositPropertiesFile.getParentFile().getAbsolutePath()); + } + else { + writeErrorMsg(depositPropertiesFile.getParentFile().getAbsolutePath()); + } } } @UnitOfWork public void onDepositChange(File depositPropertiesFile) { Optional record = depositPropertiesDAO.findById(depositPropertiesFile.getParentFile().getName()); - long folder_size = record.isPresent() ? record.get().getStorageInBytes() : 0; - Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, true, folder_size); - dpObject.ifPresent(depositPropertiesDAO::merge); - log.debug("onDepositChange: deposit.properties has been changed '{}'", depositPropertiesFile.getParentFile().getAbsolutePath()); + long folder_size = record.map(DepositProperties::getStorageInBytes).orElse(0L); + Optional dpObject = depositPropertiesAssembler.assembleObject(depositPropertiesFile, folder_size); + if (dpObject.isPresent()) { + depositPropertiesDAO.merge(dpObject.get()); + log.debug("onDepositChange: deposit.properties has been changed '{}'", depositPropertiesFile.getParentFile().getAbsolutePath()); + } + else { + writeErrorMsg(depositPropertiesFile.getParentFile().getAbsolutePath()); + } } @UnitOfWork @@ -72,8 +86,6 @@ public void onDepositDelete(File depositPropertiesFile) { // The 'move deposit' action is processed in two steps: 1. `create` deposit in the new location; 2. `delete` it from the old location. Ignore delete step if (record.isPresent()) { - log.debug("OnDepositDelete: \n record.getLocation(): {} \n deposit-path-argument: {}\n depositPropertiesFile.exists() : {}\n", record.get().getLocation(), - depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath(), depositPropertiesFile.exists()); try { if (Files.isSameFile(Path.of(record.get().getLocation()), Path.of(depositPropertiesFile.getParentFile().getParentFile().getAbsolutePath())) && !depositPropertiesFile.exists()) { String depositId = depositPropertiesFile.getParentFile().getName(); @@ -82,9 +94,13 @@ public void onDepositDelete(File depositPropertiesFile) { } } catch (IOException e) { - throw new RuntimeException(e); + writeErrorMsg(e.getMessage()); } } } + private void writeErrorMsg(String additionalInfo) { + log.error("Error creating / Updating deposit record: '{}'", additionalInfo); + } + } \ No newline at end of file diff --git a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java index 88d9058..f8d0dc0 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -19,6 +19,8 @@ import nl.knaw.dans.managedeposit.core.DepositProperties; import org.hibernate.SessionFactory; import org.hibernate.query.Query; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaDelete; @@ -26,6 +28,7 @@ import javax.persistence.criteria.CriteriaUpdate; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; +import java.time.DateTimeException; import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.ZoneOffset; @@ -37,6 +40,7 @@ @SuppressWarnings("resource") public class DepositPropertiesDAO extends AbstractDAO { + private static final Logger log = LoggerFactory.getLogger(DepositPropertiesDAO.class); public DepositPropertiesDAO(SessionFactory sessionFactory) { super(sessionFactory); @@ -106,44 +110,54 @@ private Predicate buildQueryCriteria(Map> queryParameters, Predicate orPredicateItem; List orPredicatesList = new ArrayList<>(); for (String value : values) { - if (!value.isEmpty()) { - switch (parameter) { - case "depositid": - orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); - break; - - case "user": - orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); - break; - - case "deleted": - if (Boolean.parseBoolean(value)) - orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); - else - orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); - break; - - case "state": - orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); - break; - - case "startdate": - case "enddate": - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - LocalDate date = LocalDate.parse(value, formatter); - var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); - - if (parameter.equals("startdate")) - orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); - else - orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); - break; - - default: - orPredicateItem = criteriaBuilder.equal(root.get(key), value); - } - orPredicatesList.add(orPredicateItem); + switch (parameter) { + case "depositid": + orPredicateItem = criteriaBuilder.equal(root.get("depositId"), value); + break; + + case "user": + orPredicateItem = criteriaBuilder.equal(root.get("depositor"), value); + break; + + case "deleted": + if (Boolean.parseBoolean(value)) + orPredicateItem = criteriaBuilder.isTrue(root.get("deleted")); + else + orPredicateItem = criteriaBuilder.isFalse(root.get("deleted")); + break; + + case "state": + orPredicateItem = criteriaBuilder.equal(root.get("depositState"), value); + break; + + case "startdate": + case "enddate": + //var dct = root.get("depositCreationTimestamp"); + if (value.isEmpty()) { + orPredicateItem = criteriaBuilder.isNull(root.get("depositCreationTimestamp")); + } + else { + try { + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDate date = LocalDate.parse(value, formatter); + var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); + + if (parameter.equals("startdate")) + orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); + else + orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); + } + catch (DateTimeException e) { + log.warn("Error parsing the date: {}", e.getMessage()); + continue; + } + } + break; + + default: + orPredicateItem = criteriaBuilder.equal(root.get(key), value); } + orPredicatesList.add(orPredicateItem); } orPredicateItem = criteriaBuilder.or(orPredicatesList.toArray(new Predicate[0])); diff --git a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java index ad13abd..51ce696 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -45,9 +45,11 @@ private String writeHelpInfoText() { Usage:\s - Create reports: GET basePath/report\s - Clean database: POST basePath/delete-deposit\s - Query string parameters: user, state, startdate, enddate\s - 'startdate'/'enddate' format: yyyy-MM-dd\s - Possible 'state' value: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s + - Query string parameters: user, state, startdate, enddate, deleted\s + - 'startdate'/'enddate' format: yyyy-MM-dd\s + - 'deleted' is a boolean with values: 'true' or 'false' + - Possible 'state' values: ARCHIVED, DRAFT, FAILED, FINALIZING, INVALID, REJECTED, SUBMITTED, UPLOADED, PUBLISHED\s + - To give an undefined parameter (when column's value is empty or null): 'parameterName=' (ex. 'user=')\s Examples:\s curl -i -X GET basePath/report?startdate=yyyy-MM-dd\s curl -i -X GET basePath/delete-deposit?user=XXX&state=REJECTED\s diff --git a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java index 4cc3322..6a841ca 100644 --- a/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java +++ b/src/test/java/nl/knaw/dans/managedeposit/core/service/DepositStatusUpdaterOnDepositUpdateTest.java @@ -66,7 +66,7 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); // Check the database - var maybeDepositProperties = daoTestExtension.inTransaction(() -> + var maybeDepositProperties = daoTestExtension.inTransaction(() -> dao.findById("bag") ); assertThat(maybeDepositProperties).isNotEmpty().get() @@ -78,11 +78,88 @@ public void onCreateDeposit_should_add_a_db_record() throws IOException { .hasFieldOrPropertyWithValue("location", testDir.toAbsolutePath().toString()) .hasFieldOrPropertyWithValue("storageInBytes", 113L); - var depositPropertiesList = daoTestExtension.inTransaction(() -> + var depositPropertiesList = daoTestExtension.inTransaction(() -> dao.findAll() ); assertThat(depositPropertiesList).hasSize(1); } + @Test + public void onCreateDeposit_should_create_new_db_record_when_creation_timestamp_is_empty() throws IOException { + var depositStatusUpdater = new DepositStatusUpdater(dao); + + // Prepare test data + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.writeString(propertiesFile, """ + creation.timestamp + depositor.userId = user001 + bag-store.bag-name = revision03 + """); + + // Call the method under test + depositStatusUpdater.onDepositCreate(propertiesFile.toFile()); + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("onDepositCreate: A new deposit has been registered '"); + assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); + + // Check the database + var maybeDepositProperties = daoTestExtension.inTransaction(() -> dao.findById("bag")); + assertThat(maybeDepositProperties).isNotEmpty(); + } + + @Test + public void onCreateDeposit_should_create_new_db_record_when_creation_timestamp_is_not_present() throws IOException { + var depositStatusUpdater = new DepositStatusUpdater(dao); + + // Prepare test data + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.writeString(propertiesFile, """ + depositor.userId = user001 + bag-store.bag-name = revision03 + """); + + // Call the method under test + depositStatusUpdater.onDepositCreate(propertiesFile.toFile()); + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("onDepositCreate: A new deposit has been registered '"); + assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); + + // Check the database + var maybeDepositProperties = daoTestExtension.inTransaction(() -> dao.findById("bag")); + assertThat(maybeDepositProperties).isNotEmpty(); + } + + @Test + public void onCreateDeposit_should_not_create_new_db_record_when_creation_timestamp_is_invalid() throws IOException { + var depositStatusUpdater = new DepositStatusUpdater(dao); + + // Prepare test data + var propertiesFile = testDir.resolve("bag/deposit.properties"); + createDirectories(propertiesFile.getParent()); + Files.writeString(propertiesFile, """ + creation.timestamp = 2023-08-16 17:40:41.390209+02:00 + depositor.userId = user001 + bag-store.bag-name = revision03 + """); + + // Call the method under test + depositStatusUpdater.onDepositCreate(propertiesFile.toFile()); + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("Error creating / Updating deposit record: '"); + assertThat(formattedMessage).endsWith("DepositStatusUpdaterOnDepositUpdateTest/bag'"); + + // Check the database + var maybeDepositProperties = daoTestExtension.inTransaction(() -> dao.findById("bag")); + assertThat(maybeDepositProperties).isEmpty(); + } + // TODO: other scenario's and test classes for onChangeDeposit and onDeleteDeposit } \ No newline at end of file From 977f428e98a441748e6ae93158314b302d84d64b Mon Sep 17 00:00:00 2001 From: Ali Sheikhi Date: Fri, 26 Apr 2024 01:13:48 +0200 Subject: [PATCH 24/25] DD-1438 Deposit Creation Time might be null - strange Build Error --- .../nl/knaw/dans/managedeposit/core/DepositProperties.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index 1cc52f5..c54f8a8 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -83,10 +83,6 @@ public String getDepositId() { return depositId; } - public String getDepositId() { - return depositId; - } - public String getDepositor() { return depositor; } From d607b6a0502fc399105ec4bbefd1767a3782767e Mon Sep 17 00:00:00 2001 From: Alias <93505057+aliassheikh@users.noreply.github.com> Date: Fri, 3 May 2024 10:40:03 +0200 Subject: [PATCH 25/25] Update src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java Commit Suggestion from Reviewer Joke Co-authored-by: Jo Pol --- .../java/nl/knaw/dans/managedeposit/core/DepositProperties.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java index c54f8a8..941e074 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/DepositProperties.java @@ -177,7 +177,7 @@ public int hashCode() { @Override public String toString() { return "DepositProperties{" + - "depositor='" + depositor + '\'' + + "depositor='" + depositor + "'" + ", depositId='" + depositId + '\'' + ", bagName='" + bagName + '\'' + ", depositState='" + depositState + '\'' +