diff --git a/docs/api/api.yml b/docs/api/api.yml index 51efc40..ab887d5 100755 --- a/docs/api/api.yml +++ b/docs/api/api.yml @@ -108,4 +108,4 @@ paths: '204': description: the deletion was carried out successfully '500': - description: the deletion could not be carred out + description: the deletion could not be carried out 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 b0cfbd2..5e8b92e 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 @@ -80,7 +80,7 @@ public void start() throws Exception { } catch (IOException | InterruptedException e) { log.error(e.getMessage(), e); - throw new InvalidTransferItemException(e.getMessage()); + throw new Exception(e); } } @@ -100,8 +100,12 @@ public void stop() throws Exception { @Override public void onFileCreate(File file) { + Path absolutePath = Path.of(file.getAbsolutePath()); log.debug("onFileCreate: '{}'", file.getAbsolutePath()); - depositStatusUpdater.onDepositCreate(file); + if (checkCleanBaseFolders(absolutePath)) + depositStatusUpdater.onDepositCreate(file); + else + log.error("Error: file '{}' is directly in a base-folder.", file.getAbsolutePath()); } @Override @@ -116,4 +120,13 @@ public void onFileChange(File file) { depositStatusUpdater.onDepositChange(file); } + private boolean checkCleanBaseFolders(Path depositPropertiesFile) { + Path parent = depositPropertiesFile.getParent(); + for (Path folder : toMonitorPaths) { + if (parent.endsWith(folder)) + return false; + } + return true; + } + } diff --git a/src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidTransferItemException.java b/src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidRequestParameterException.java similarity index 79% rename from src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidTransferItemException.java rename to src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidRequestParameterException.java index e736282..b3b813e 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidTransferItemException.java +++ b/src/main/java/nl/knaw/dans/managedeposit/core/service/InvalidRequestParameterException.java @@ -15,13 +15,13 @@ */ package nl.knaw.dans.managedeposit.core.service; -public class InvalidTransferItemException extends Exception { +public class InvalidRequestParameterException extends RuntimeException { - public InvalidTransferItemException(String msg, Throwable t) { + public InvalidRequestParameterException(String msg, Throwable t) { super(msg, t); } - public InvalidTransferItemException(String msg) { + public InvalidRequestParameterException(String msg) { this(msg, null); } } 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 60db574..0b847c1 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java +++ b/src/main/java/nl/knaw/dans/managedeposit/db/DepositPropertiesDAO.java @@ -17,6 +17,7 @@ import io.dropwizard.hibernate.AbstractDAO; import nl.knaw.dans.managedeposit.core.DepositProperties; +import nl.knaw.dans.managedeposit.core.service.InvalidRequestParameterException; import org.hibernate.SessionFactory; import org.hibernate.query.Query; import org.slf4j.Logger; @@ -86,8 +87,9 @@ public List findSelection(Map> queryPara public Optional deleteSelection(Map> queryParameters) { var criteriaBuilder = currentSession().getCriteriaBuilder(); - if (queryParameters.isEmpty()) // Note: all records will be deleted (accidentally) without any specified query parameter - return Optional.of(0); + if (queryParameters.isEmpty()) { + throw new InvalidRequestParameterException("Delete command without argument would make database empty. It is not accepted."); + } CriteriaDelete deleteQuery = criteriaBuilder.createCriteriaDelete(DepositProperties.class); Root root = deleteQuery.from(DepositProperties.class); @@ -101,11 +103,15 @@ public Optional deleteSelection(Map> queryParamete private Predicate buildQueryCriteria(Map> queryParameters, CriteriaBuilder criteriaBuilder, Root root) { List predicates = new ArrayList<>(); + boolean startDateSpecifiedAlready = false, endDateSpecifiedAlready = false; Predicate predicate; for (String key : queryParameters.keySet()) { List values = queryParameters.get(key); String parameter = key.toLowerCase(); + if (values.isEmpty()) { + throw new InvalidRequestParameterException(String.format("Empty value of parameter %s", parameter)); + } //javax.persistence.criteria Predicate orPredicateItem; List orPredicatesList = new ArrayList<>(); @@ -141,20 +147,29 @@ private Predicate buildQueryCriteria(Map> queryParameters, LocalDate date = LocalDate.parse(value, formatter); var asked_date = OffsetDateTime.of(date.atStartOfDay(), ZoneOffset.UTC); - if (parameter.equals("startdate")) + if (parameter.equals("startdate")) { + if (startDateSpecifiedAlready) + throw new InvalidRequestParameterException(String.format("Duplicated startdate parameter %s = %s ", parameter, value)); orPredicateItem = criteriaBuilder.greaterThan(root.get("depositCreationTimestamp"), asked_date); - else + startDateSpecifiedAlready = true; + } + else { + if (endDateSpecifiedAlready) + throw new InvalidRequestParameterException(String.format("Duplicated enddate parameter %s = %s ", parameter, value)); orPredicateItem = criteriaBuilder.lessThan(root.get("depositCreationTimestamp"), asked_date); + endDateSpecifiedAlready = true; + } } catch (DateTimeException e) { - log.warn("Error parsing the date: {}", e.getMessage()); - continue; + log.error("Invalid or incorrectly formatted date parameter {}", e.getMessage()); + String error = String.format("Invalid or incorrectly formatted parameter %s = %s ", parameter, value); + throw new InvalidRequestParameterException(String.format("Invalid or incorrectly formatted parameter %s = %s ", parameter, value)); } } break; default: - orPredicateItem = criteriaBuilder.equal(root.get(key), value); + throw new InvalidRequestParameterException(String.format("Unknown parameter %s = %s ", parameter, value)); } orPredicatesList.add(orPredicateItem); } 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..5f9f895 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesDeleteResource.java @@ -16,19 +16,27 @@ package nl.knaw.dans.managedeposit.resources; import io.dropwizard.hibernate.UnitOfWork; +import nl.knaw.dans.managedeposit.core.service.InvalidRequestParameterException; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.ws.rs.ClientErrorException; 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.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; +import java.util.Optional; @Path("/delete-deposit") public class DepositPropertiesDeleteResource { + private static final Logger log = LoggerFactory.getLogger(DepositPropertiesDeleteResource.class); private final DepositPropertiesDAO depositPropertiesDAO; public DepositPropertiesDeleteResource(DepositPropertiesDAO depositPropertiesDAO) { @@ -39,9 +47,29 @@ public DepositPropertiesDeleteResource(DepositPropertiesDAO depositPropertiesDAO @UnitOfWork @Produces("text/plain") @Consumes(MediaType.TEXT_PLAIN) - public String deleteDepositPropertiesUsingParams(@Context UriInfo uriInfo) { - int deletedNumber = depositPropertiesDAO.deleteSelection(uriInfo.getQueryParameters()).orElseThrow(() -> new NotFoundException("Not such deposit with given criteria")); - return String.format("Deleted number(s): %d.", deletedNumber); + public Response deleteDepositPropertiesUsingParams(@Context UriInfo uriInfo) { + try { + Optional deletedNumber = depositPropertiesDAO.deleteSelection(uriInfo.getQueryParameters()); + if (deletedNumber.isPresent()) { + log.debug("Deleted number(s): {}.", deletedNumber); + return Response.status(Response.Status.OK) + .encoding(String.format("Deleted number(s): %d.", deletedNumber.get())) + .build(); + } + else { + log.warn("Not such deposit with given criteria: {}", uriInfo); + throw new NotFoundException(String.format("No such deposit: %s", uriInfo)); + } + + } + catch (InvalidRequestParameterException e) { + log.error(e.getMessage()); + throw new ClientErrorException(e.getMessage(), Response.Status.BAD_REQUEST); + } + catch (Exception e) { + log.error(e.getMessage()); + throw new WebApplicationException(e.getMessage(), Response.Status.BAD_REQUEST); + } } } \ No newline at end of file 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..87615c6 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesReportResource.java @@ -17,20 +17,26 @@ import io.dropwizard.hibernate.UnitOfWork; import nl.knaw.dans.managedeposit.core.DepositProperties; +import nl.knaw.dans.managedeposit.core.service.InvalidRequestParameterException; import nl.knaw.dans.managedeposit.db.DepositPropertiesDAO; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.ws.rs.ClientErrorException; 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.WebApplicationException; import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; -import java.util.List; import java.util.Optional; @Path("/report") public class DepositPropertiesReportResource { + private static final Logger log = LoggerFactory.getLogger(DepositPropertiesReportResource.class); private final DepositPropertiesDAO depositPropertiesDAO; public DepositPropertiesReportResource(DepositPropertiesDAO depositPropertiesDAO) { @@ -40,16 +46,35 @@ public DepositPropertiesReportResource(DepositPropertiesDAO depositPropertiesDAO @GET @UnitOfWork @Produces({ "application/json", "text/csv" }) - public List listDepositProperties(@Context UriInfo uriInfo) { - return depositPropertiesDAO.findSelection(uriInfo.getQueryParameters()); + public Response listDepositProperties(@Context UriInfo uriInfo) { + try { + return Response.status(Response.Status.OK) + .entity(depositPropertiesDAO.findSelection(uriInfo.getQueryParameters())) + .build(); + } + catch (InvalidRequestParameterException e) { + log.error(e.getMessage()); + throw new ClientErrorException(e.getMessage(), Response.Status.BAD_REQUEST); + } + catch (Exception e) { + log.error(e.getMessage()); + throw new WebApplicationException(e.getMessage(), Response.Status.BAD_REQUEST); + } + } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @GET @UnitOfWork @Produces("application/json") @Path("/{depositId}") public DepositProperties getDepositId(@PathParam("depositId") Optional depositId) { - return depositPropertiesDAO.findById(depositId.get()).orElseThrow(() -> new NotFoundException(String.format("No such deposit: %s", depositId.orElse("")))); + if (depositId.isPresent()) + return depositPropertiesDAO.findById(depositId.get()).orElseThrow(() -> new NotFoundException(String.format("No such deposit: %s", depositId.orElse("")))); + else { + log.warn("Empty argument 'depositId'"); + throw new ClientErrorException("Empty argument 'depositId'", Response.Status.BAD_REQUEST); + } } } 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 343132c..699777f 100644 --- a/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java +++ b/src/main/java/nl/knaw/dans/managedeposit/resources/DepositPropertiesResource.java @@ -52,7 +52,8 @@ private String writeHelpInfoText() { - 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 + curl -i -X GET basePath/report?user=XXX&state=REJECTED\s + curl -i -X GET basePath/report/{depositId}\s curl -i -X POST basePath/delete-deposit?user=XXX\s curl -i -X POST basePath/delete-deposit?user=XXX&state=REJECTED"""; } 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 b7d2a39..4fe781e 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,18 +15,36 @@ */ 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.AbstractTestWithTestDir; import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import org.slf4j.LoggerFactory; 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; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; public class IngestPathMonitorTest extends AbstractTestWithTestDir { + private ListAppender listAppender; + + @BeforeEach + public void setup() throws Exception { + super.setUp(); + var logger = (Logger) LoggerFactory.getLogger(IngestPathMonitor.class); + listAppender = new ListAppender<>(); + listAppender.start(); + logger.setLevel(Level.ERROR); + logger.addAppender(listAppender); + } private IngestPathMonitor startMonitor(DepositStatusUpdater mockUpdater, int pollingInterval) throws Exception { var monitor = new IngestPathMonitor(singletonList(testDir), mockUpdater, pollingInterval); @@ -44,9 +62,14 @@ public void should_ignore_properties_in_root() throws Exception { 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()); + monitor.onFileCreate(propertiesFile.toFile()); Mockito.verifyNoMoreInteractions(mockUpdater); + Thread.sleep(70);// Wait for the monitor to pick up the new file + + // Check the logs + var formattedMessage = listAppender.list.get(0).getFormattedMessage(); + assertThat(formattedMessage).startsWith("Error: file"); + assertThat(formattedMessage).endsWith("is directly in a base-folder."); monitor.stop(); } @@ -83,7 +106,6 @@ public void should_ignore_properties_in_bag_content() throws Exception { monitor.stop(); } - @Test public void should_pick_up_deleted_bag() throws Exception { var mockUpdater = Mockito.mock(DepositStatusUpdater.class);