From 210aedbd5bf258d4e8cafbf180112832c745e399 Mon Sep 17 00:00:00 2001 From: Russ Poetker Date: Mon, 19 Feb 2024 08:45:05 -0500 Subject: [PATCH 1/2] Check for deposit status processor for status update --- .../deposit/service/DepositProcessor.java | 2 +- .../deposit/service/DepositTaskHelper.java | 118 +++++++------- .../pass/deposit/service/DepositUpdater.java | 2 +- .../deposit/service/DepositProcessorIT.java | 116 +++++++++++++ .../deposit/service/DepositProcessorTest.java | 2 +- .../service/DepositTaskHelperTest.java | 154 ++---------------- .../deposit/service/DepositUpdaterIT.java | 9 +- .../deposit/service/DepositUpdaterTest.java | 4 +- .../resources/full-test-repositories.json | 5 + 9 files changed, 207 insertions(+), 205 deletions(-) create mode 100644 pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java diff --git a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositProcessor.java b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositProcessor.java index 78c27ac2..a25ee9f5 100644 --- a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositProcessor.java +++ b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositProcessor.java @@ -70,7 +70,7 @@ public void accept(Deposit deposit) { // if result is still intermediate, add Deposit to queue for processing? // Determine the logical success or failure of the Deposit, and persist the Deposit and RepositoryCopy - depositHelper.processDepositStatus(deposit.getId()); + depositHelper.processDepositStatus(deposit); } } diff --git a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositTaskHelper.java b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositTaskHelper.java index 57ab3799..054cb9ea 100644 --- a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositTaskHelper.java +++ b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositTaskHelper.java @@ -20,6 +20,7 @@ import static java.lang.System.identityHashCode; import static org.eclipse.deposit.util.loggers.Loggers.WORKERS_LOGGER; +import java.io.IOException; import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; @@ -27,6 +28,7 @@ import java.util.function.Function; import java.util.function.Predicate; +import org.apache.commons.lang3.StringUtils; import org.eclipse.pass.deposit.DepositServiceErrorHandler; import org.eclipse.pass.deposit.DepositServiceRuntimeException; import org.eclipse.pass.deposit.RemedialDepositException; @@ -69,9 +71,6 @@ public class DepositTaskHelper { "Missing Packager for Repository named '{}', marking Deposit as " + "FAILED."; private static final String PRECONDITION_FAILED = "Refusing to update {}, the following pre-condition failed: "; - private static final String ERR_RESOLVE_REPOSITORY = "Unable to resolve Repository Configuration for Repository " + - "%s (%s). Verify the Deposit Services runtime configuration" + - " location and " + "content."; private static final String ERR_PARSING_STATUS_DOC = "Failed to update deposit status for [%s], parsing the " + "status document referenced by %s failed: %s"; private static final String ERR_MAPPING_STATUS = "Failed to update deposit status for [%s], mapping the status " + @@ -146,20 +145,69 @@ public void submitDeposit(Submission submission, DepositSubmission depositSubmis } } - void processDepositStatus(String depositId) { + void processDepositStatus(Deposit deposit) { + try { + Repository repository = passClient.getObject(deposit.getRepository()); + RepositoryConfig repositoryConfig = repositories.getConfig(repository.getRepositoryKey()); + getDepositStatusProcessor(repositoryConfig) + .ifPresentOrElse( + depositStatusProcessor -> + updateDepositStatusIfNeeded(deposit, repositoryConfig, depositStatusProcessor), + () -> LOG.info("No deposit status processor found for Deposit {}, status not updated", + deposit.getId()) + ); + } catch (IOException e) { + LOG.error("Failed to update Deposit Status {}", deposit.getId(), e); + } + } + + void updateDepositRepositoryCopyStatus(Deposit deposit) { + try { + RepositoryCopy repoCopy = passClient.getObject(deposit.getRepositoryCopy()); + switch (deposit.getDepositStatus()) { + case ACCEPTED -> { + LOG.debug("Deposit {} was accepted.", deposit.getId()); + repoCopy.setCopyStatus(CopyStatus.COMPLETE); + passClient.updateObject(repoCopy); + } + case REJECTED -> { + LOG.debug("Deposit {} was rejected.", deposit.getId()); + repoCopy.setCopyStatus(CopyStatus.REJECTED); + passClient.updateObject(repoCopy); + } + default -> { + } + } + } catch (Exception e) { + String msg = String.format(ERR_UPDATE_REPOCOPY, deposit.getRepositoryCopy(), deposit.getId()); + throw new DepositServiceRuntimeException(msg, e, deposit); + } + } + + private Optional getDepositStatusProcessor(RepositoryConfig repositoryConfig) { + if (Objects.isNull(repositoryConfig) + || Objects.isNull(repositoryConfig.getRepositoryDepositConfig()) + || Objects.isNull(repositoryConfig.getRepositoryDepositConfig().getDepositProcessing()) + || Objects.isNull(repositoryConfig.getRepositoryDepositConfig().getDepositProcessing().getProcessor())) { + return Optional.empty(); + } + return Optional.of(repositoryConfig.getRepositoryDepositConfig().getDepositProcessing().getProcessor()); + } + private void updateDepositStatusIfNeeded(Deposit deposit, RepositoryConfig repositoryConfig, + DepositStatusProcessor depositStatusProcessor) { CriticalResult cr = cri.performCritical( - depositId, Deposit.class, + deposit.getId(), Deposit.class, DepositStatusCriFunc.precondition(), DepositStatusCriFunc.postcondition(), - DepositStatusCriFunc.critical(repositories, passClient) + DepositStatusCriFunc.critical(repositoryConfig, depositStatusProcessor) ); if (!cr.success()) { if (cr.throwable().isPresent()) { Throwable t = cr.throwable().get(); if (t instanceof RemedialDepositException) { - LOG.error(format("Failed to update Deposit %s", depositId), t); + LOG.error(format("Failed to update Deposit %s", deposit.getId()), t); return; } @@ -169,49 +217,18 @@ void processDepositStatus(String depositId) { if (cr.resource().isPresent()) { throw new DepositServiceRuntimeException( - format("Failed to update Deposit %s: %s", depositId, t.getMessage()), + format("Failed to update Deposit %s: %s", deposit.getId(), t.getMessage()), t, cr.resource().get()); } } LOG.debug(format("Failed to update Deposit %s: no cause was present, probably a pre- or post-condition " + - "was not satisfied.", depositId)); + "was not satisfied.", deposit.getId())); return; } cr.result().ifPresent(this::updateDepositRepositoryCopyStatus); - LOG.info("Successfully processed Deposit {}", depositId); - } - - RepositoryCopy updateDepositRepositoryCopyStatus(Deposit deposit) { - try { - RepositoryCopy repoCopy = passClient.getObject(deposit.getRepositoryCopy()); - switch (deposit.getDepositStatus()) { - case ACCEPTED -> { - LOG.debug("Deposit {} was accepted.", deposit.getId()); - repoCopy.setCopyStatus(CopyStatus.COMPLETE); - passClient.updateObject(repoCopy); - } - case REJECTED -> { - LOG.debug("Deposit {} was rejected.", deposit.getId()); - repoCopy.setCopyStatus(CopyStatus.REJECTED); - passClient.updateObject(repoCopy); - } - default -> { - } - } - return repoCopy; - } catch (Exception e) { - String msg = String.format(ERR_UPDATE_REPOCOPY, deposit.getRepositoryCopy(), deposit.getId()); - throw new DepositServiceRuntimeException(msg, e, deposit); - } - } - - static Optional lookupConfig(Repository passRepository, Repositories repositories) { - if (passRepository.getRepositoryKey() != null) { - return Optional.of(repositories.getConfig(passRepository.getRepositoryKey())); - } - return Optional.empty(); + LOG.info("Successfully processed Deposit {}", deposit.getId()); } static class DepositStatusCriFunc { @@ -233,7 +250,7 @@ static Predicate precondition() { return false; } - if (deposit.getDepositStatusRef() == null || deposit.getDepositStatusRef().trim().length() == 0) { + if (deposit.getDepositStatusRef() == null || StringUtils.isBlank(deposit.getDepositStatusRef())) { LOG.debug(PRECONDITION_FAILED + " missing Deposit status reference.", deposit.getId()); return false; } @@ -258,23 +275,12 @@ static BiPredicate postcondition() { return (deposit, updatedDeposit) -> Objects.nonNull(updatedDeposit.getRepositoryCopy()); } - static Function critical(Repositories repositories, PassClient passClient) { + static Function critical(RepositoryConfig repositoryConfig, + DepositStatusProcessor depositStatusProcessor) { return (deposit) -> { AtomicReference status = new AtomicReference<>(); try { - - Repository repo = passClient.getObject(deposit.getRepository()); - - RepositoryConfig repoConfig = lookupConfig(repo, repositories) - .orElseThrow(() -> - new RemedialDepositException( - format(ERR_RESOLVE_REPOSITORY, repo.getName(), repo.getId()), repo)); - DepositStatusProcessor statusProcessor = repoConfig.getRepositoryDepositConfig() - .getDepositProcessing() - .getProcessor(); - status.set(statusProcessor.process(deposit, repoConfig)); - } catch (RemedialDepositException e) { - throw e; + status.set(depositStatusProcessor.process(deposit, repositoryConfig)); } catch (Exception e) { String msg = format(ERR_PARSING_STATUS_DOC, deposit.getId(), deposit.getDepositStatusRef(), e.getMessage()); diff --git a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositUpdater.java b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositUpdater.java index 112c9b9e..344d9c86 100644 --- a/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositUpdater.java +++ b/pass-deposit-services/deposit-core/src/main/java/org/eclipse/pass/deposit/service/DepositUpdater.java @@ -105,7 +105,7 @@ private void updateSubmittedDeposits(ZonedDateTime submissionFromDate) throws IO submittedDeposits.forEach(deposit -> { try { LOG.info("Updating Deposit.depositStatus for {}", deposit.getId()); - depositHelper.processDepositStatus(deposit.getId()); + depositHelper.processDepositStatus(deposit); } catch (Exception e) { LOG.warn("Failed to update Deposit {}: {}", deposit.getId(), e.getMessage(), e); } diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java new file mode 100644 index 00000000..542051cd --- /dev/null +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java @@ -0,0 +1,116 @@ +/* + * Copyright 2024 Johns Hopkins University + * + * 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 org.eclipse.pass.deposit.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.time.ZonedDateTime; + +import org.eclipse.pass.deposit.util.ResourceTestUtil; +import org.eclipse.pass.support.client.model.CopyStatus; +import org.eclipse.pass.support.client.model.Deposit; +import org.eclipse.pass.support.client.model.DepositStatus; +import org.eclipse.pass.support.client.model.Repository; +import org.eclipse.pass.support.client.model.RepositoryCopy; +import org.eclipse.pass.support.client.model.Submission; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.TestPropertySource; + +/** + * @author Russ Poetker (rpoetke1@jh.edu) + */ +@TestPropertySource(properties = { + "pass.deposit.repository.configuration=classpath:/full-test-repositories.json" +}) +public class DepositProcessorIT extends AbstractDepositIT { + + @Autowired private DepositProcessor depositProcessor; + + @Test + public void testDepositProcessor_WithDepositProcessor() throws Exception { + // GIVEN + Deposit j10pDeposit = initJScholarshipDeposit(); + mockSword(); + + // WHEN + depositProcessor.accept(j10pDeposit); + + // THEN + Deposit actualDeposit = passClient.getObject(j10pDeposit); + assertEquals(DepositStatus.ACCEPTED, actualDeposit.getDepositStatus()); + assertEquals("test-j10p-ref", actualDeposit.getDepositStatusRef()); + verify(passClient).updateObject(eq(actualDeposit)); + } + + @Test + public void testDepositProcessor_NoUpdateOnDepositNoDepositProcessor() throws Exception { + // GIVEN + Deposit pmcDeposit = initPmcDeposit(); + + // WHEN + depositProcessor.accept(pmcDeposit); + + // THEN + Deposit actualDeposit = passClient.getObject(pmcDeposit); + assertEquals(DepositStatus.SUBMITTED, actualDeposit.getDepositStatus()); + assertEquals("test-pmc-package", actualDeposit.getDepositStatusRef()); + verify(passClient, times(0)).updateObject(eq(actualDeposit)); + } + + private Deposit initPmcDeposit() throws Exception { + Submission submission = findSubmission(createSubmission( + ResourceTestUtil.readSubmissionJson("sample1"))); + submission.setSubmittedDate(ZonedDateTime.now()); + passClient.updateObject(submission); + Repository pmcRepo = submission.getRepositories().stream() + .filter(repository -> repository.getRepositoryKey().equals("PubMed Central")) + .findFirst().get(); + Deposit pmcDeposit = new Deposit(); + pmcDeposit.setSubmission(submission); + pmcDeposit.setRepository(pmcRepo); + pmcDeposit.setDepositStatus(DepositStatus.SUBMITTED); + pmcDeposit.setDepositStatusRef("test-pmc-package"); + passClient.createObject(pmcDeposit); + return pmcDeposit; + } + + private Deposit initJScholarshipDeposit() throws Exception { + Submission submission = findSubmission(createSubmission( + ResourceTestUtil.readSubmissionJson("sample1"))); + submission.setSubmittedDate(ZonedDateTime.now()); + passClient.updateObject(submission); + Repository j10pRepo = submission.getRepositories().stream() + .filter(repository -> repository.getRepositoryKey().equals("JScholarship")) + .findFirst().get(); + RepositoryCopy repositoryCopy = new RepositoryCopy(); + repositoryCopy.setRepository(j10pRepo); + repositoryCopy.setCopyStatus(CopyStatus.IN_PROGRESS); + passClient.createObject(repositoryCopy); + Deposit j10pDeposit = new Deposit(); + j10pDeposit.setSubmission(submission); + j10pDeposit.setRepository(j10pRepo); + j10pDeposit.setRepositoryCopy(repositoryCopy); + j10pDeposit.setDepositStatus(DepositStatus.SUBMITTED); + j10pDeposit.setDepositStatusRef("test-j10p-ref"); + passClient.createObject(j10pDeposit); + return j10pDeposit; + } + +} diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorTest.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorTest.java index d34672a4..35ae57ab 100644 --- a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorTest.java +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorTest.java @@ -183,7 +183,7 @@ public void acceptDepositWithIntermediateStatus() { verify(intermediateDeposit).getDepositStatus(); verifyNoInteractions(cri); - verify(depositTaskHelper).processDepositStatus(depositId); + verify(depositTaskHelper).processDepositStatus(intermediateDeposit); } private void prepareCriFuncCriticalSuccess(DepositStatus depositStatus) throws IOException { diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositTaskHelperTest.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositTaskHelperTest.java index 25a9d988..369efe66 100644 --- a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositTaskHelperTest.java +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositTaskHelperTest.java @@ -35,7 +35,6 @@ import java.io.IOException; import org.eclipse.pass.deposit.DepositServiceRuntimeException; -import org.eclipse.pass.deposit.RemedialDepositException; import org.eclipse.pass.deposit.config.repository.DepositProcessing; import org.eclipse.pass.deposit.config.repository.Repositories; import org.eclipse.pass.deposit.config.repository.RepositoryConfig; @@ -50,33 +49,17 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -// TODO this test needs to be redone. It doesn't look to actually test DepositTaskHelper /** * @author Elliot Metsger (emetsger@jhu.edu) */ public class DepositTaskHelperTest { private PassClient passClient; - private Repository repository; private Deposit deposit; - private Repositories repositories; @BeforeEach public void setUp() throws Exception { passClient = mock(PassClient.class); - repositories = mock(Repositories.class); deposit = mock(Deposit.class); - repository = mock(Repository.class); - } - - @Test - public void lookupRepositoryConfigByKey() { - String key = "repoKey"; - Repository repo = newRepositoryWithKey(key); - Repositories repositories = newRepositoriesWithConfigFor(key); - - DepositTaskHelper.lookupConfig(repo, repositories) - .orElseThrow( - () -> new RuntimeException("Missing expected repository config for key '" + key + "'")); } /** @@ -90,7 +73,7 @@ public void lookupRepositoryConfigByKey() { * @throws IOException */ @Test - public void depositCriFuncPreconditionSuccess() throws IOException { + public void depositCriFuncPreconditionSuccess() { String repoKey = randomId(); String repoCopyId = randomId(); @@ -208,7 +191,7 @@ public void depositCriFuncPreconditionFailNullRepoCopy() { String repoKey = randomId(); when(deposit.getDepositStatus()).thenReturn( randomDepositStatusExcept(DepositStatus.ACCEPTED, DepositStatus.REJECTED)); - when(deposit.getDepositStatusRef()).thenReturn(statusRef.toString()); + when(deposit.getDepositStatusRef()).thenReturn(statusRef); when(deposit.getRepository()).thenReturn(new Repository(repoKey)); assertFalse(DepositStatusCriFunc.precondition().test(deposit)); @@ -253,7 +236,7 @@ public void depositCriFuncPostconditionFailNullRepoCopy() { } @Test - public void depositCriFuncCriticalSuccessAccepted() throws IOException { + public void depositCriFuncCriticalSuccessAccepted() { Deposit testDeposit = new Deposit(); testDepositCriFuncCriticalForStatus(DepositStatus.ACCEPTED, testDeposit, passClient); } @@ -270,84 +253,19 @@ public void depositCriFuncCriticalSuccessRejected() throws IOException { * @throws IOException */ @Test - public void depositCriFuncCriticalSuccessIntermediate() throws IOException { + public void depositCriFuncCriticalSuccessIntermediate() { DepositStatus statusProcessorResult = randomDepositStatusExcept(DepositStatus.ACCEPTED, DepositStatus.REJECTED); String repoKey = randomId(); DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); + newRepositoryWithKey(repoKey); Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - RepositoryCopy repoCopy = mock(RepositoryCopy.class); - - when(deposit.getRepository()).thenReturn(repo); - when(deposit.getRepositoryCopy()).thenReturn(repoCopy); - - when(passClient.getObject(repo)).thenReturn(repo); - when(passClient.getObject(repoCopy)).thenReturn(repoCopy); - + RepositoryConfig repositoryConfig = repos.getConfig(repoKey); when(statusProcessor.process(eq(deposit), any())).thenReturn(statusProcessorResult); - assertSame(deposit, DepositStatusCriFunc.critical(repos, passClient).apply(deposit)); + assertSame(deposit, DepositStatusCriFunc.critical(repositoryConfig, statusProcessor).apply(deposit)); - verify(passClient).getObject(repo); - verifyNoMoreInteractions(passClient); verify(statusProcessor).process(eq(deposit), any()); - verifyNoInteractions(repoCopy); - } - - /** - * When there is an error looking up the RepositoryConfig insure there is a proper error message - * @throws IOException - */ - @Test - public void depositCriFuncCriticalMissingRepositoryConfig() throws IOException { - when(deposit.getRepository()).thenReturn(repository); - when(passClient.getObject(repository)).thenReturn(repository); - - Exception e = assertThrows(RemedialDepositException.class, () -> { - DepositStatusCriFunc.critical(repositories, passClient).apply(deposit); - }); - - assertTrue(e.getMessage().contains("Unable to resolve Repository Configuration for Repository")); - - verify(passClient).getObject(repository); - verifyNoMoreInteractions(passClient); - } - - /** - * When there is an error resolving the DepositStatusProcessor, insure there is a proper error message - * @throws IOException - */ - @Test - public void depositCriFuncCriticalNullDepositConfig() throws IOException { - String repoKey = randomId(); - DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); - Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - - when(deposit.getRepository()).thenReturn(repo); - when(passClient.getObject(repo)).thenReturn(repo); - repos.getConfig(repoKey).setRepositoryDepositConfig(null); - - verifyNullObjectInDepositStatusProcessorLookup(repo, repos); - } - - /** - * When there is an error resolving the DepositStatusProcessor, insure there is a proper error message - * @throws IOException - */ - @Test - public void depositCriFuncCriticalNullDepositProcessing() throws IOException { - String repoKey = randomId(); - DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); - Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - - when(deposit.getRepository()).thenReturn(repo); - when(passClient.getObject(Repository.class, repoKey)).thenReturn(repo); - repos.getConfig(repoKey).getRepositoryDepositConfig().setDepositProcessing(null); - - verifyNullObjectInDepositStatusProcessorLookup(repo, repos); } /** @@ -355,55 +273,20 @@ public void depositCriFuncCriticalNullDepositProcessing() throws IOException { * @throws IOException */ @Test - public void depositCriFuncCriticalNullDepositStatusProcessor() throws IOException { + public void depositCriFuncCriticalDepositStatusProcessorProducesNullStatus() { String repoKey = randomId(); DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); + newRepositoryWithKey(repoKey); Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - - when(deposit.getRepository()).thenReturn(repo); - when(passClient.getObject(Repository.class, repoKey)).thenReturn(repo); - repos.getConfig(repoKey).getRepositoryDepositConfig().getDepositProcessing().setProcessor(null); - - verifyNullObjectInDepositStatusProcessorLookup(repo, repos); - } - - /** - * When there is an error resolving the DepositStatusProcessor, insure there is a proper error message - * @throws IOException - */ - @Test - public void depositCriFuncCriticalDepositStatusProcessorProducesNullStatus() throws IOException { - String repoKey = randomId(); - DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); - Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - - when(deposit.getRepository()).thenReturn(repo); - when(passClient.getObject(repo)).thenReturn(repo); + RepositoryConfig repositoryConfig = repos.getConfig(repoKey); when(statusProcessor.process(deposit, repos.getConfig(repoKey))).thenReturn(null); Exception e = assertThrows(DepositServiceRuntimeException.class, () -> { - DepositStatusCriFunc.critical(repos, passClient).apply(deposit); + DepositStatusCriFunc.critical(repositoryConfig, statusProcessor).apply(deposit); }); assertTrue(e.getMessage().contains("Failed to update deposit status")); - verify(deposit).getRepository(); - verify(passClient).getObject(repo); - verifyNoMoreInteractions(passClient); - } - - private void verifyNullObjectInDepositStatusProcessorLookup(Repository repository, Repositories repos) - throws IOException { - - Exception e = assertThrows(DepositServiceRuntimeException.class, () -> { - DepositStatusCriFunc.critical(repos, passClient).apply(deposit); - }); - - assertTrue(e.getMessage().contains("parsing the status document referenced by")); - - verify(passClient).getObject(repository); verifyNoMoreInteractions(passClient); } @@ -437,22 +320,15 @@ private static Repositories newRepositoriesWithConfigFor(String key, DepositStat private static void testDepositCriFuncCriticalForStatus(DepositStatus statusProcessorResult, Deposit deposit, - PassClient passClient) throws IOException { + PassClient passClient) { String repoKey = randomId(); DepositStatusProcessor statusProcessor = mock(DepositStatusProcessor.class); - Repository repo = newRepositoryWithKey(repoKey); + newRepositoryWithKey(repoKey); Repositories repos = newRepositoriesWithConfigFor(repoKey, statusProcessor); - RepositoryCopy repoCopy = new RepositoryCopy(); // concrete to capture state changes performed by critical - - deposit.setRepository(repo); - deposit.setRepositoryCopy(repoCopy); - - when(passClient.getObject(repo)).thenReturn(repo); - when(passClient.getObject(repoCopy)).thenReturn(repoCopy); - + RepositoryConfig repositoryConfig = repos.getConfig(repoKey); when(statusProcessor.process(eq(deposit), any())).thenReturn(statusProcessorResult); - Deposit result = DepositStatusCriFunc.critical(repos, passClient).apply(deposit); + Deposit result = DepositStatusCriFunc.critical(repositoryConfig, statusProcessor).apply(deposit); assertEquals(statusProcessorResult, result.getDepositStatus()); verify(statusProcessor).process(eq(deposit), any()); diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterIT.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterIT.java index 3d65a8b7..8360ec8b 100644 --- a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterIT.java +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterIT.java @@ -62,16 +62,15 @@ public void testDoUpdate_SkipNoDepositStatusProcessorRepos() throws Exception { // THEN PassClientSelector depositSelector = new PassClientSelector<>(Deposit.class); depositSelector.setFilter(RSQL.equals("submission.id", submission.getId())); - depositSelector.setInclude("repository"); PassClientResult actualDeposits = passClient.selectObjects(depositSelector); Deposit pmcDeposit = actualDeposits.getObjects().stream() - .filter(deposit -> deposit.getRepository().getRepositoryKey().equals("PubMed Central")) + .filter(deposit -> deposit.getDepositStatusRef().equals("test-pmc-package")) .findFirst().get(); Deposit j10pDeposit = actualDeposits.getObjects().stream() - .filter(deposit -> deposit.getRepository().getRepositoryKey().equals("JScholarship")) + .filter(deposit -> deposit.getDepositStatusRef().equals("test-j10p-ref")) .findFirst().get(); - verify(depositTaskHelper, times(0)).processDepositStatus(eq(pmcDeposit.getId())); - verify(depositTaskHelper, times(1)).processDepositStatus(eq(j10pDeposit.getId())); + verify(depositTaskHelper, times(0)).processDepositStatus(eq(pmcDeposit)); + verify(depositTaskHelper, times(1)).processDepositStatus(eq(j10pDeposit)); } private Submission initSubmissionAndDeposits() throws Exception { diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterTest.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterTest.java index 4b507f1f..372290b9 100644 --- a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterTest.java +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositUpdaterTest.java @@ -77,8 +77,8 @@ void testDoUpdate() throws IOException { depositUpdater.doUpdate(); // THEN - verify(depositTaskHelper).processDepositStatus("dp-1"); - verify(depositTaskHelper).processDepositStatus("dp-2"); + verify(depositTaskHelper).processDepositStatus(deposit1); + verify(depositTaskHelper).processDepositStatus(deposit2); ArgumentCaptor> argument = ArgumentCaptor.forClass(PassClientSelector.class); verify(passClient, times(2)).streamObjects(argument.capture()); diff --git a/pass-deposit-services/deposit-core/src/test/resources/full-test-repositories.json b/pass-deposit-services/deposit-core/src/test/resources/full-test-repositories.json index e18f77dc..687cb367 100644 --- a/pass-deposit-services/deposit-core/src/test/resources/full-test-repositories.json +++ b/pass-deposit-services/deposit-core/src/test/resources/full-test-repositories.json @@ -3,6 +3,11 @@ "deposit-config": { "processing": { "beanName": "org.eclipse.pass.deposit.status.DefaultDepositStatusProcessor" + }, + "mapping": { + "http://dspace.org/state/archived": "accepted", + "http://dspace.org/state/withdrawn": "rejected", + "default-mapping": "submitted" } }, "assembler": { From 2f8180fece7b051ae3460c9b883c31b23b79e787 Mon Sep 17 00:00:00 2001 From: Russ Poetker Date: Mon, 19 Feb 2024 09:48:13 -0500 Subject: [PATCH 2/2] Add test --- .../deposit/service/DepositProcessorIT.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java index 542051cd..8b59417f 100644 --- a/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java +++ b/pass-deposit-services/deposit-core/src/test/java/org/eclipse/pass/deposit/service/DepositProcessorIT.java @@ -16,12 +16,17 @@ package org.eclipse.pass.deposit.service; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import java.time.ZonedDateTime; +import org.eclipse.pass.deposit.DepositServiceRuntimeException; +import org.eclipse.pass.deposit.status.DefaultDepositStatusProcessor; import org.eclipse.pass.deposit.util.ResourceTestUtil; import org.eclipse.pass.support.client.model.CopyStatus; import org.eclipse.pass.support.client.model.Deposit; @@ -31,6 +36,7 @@ import org.eclipse.pass.support.client.model.Submission; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.test.context.TestPropertySource; /** @@ -42,6 +48,7 @@ public class DepositProcessorIT extends AbstractDepositIT { @Autowired private DepositProcessor depositProcessor; + @SpyBean private DefaultDepositStatusProcessor defaultDepositStatusProcessor; @Test public void testDepositProcessor_WithDepositProcessor() throws Exception { @@ -59,6 +66,27 @@ public void testDepositProcessor_WithDepositProcessor() throws Exception { verify(passClient).updateObject(eq(actualDeposit)); } + @Test + public void testDepositProcessor_WithDepositProcessorThrowsException() throws Exception { + // GIVEN + Deposit j10pDeposit = initJScholarshipDeposit(); + doThrow(new RuntimeException("Testing deposit status error")) + .when(defaultDepositStatusProcessor).process(any(Deposit.class), any()); + + // WHEN + DepositServiceRuntimeException exception = + assertThrows(DepositServiceRuntimeException.class, () -> depositProcessor.accept(j10pDeposit)); + + // THEN + assertEquals("Failed to update deposit status for [" + j10pDeposit.getId() + + "], parsing the status document referenced by test-j10p-ref failed: Testing deposit status error", + exception.getMessage()); + Deposit actualDeposit = passClient.getObject(j10pDeposit); + assertEquals(DepositStatus.SUBMITTED, actualDeposit.getDepositStatus()); + assertEquals("test-j10p-ref", actualDeposit.getDepositStatusRef()); + verify(passClient, times(0)).updateObject(eq(actualDeposit)); + } + @Test public void testDepositProcessor_NoUpdateOnDepositNoDepositProcessor() throws Exception { // GIVEN