#255 disallow to merge or convert idigbio entities + only admins can …

…use these endpoints
gbif · Dec 15, 2020 · 710f9ae · 710f9ae
1 parent 2619e4e
commit 710f9ae
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 10 deletions.
diff --git a/...ion-tests/src/test/java/org/gbif/registry/ws/it/collections/merge/BaseMergeServiceIT.java b/...ion-tests/src/test/java/org/gbif/registry/ws/it/collections/merge/BaseMergeServiceIT.java
@@ -183,11 +183,13 @@ public void mergeTest() {
   public void preconditionsTest() {
     T e1 = createEntityToReplace();
     crudService.create(e1);
-    identifierService.addIdentifier(e1.getKey(), new Identifier(IdentifierType.IH_IRN, "test"));
+    Identifier id1 = new Identifier(IdentifierType.IH_IRN, "test");
+    identifierService.addIdentifier(e1.getKey(), id1);
 
     T e2 = createReplacement();
     crudService.create(e2);
-    identifierService.addIdentifier(e2.getKey(), new Identifier(IdentifierType.IH_IRN, "test"));
+    Identifier id2 = new Identifier(IdentifierType.IH_IRN, "test");
+    identifierService.addIdentifier(e2.getKey(), id2);
 
     assertThrows(
         IllegalArgumentException.class, () -> mergeService.merge(e1.getKey(), e2.getKey(), "user"));
@@ -202,6 +204,15 @@ public void preconditionsTest() {
 
     assertThrows(
         IllegalArgumentException.class, () -> mergeService.merge(e1.getKey(), e2.getKey(), null));
+
+    // test that we can't merge 2 idigbio entities
+    identifierService.deleteIdentifier(e1.getKey(), id1.getKey());
+    identifierService.deleteIdentifier(e2.getKey(), id2.getKey());
+
+    machineTagService.addMachineTag(e1.getKey(), new MachineTag("iDigBio.org", "foo", "bar"));
+    machineTagService.addMachineTag(e2.getKey(), new MachineTag("iDigBio.org", "foo2", "bar2"));
+    assertThrows(
+        IllegalArgumentException.class, () -> mergeService.merge(e1.getKey(), e2.getKey(), "user"));
   }
 
   protected Dataset createDataset() {

diff --git a/...ts/src/test/java/org/gbif/registry/ws/it/collections/merge/InstitutionMergeServiceIT.java b/...ts/src/test/java/org/gbif/registry/ws/it/collections/merge/InstitutionMergeServiceIT.java
@@ -195,6 +195,22 @@ public void convertToCollectionMissingArgsTest() {
         () -> institutionMergeService.convertToCollection(UUID.randomUUID(), null, null, "user"));
   }
 
+  @Test
+  public void convertIDigBioInstitution() {
+    Institution toConvert = new Institution();
+    toConvert.setCode("idig");
+    toConvert.setName("idig");
+    institutionService.create(toConvert);
+
+    institutionService.addMachineTag(
+        toConvert.getKey(), new MachineTag("iDigBio.org", "foo", "bar"));
+
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            institutionMergeService.convertToCollection(toConvert.getKey(), null, "test", "user"));
+  }
+
   @Override
   Institution createEntityToReplace() {
     Institution toReplace = new Institution();

diff --git a/...y-service/src/main/java/org/gbif/registry/service/collections/merge/BaseMergeService.java b/...y-service/src/main/java/org/gbif/registry/service/collections/merge/BaseMergeService.java
@@ -110,6 +110,11 @@ public void merge(UUID entityToReplaceKey, UUID replacementKey, String user) {
           "Cannot do the replacement because both entities have an IH IRN identifier");
     }
 
+    if (isIDigBioRecord(entityToReplace) && isIDigBioRecord(replacement)) {
+      throw new IllegalArgumentException(
+          "Cannot do the replacement because both entities are iDigBio records");
+    }
+
     checkMergeExtraPreconditions(entityToReplace, replacement);
 
     // delete and set the replacement
@@ -132,7 +137,6 @@ public void merge(UUID entityToReplaceKey, UUID replacementKey, String user) {
             });
 
     // copy iDigBio machine tags
-    // TODO: what if the replacement has idigbio machine tags already?
     entityToReplace.getMachineTags().stream()
         .filter(mt -> mt.getNamespace().equals("iDigBio.org"))
         .forEach(
@@ -156,10 +160,11 @@ public void merge(UUID entityToReplaceKey, UUID replacementKey, String user) {
     // update occurrence mappings
     List<OccurrenceMapping> occMappings =
         occurrenceMappeableMapper.listOccurrenceMappings(entityToReplaceKey);
-    occMappings.forEach(om -> {
-      occurrenceMappingMapper.createOccurrenceMapping(om);
-      occurrenceMappeableMapper.addOccurrenceMapping(replacementKey, om.getKey());
-    });
+    occMappings.forEach(
+        om -> {
+          occurrenceMappingMapper.createOccurrenceMapping(om);
+          occurrenceMappeableMapper.addOccurrenceMapping(replacementKey, om.getKey());
+        });
 
     additionalOperations(entityToReplace, replacement);
   }
@@ -168,6 +173,10 @@ protected boolean containsIHIdentifier(T entity) {
     return entity.getIdentifiers().stream().anyMatch(i -> i.getType() == IdentifierType.IH_IRN);
   }
 
+  protected boolean isIDigBioRecord(T entity) {
+    return entity.getMachineTags().stream().anyMatch(mt -> mt.getNamespace().equals("iDigBio.org"));
+  }
+
   protected void setNullFields(T target, T source) {
     Class<T> clazz = (Class<T>) target.getClass();
     Arrays.stream(clazz.getDeclaredFields())

diff --git a/...ce/src/main/java/org/gbif/registry/service/collections/merge/InstitutionMergeService.java b/...ce/src/main/java/org/gbif/registry/service/collections/merge/InstitutionMergeService.java
@@ -86,6 +86,7 @@ public UUID convertToCollection(
     Institution institutionToConvert = institutionMapper.get(institutionKey);
     checkArgument(
         institutionToConvert.getDeleted() == null, "Cannot convert a deleted institution");
+    checkArgument(!isIDigBioRecord(institutionToConvert), "Cannot convert an iDigBio institution");
 
     Collection newCollection = new Collection();
     newCollection.setKey(UUID.randomUUID());

diff --git a/...ain/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java b/...ain/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java
@@ -329,7 +329,7 @@ public void deleteOccurrenceMapping(
   }
 
   @PostMapping(value = "{key}/merge")
-  @Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
+  @Secured(GRSCICOLL_ADMIN_ROLE)
   public void merge(@PathVariable("key") UUID entityKey, @RequestBody MergeParams params) {
     Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
     mergeService.merge(entityKey, params.replacementEntityKey, authentication.getName());

diff --git a/...stry-ws/src/main/java/org/gbif/registry/ws/resources/collections/InstitutionResource.java b/...stry-ws/src/main/java/org/gbif/registry/ws/resources/collections/InstitutionResource.java
@@ -56,7 +56,6 @@
 import com.google.common.base.Strings;
 
 import static org.gbif.registry.security.UserRoles.GRSCICOLL_ADMIN_ROLE;
-import static org.gbif.registry.security.UserRoles.GRSCICOLL_EDITOR_ROLE;
 
 /**
  * Class that acts both as the WS endpoint for {@link Institution} entities and also provides an *
@@ -152,7 +151,7 @@ public List<KeyCodeNameResult> suggest(@RequestParam(value = "q", required = fal
   }
 
   @PostMapping("{key}/convertToCollection")
-  @Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
+  @Secured(GRSCICOLL_ADMIN_ROLE)
   public UUID convertToCollection(
       @PathVariable("key") UUID entityKey, @RequestBody ConvertToCollectionParams params) {
     Authentication authentication = SecurityContextHolder.getContext().getAuthentication();