EVA-4025 check for pre-existing accessions before saving

accession-commons-core/src/main/java/uk/ac/ebi/ampt2d/commons/accession/core/AccessionSaveMode.java

@@ -0,0 +1,6 @@
+package uk.ac.ebi.ampt2d.commons.accession.core;
+
+public enum AccessionSaveMode {
+    SAVE_ALL_THEN_RESOLVE,
+    PREFILTER_EXISTING
+}

accession-commons-core/src/main/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicAccessioningService.java

@@ -59,13 +59,17 @@ public class BasicAccessioningService<MODEL, HASH, ACCESSION extends Serializabl
 
     private final Function<MODEL, HASH> hashingFunction;
 
+    private final AccessionSaveMode accessionSaveMode;
+
     public BasicAccessioningService(AccessionGenerator<MODEL, ACCESSION> accessionGenerator,
                                     DatabaseService<MODEL, HASH, ACCESSION> dbService,
                                     Function<MODEL, String> summaryFunction,
-                                    Function<String, HASH> hashingFunction) {
+                                    Function<String, HASH> hashingFunction,
+                                    AccessionSaveMode accessionSaveMode) {
         this.accessionGenerator = accessionGenerator;
         this.dbService = dbService;
         this.hashingFunction = summaryFunction.andThen(hashingFunction);
+        this.accessionSaveMode = accessionSaveMode != null ? accessionSaveMode : AccessionSaveMode.SAVE_ALL_THEN_RESOLVE;
     }
 
     @Override
@@ -89,7 +93,17 @@ private Map<HASH, MODEL> mapHashOfMessages(List<? extends MODEL> messages) {
      * application instance / thread with a different id.
      * See {@link #getPreexistingAccessions(List)} } for more details.
      */
-    private List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> saveAccessions(
+    private List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> saveAccessions(List<AccessionWrapper<MODEL, HASH, ACCESSION>> accessions) {
+        switch (this.accessionSaveMode) {
+            case PREFILTER_EXISTING:
+                return saveAccessionsPrefilteringExisting(accessions);
+            case SAVE_ALL_THEN_RESOLVE:
+            default:
+                return saveAllAccessionsThenResolve(accessions);
+        }
+    }
+
+    private List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> saveAllAccessionsThenResolve(
             List<AccessionWrapper<MODEL, HASH, ACCESSION>> accessions) {
         SaveResponse<ACCESSION> response = dbService.save(accessions);
         accessionGenerator.postSave(response);
@@ -110,6 +124,28 @@ private List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> saveAccessions
         return savedAccessions;
     }
 
+    private List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> saveAccessionsPrefilteringExisting(
+            List<AccessionWrapper<MODEL, HASH, ACCESSION>> accessions) {
+        Set<HASH> allHashes = accessions.stream().map(AccessionWrapper::getHash).collect(Collectors.toSet());
+        List<AccessionWrapper<MODEL, HASH, ACCESSION>> preexistingAccessions = dbService.findAllByHash(allHashes);
+        Set<HASH> preexistingHashes = preexistingAccessions.stream().map(AccessionWrapper::getHash).collect(Collectors.toSet());
+
+        List<AccessionWrapper<MODEL, HASH, ACCESSION>> accessionsToSave = accessions.stream()
+                .filter(accession -> !preexistingHashes.contains(accession.getHash()))
+                .collect(Collectors.toList());
+
+        final List<GetOrCreateAccessionWrapper<MODEL, HASH, ACCESSION>> result = new ArrayList<>();
+
+        if (!accessionsToSave.isEmpty()) {
+            result.addAll(saveAllAccessionsThenResolve(accessionsToSave));
+        }
+
+        // add pre-existing back to result
+        preexistingAccessions.stream().map(GetOrCreateAccessionWrapper::oldAccession).forEach(result::add);
+
+        return result;
+    }
+
     /**
      * We try to recover all elements that could not be saved to return their accession to the user. This is only
      * expected when another application instance or thread has saved that element already with a different id. If

accession-commons-jpa/src/test/java/uk/ac/ebi/ampt2d/test/configuration/CucumberTestConfiguration.java

@@ -28,6 +28,7 @@
 import org.springframework.context.annotation.Scope;
 import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
 import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
+import uk.ac.ebi.ampt2d.commons.accession.core.AccessionSaveMode;
 import uk.ac.ebi.ampt2d.commons.accession.core.AccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.BasicAccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.DatabaseService;
@@ -97,7 +98,8 @@ public AccessioningService<TestModel, String, String> accessioningService() {
                 ),
                 databaseService(),
                 TestModel::getValue,
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }
 

accession-commons-jpa/src/test/java/uk/ac/ebi/ampt2d/test/configuration/TestJpaDatabaseServiceTestConfiguration.java

@@ -27,6 +27,7 @@
 import org.springframework.context.annotation.Configuration;
 import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
 import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
+import uk.ac.ebi.ampt2d.commons.accession.core.AccessionSaveMode;
 import uk.ac.ebi.ampt2d.commons.accession.core.AccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.BasicAccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.generators.SingleAccessionGenerator;
@@ -95,7 +96,8 @@ public AccessioningService<TestModel, String, String> accessioningService() {
                 ),
                 databaseService(),
                 TestModel::getValue,
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }
 

accession-commons-mongodb/src/test/java/uk/ac/ebi/ampt2d/test/configuration/MongoDbCucumberTestConfiguration.java

@@ -25,6 +25,7 @@
 import org.springframework.context.annotation.Scope;
 import org.springframework.data.mongodb.config.EnableMongoAuditing;
 import org.springframework.data.mongodb.repository.config.EnableMongoRepositories;
+import uk.ac.ebi.ampt2d.commons.accession.core.AccessionSaveMode;
 import uk.ac.ebi.ampt2d.commons.accession.core.AccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.BasicAccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.DatabaseService;
@@ -86,7 +87,8 @@ public AccessioningService<TestModel, String, String> testMongoDbAccessioningSer
                         "id-service-" + o.getValue()),
                 testMongoDbService(),
                 TestModel::getValue,
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }
 

accession-commons-mongodb/src/test/java/uk/ac/ebi/ampt2d/test/configuration/MongoDbTestConfiguration.java

@@ -24,6 +24,7 @@
 import org.springframework.context.annotation.Configuration;
 import org.springframework.data.mongodb.config.EnableMongoAuditing;
 import org.springframework.data.mongodb.repository.config.EnableMongoRepositories;
+import uk.ac.ebi.ampt2d.commons.accession.core.AccessionSaveMode;
 import uk.ac.ebi.ampt2d.commons.accession.core.AccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.BasicAccessioningService;
 import uk.ac.ebi.ampt2d.commons.accession.core.DatabaseService;
@@ -83,7 +84,8 @@ public AccessioningService<TestModel, String, String> testMongoDbAccessioningSer
                 new SingleAccessionGenerator<>(o -> "id-" + o.getValue()),
                 testMongoDbService(),
                 testModel -> testModel.getValue(),
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }
 

accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningTest.java

@@ -75,7 +75,8 @@ private AccessioningService<TestModel, String, Long> getAccessioningService() {
                 monotonicAccessionGenerator,
                 databaseService,
                 TestModel::getValue,
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }
 

accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningTestWithPreFiltering.java

@@ -0,0 +1,181 @@
+/*
+ *
+ * Copyright 2018 EMBL - European Bioinformatics Institute
+ *
+ * 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 uk.ac.ebi.ampt2d.commons.accession.core;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.test.context.transaction.TestTransaction;
+import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionCouldNotBeGeneratedException;
+import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionDeprecatedException;
+import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionDoesNotExistException;
+import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionMergedException;
+import uk.ac.ebi.ampt2d.commons.accession.core.models.AccessionWrapper;
+import uk.ac.ebi.ampt2d.commons.accession.core.models.GetOrCreateAccessionWrapper;
+import uk.ac.ebi.ampt2d.commons.accession.generators.monotonic.MonotonicAccessionGenerator;
+import uk.ac.ebi.ampt2d.commons.accession.hashing.SHA1HashingFunction;
+import uk.ac.ebi.ampt2d.test.configuration.TestMonotonicDatabaseServiceTestConfiguration;
+import uk.ac.ebi.ampt2d.test.models.TestModel;
+import uk.ac.ebi.ampt2d.test.persistence.TestMonotonicRepository;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(SpringRunner.class)
+@DataJpaTest
+@ContextConfiguration(classes = {TestMonotonicDatabaseServiceTestConfiguration.class})
+public class BasicMonotonicAccessioningTestWithPreFiltering {
+
+    private static String APPLICATION_INSTANCE_ID = "TEST_APPPLICATION_INSTANCE_ID";
+
+    @Autowired
+    private TestMonotonicRepository repository;
+
+    @Autowired
+    private DatabaseService<TestModel, String, Long> databaseService;
+
+    @Autowired
+    private MonotonicAccessionGenerator<TestModel> monotonicAccessionGenerator;
+
+    @Test
+    public void testCreateAccessions() throws AccessionCouldNotBeGeneratedException {
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-1"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(3, accessions.size());
+    }
+
+    private AccessioningService<TestModel, String, Long> getAccessioningService() {
+        return new BasicAccessioningService<>(
+                monotonicAccessionGenerator,
+                databaseService,
+                TestModel::getValue,
+                new SHA1HashingFunction(),
+                AccessionSaveMode.PREFILTER_EXISTING
+        );
+    }
+
+    @Test
+    public void testGetOrCreateFiltersRepeated() throws AccessionCouldNotBeGeneratedException {
+
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-1"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(3, accessions.size());
+    }
+
+    @Test
+    public void testGetAccessions() throws AccessionCouldNotBeGeneratedException {
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+
+        List<AccessionWrapper<TestModel, String, Long>> accessions = accessioningService.get(
+                Arrays.asList(
+                        TestModel.of("service-test-1"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-3")
+                ));
+        assertEquals(0, accessions.size());
+    }
+
+    @Test
+    public void testGetWithExistingEntries() throws AccessionCouldNotBeGeneratedException {
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions1 = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(1, accessions1.size());
+
+        List<AccessionWrapper<TestModel, String, Long>> accessions2 = accessioningService.get(
+                Arrays.asList(
+                        TestModel.of("service-test-1"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-3")
+                ));
+        assertEquals(1, accessions2.size());
+        assertEquals("service-test-3", accessions2.get(0).getData().getValue());
+    }
+
+    @Test
+    public void testGetByAccessionsWithExistingEntries() throws AccessionCouldNotBeGeneratedException,
+            AccessionDoesNotExistException, AccessionMergedException, AccessionDeprecatedException {
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions1 = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(1, accessions1.size());
+
+        AccessionWrapper<TestModel, String, Long> accession2 =
+                accessioningService.getByAccession(accessions1.get(0).getAccession());
+        assertEquals(accessions1.get(0).getAccession(), accession2.getAccession());
+    }
+
+    @Test
+    public void testGetOrCreateWithExistingEntries() throws AccessionCouldNotBeGeneratedException {
+        AccessioningService<TestModel, String, Long> accessioningService = getAccessioningService();
+        TestTransaction.flagForCommit();
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions1 = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(1, accessions1.size());
+        assertEquals(true, accessions1.get(0).isNewAccession());
+        TestTransaction.end();
+
+        List<GetOrCreateAccessionWrapper<TestModel, String, Long>> accessions2 = accessioningService.getOrCreate(
+                Arrays.asList(
+                        TestModel.of("service-test-1"),
+                        TestModel.of("service-test-2"),
+                        TestModel.of("service-test-3")
+                ), APPLICATION_INSTANCE_ID);
+        assertEquals(3, accessions2.size());
+        accessions2.stream().forEach(wrapper -> {
+            if (!wrapper.isNewAccession()) {
+                assertEquals("service-test-3",wrapper.getData().getValue());
+            }
+        });
+        assertEquals(2,accessions2.stream().filter(GetOrCreateAccessionWrapper::isNewAccession).count());
+
+        TestTransaction.start();
+        for (AccessionWrapper<TestModel, String, Long> accession : accessions2) {
+            repository.deleteById(accession.getHash());
+        }
+        TestTransaction.flagForCommit();
+        TestTransaction.end();
+    }
+
+}

accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java

@@ -226,7 +226,8 @@ private BasicAccessioningService<TestModel, String, Long> getAccessioningService
                 getGenerator(categoryId, instanceId),
                 databaseService,
                 TestModel::getValue,
-                new SHA1HashingFunction()
+                new SHA1HashingFunction(),
+                AccessionSaveMode.SAVE_ALL_THEN_RESOLVE
         );
     }