From b1ff6cbe378f9de084b02035ea80b433a144357d Mon Sep 17 00:00:00 2001
From: ianmarshall <ian@simpatico.ai>
Date: Fri, 3 Jul 2020 14:25:24 -0400
Subject: [PATCH] Changes from code review

---
 .../jpa/term/TermDeferredStorageSvcImpl.java  | 14 +++++---
 .../term/TermDeferredStorageSvcImplTest.java  | 32 +++++++++++++++++++
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImpl.java
index afb60c1b580..589652cdb36 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImpl.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImpl.java
@@ -50,7 +50,6 @@ import org.springframework.transaction.annotation.Transactional;
 import org.springframework.transaction.support.TransactionTemplate;
 
 import javax.annotation.PostConstruct;
-import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -146,10 +145,15 @@ public class TermDeferredStorageSvcImpl implements ITermDeferredStorageSvc {
 		ourLog.info("Saving {} deferred concepts...", count);
 		while (codeCount < count && myDeferredConcepts.size() > 0) {
 			TermConcept next = myDeferredConcepts.remove(0);
-			try {
-         	codeCount += myCodeSystemStorageSvc.saveConcept(next);
-			} catch (Exception theE) {
-				ourLog.warn("Unable to save deferred TermConcept {}, possibly because Code System {} version PID {} may have since been replaced.",
+			if(myCodeSystemVersionDao.findById(next.getCodeSystemVersion().getPid()).isPresent()) {
+				try {
+					codeCount += myCodeSystemStorageSvc.saveConcept(next);
+				} catch (Exception theE) {
+					ourLog.error("Exception thrown when attempting to save TermConcept {} in Code System {}",
+						next.getCode(), next.getCodeSystemVersion().getCodeSystemDisplayName(), theE);
+				}
+			} else {
+				ourLog.warn("Unable to save deferred TermConcept {} because Code System {} version PID {} is no longer valid. Code system may have since been replaced.",
 					next.getCode(), next.getCodeSystemVersion().getCodeSystemDisplayName(), next.getCodeSystemVersion().getPid());
 			}
 		}
diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImplTest.java
index e98b68c5270..131065c49e5 100644
--- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImplTest.java
+++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermDeferredStorageSvcImplTest.java
@@ -13,6 +13,8 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.transaction.PlatformTransactionManager;
 
+import java.util.Optional;
+
 import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.*;
 
@@ -50,6 +52,7 @@ public class TermDeferredStorageSvcImplTest {
 		svc.setCodeSystemStorageSvcForUnitTest(myTermConceptStorageSvc);
 		svc.setDaoConfigForUnitTest(new DaoConfig());
 
+		when(myTermCodeSystemVersionDao.findById(anyLong())).thenReturn(Optional.of(myTermCodeSystemVersion));
 		svc.setCodeSystemVersionDaoForUnitTest(myTermCodeSystemVersionDao);
 		svc.setProcessDeferred(true);
 		svc.addConceptToStorageQueue(concept);
@@ -65,6 +68,7 @@ public class TermDeferredStorageSvcImplTest {
 		concept.setCode("CODE_A");
 
 		TermCodeSystemVersion myTermCodeSystemVersion = new TermCodeSystemVersion();
+		myTermCodeSystemVersion.setId(1L);
 		concept.setCodeSystemVersion(myTermCodeSystemVersion);
 
 		TermDeferredStorageSvcImpl svc = new TermDeferredStorageSvcImpl();
@@ -72,6 +76,34 @@ public class TermDeferredStorageSvcImplTest {
 		svc.setCodeSystemStorageSvcForUnitTest(myTermConceptStorageSvc);
 		svc.setDaoConfigForUnitTest(new DaoConfig());
 
+		when(myTermCodeSystemVersionDao.findById(anyLong())).thenReturn(Optional.empty());
+		svc.setCodeSystemVersionDaoForUnitTest(myTermCodeSystemVersionDao);
+		svc.setProcessDeferred(true);
+		svc.addConceptToStorageQueue(concept);
+		svc.saveDeferred();
+
+		verify(myTermConceptStorageSvc, times(0)).saveConcept(same(concept));
+		verifyNoMoreInteractions(myTermConceptStorageSvc);
+
+	}
+
+	@Test
+	public void testSaveDeferred_Concept_Exception() {
+		// There is a small
+		TermConcept concept = new TermConcept();
+		concept.setCode("CODE_A");
+
+		TermCodeSystemVersion myTermCodeSystemVersion = new TermCodeSystemVersion();
+		myTermCodeSystemVersion.setId(1L);
+		concept.setCodeSystemVersion(myTermCodeSystemVersion);
+
+		TermDeferredStorageSvcImpl svc = new TermDeferredStorageSvcImpl();
+		svc.setTransactionManagerForUnitTest(myTxManager);
+		svc.setCodeSystemStorageSvcForUnitTest(myTermConceptStorageSvc);
+		svc.setDaoConfigForUnitTest(new DaoConfig());
+
+		// Simulate the case where an exception is thrown despite a valid code system version.
+		when(myTermCodeSystemVersionDao.findById(anyLong())).thenReturn(Optional.of(myTermCodeSystemVersion));
 		when(myTermConceptStorageSvc.saveConcept(concept)).thenThrow(new RuntimeException("Foreign Constraint Violation"));
 		svc.setCodeSystemVersionDaoForUnitTest(myTermCodeSystemVersionDao);
 		svc.setProcessDeferred(true);