MODLD-617: Fix code based on code review comments

folio-org · Feb 21, 2025 · 0b7d253 · 0b7d253
1 parent 5c58e17
commit 0b7d253
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 24 deletions.
diff --git a/...in/java/org/folio/linked/data/service/resource/marc/ResourceMarcAuthorityServiceImpl.java b/...in/java/org/folio/linked/data/service/resource/marc/ResourceMarcAuthorityServiceImpl.java
@@ -128,15 +128,17 @@ public Long saveMarcAuthority(org.folio.ld.dictionary.model.Resource modelResour
     var resource = resourceModelMapper.toEntity(modelResource);
     validateResource(resource);
 
-    if (existsBySrsIdAndIdUnchanged(resource)) {
-      return updateAuthority(resource);
-    }
+    var srsId = resource.getFolioMetadata().getSrsId();
+    return getResourceIdBySrsId(srsId)
+      .map(existingResourceId -> replaceOrUpdate(resource, existingResourceId))
+      .orElseGet(() -> createAuthority(resource));
+  }
 
-    if (existsBySrsId(resource)) {
-      return replaceAuthority(resource);
+  private Long replaceOrUpdate(Resource incomingResource, Long existingResourceId) {
+    if (existingResourceId.equals(incomingResource.getId())) {
+      return updateAuthority(incomingResource);
     }
-
-    return createAuthority(resource);
+    return replaceAuthority(incomingResource);
   }
 
   private static void validateResource(Resource resource) {
@@ -154,21 +156,11 @@ private static void logAndThrow(String message) {
     throw new IllegalArgumentException(message);
   }
 
-  private boolean existsBySrsIdAndIdUnchanged(Resource resource) {
-    var id = resource.getId();
-    var srsId  = resource.getFolioMetadata().getSrsId();
-    return resourceRepo.existsById(id) && getIdBySrsId(srsId).filter(id::equals).isPresent();
-  }
-
-  private Optional<Long> getIdBySrsId(String srsId) {
+  private Optional<Long> getResourceIdBySrsId(String srsId) {
     return folioMetadataRepository.findIdBySrsId(srsId)
       .map(FolioMetadataRepository.IdOnly::getId);
   }
 
-  private boolean existsBySrsId(Resource resource) {
-    return folioMetadataRepository.existsBySrsId(resource.getFolioMetadata().getSrsId());
-  }
-
   private Long updateAuthority(Resource resource) {
     var id = resource.getId();
     var srsId = resource.getFolioMetadata().getSrsId();

diff --git a/...ava/org/folio/linked/data/service/resource/marc/ResourceMarcAuthorityServiceImplTest.java b/...ava/org/folio/linked/data/service/resource/marc/ResourceMarcAuthorityServiceImplTest.java
@@ -160,7 +160,7 @@ void fetchAuthorityOrCreateFromSrsRecord_shouldThrowNotFound_ifRecordNotExistsIn
   }
 
   @Test
-  void saveMarcAuthority_shouldCreateNewAuthority_ifGivenModelDoesNotExistsByIdAndSrsId() {
+  void saveMarcAuthority_shouldCreateNewAuthority_ifGivenModelDoesNotExistsBySrsId() {
     // given
     var id = randomLong();
     var srsId = UUID.randomUUID().toString();
@@ -169,8 +169,8 @@ void saveMarcAuthority_shouldCreateNewAuthority_ifGivenModelDoesNotExistsByIdAnd
     var mapped = new Resource().setId(id).addTypes(PERSON);
     mapped.setFolioMetadata(new org.folio.linked.data.model.entity.FolioMetadata(mapped).setSrsId(srsId));
     doReturn(mapped).when(resourceModelMapper).toEntity(model);
-    doReturn(false).when(resourceRepo).existsById(id);
     doReturn(mapped).when(resourceGraphService).saveMergingGraph(mapped);
+    doReturn(Optional.empty()).when(folioMetadataRepo).findIdBySrsId(srsId);
 
     // when
     var result = resourceMarcAuthorityService.saveMarcAuthority(model);
@@ -191,7 +191,6 @@ void saveMarcAuthority_shouldUpdateAuthority_ifGivenModelExistsByIdAndSrsId() {
     var mapped = new Resource().setId(id).addTypes(PERSON);
     mapped.setFolioMetadata(new org.folio.linked.data.model.entity.FolioMetadata(mapped).setSrsId(srsId));
     doReturn(mapped).when(resourceModelMapper).toEntity(model);
-    doReturn(true).when(resourceRepo).existsById(id);
     doReturn(Optional.of((FolioMetadataRepository.IdOnly) () -> id)).when(folioMetadataRepo).findIdBySrsId(srsId);
     doReturn(mapped).when(resourceGraphService).saveMergingGraph(mapped);
 
@@ -212,15 +211,14 @@ void saveMarcAuthority_shouldCreateNewAuthorityVersionAndMarkOldAsObsolete_ifGiv
     var existed = new Resource().setId(id).setManaged(true);
     existed.setFolioMetadata(new org.folio.linked.data.model.entity.FolioMetadata(existed));
     doReturn(of(existed)).when(resourceRepo).findByFolioMetadataSrsId(srsId);
-    doReturn(true).when(folioMetadataRepo).existsBySrsId(srsId);
     var model = new org.folio.ld.dictionary.model.Resource()
       .setId(id)
       .setFolioMetadata(new FolioMetadata().setSrsId(srsId));
     var mapped = new Resource().setId(id).addTypes(PERSON);
     mapped.setFolioMetadata(new org.folio.linked.data.model.entity.FolioMetadata(mapped).setSrsId(srsId));
     doReturn(mapped).when(resourceModelMapper).toEntity(model);
-    doReturn(false).when(resourceRepo).existsById(id);
-
+    var anotherResourceId = Optional.of((FolioMetadataRepository.IdOnly) () -> id - 1);
+    doReturn(anotherResourceId).when(folioMetadataRepo).findIdBySrsId(srsId);
     doReturn(mapped).when(resourceGraphService).saveMergingGraph(mapped);
 
     // when