From b946f53d847097ef6683565605e1a9fe2488fd8c Mon Sep 17 00:00:00 2001
From: fde <franck.desaize@kereval.com>
Date: Mon, 12 Jul 2021 16:41:16 +0200
Subject: [PATCH] Refactoring and clean code

---
 .../provider/ChPatientResourceProvider.java   | 22 ++++++---
 .../provider/IhePatientResourceProvider.java  | 48 ++++++++++---------
 .../ChPatientResourceProviderTest.java        | 30 ++++++------
 .../IhePatientResourceProviderTest.java       | 22 +++++----
 4 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/src/main/java/net/ihe/gazelle/business/provider/ChPatientResourceProvider.java b/src/main/java/net/ihe/gazelle/business/provider/ChPatientResourceProvider.java
index 7941e95..1b65406 100644
--- a/src/main/java/net/ihe/gazelle/business/provider/ChPatientResourceProvider.java
+++ b/src/main/java/net/ihe/gazelle/business/provider/ChPatientResourceProvider.java
@@ -6,6 +6,8 @@ import java.util.List;
 import javax.inject.Inject;
 import javax.inject.Named;
 
+import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Patient;
@@ -38,6 +40,9 @@ public class ChPatientResourceProvider implements IResourceProvider {
     public static final String INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY = "Invalid request : targetDomain can not be empty";
     public static final String INVALID_REQUEST_THE_REQUEST_SHALL_CONTAIN_ONE_OR_TWO_TARGET_DOMAIN = "Invalid request : The request shall contain " +
             "one or two targetDomain";
+    public static final String SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND = "sourceIdentifier Patient Identifier not found";
+    public static final String SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND = "sourceIdentifier Assigning Authority not found";
+    public static final String TARGET_SYSTEM_NOT_FOUND = "targetSystem not found";
     private static final Logger patientLogger = LoggerFactory.getLogger(ChPatientResourceProvider.class);
     @Inject
     private PatientRegistryXRefSearchClient patientRegistryXRefSearchClient;
@@ -86,18 +91,21 @@ public class ChPatientResourceProvider implements IResourceProvider {
 
     private EntityIdentifier createEntityIdentifierFromSourceIdentifier(TokenParam sourceIdentifier) {
         if (sourceIdentifier == null) {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         String sourceIdentifierSystem = sourceIdentifier.getSystem();
         String sourceIdentifierValue = sourceIdentifier.getValue();
         if (sourceIdentifierSystem == null) {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " null system");
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         if (sourceIdentifierValue == null) {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " null value");
+            throw new InvalidRequestException(SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND);
         }
         if (sourceIdentifierSystem.isBlank() || sourceIdentifierValue.isBlank()) {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " empty system or value");
+            throw new InvalidRequestException(SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND);
         }
         patientLogger.info("Searching Patient with given sourceIdentifier");
 
@@ -111,7 +119,7 @@ public class ChPatientResourceProvider implements IResourceProvider {
             wellFormedEntityIdentifier.setType("ISO");
             wellFormedEntityIdentifier.setValue(sourceIdentifierValue);
         } else {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         return wellFormedEntityIdentifier;
     }
@@ -130,7 +138,7 @@ public class ChPatientResourceProvider implements IResourceProvider {
         for (StringOrListParam listParam : targetSystemParam.getValuesAsQueryTokens()) {
             List<StringParam> queryStrings = listParam.getValuesAsQueryTokens();
             if (queryStrings.isEmpty() || queryStrings.size() > 2) {
-                throw new InvalidRequestException(INVALID_REQUEST_THE_REQUEST_SHALL_CONTAIN_ONE_OR_TWO_TARGET_DOMAIN);
+                throw new ForbiddenOperationException(TARGET_SYSTEM_NOT_FOUND);
             }
             for (StringParam singleParam : queryStrings) {
                 String singleParamValue = singleParam.getValue();
@@ -138,7 +146,7 @@ public class ChPatientResourceProvider implements IResourceProvider {
                     singleParamValue = singleParamValue.replace(URN_OID, "");
                 }
                 if (singleParamValue.equals("")) {
-                    throw new InvalidRequestException(INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY);
+                    throw new ForbiddenOperationException(TARGET_SYSTEM_NOT_FOUND);
                 }
                 targetSystemList.add(singleParamValue);
             }
diff --git a/src/main/java/net/ihe/gazelle/business/provider/IhePatientResourceProvider.java b/src/main/java/net/ihe/gazelle/business/provider/IhePatientResourceProvider.java
index e94622c..d0995a9 100644
--- a/src/main/java/net/ihe/gazelle/business/provider/IhePatientResourceProvider.java
+++ b/src/main/java/net/ihe/gazelle/business/provider/IhePatientResourceProvider.java
@@ -1,17 +1,5 @@
 package net.ihe.gazelle.business.provider;
 
-import java.util.ArrayList;
-import java.util.List;
-
-import javax.inject.Inject;
-import javax.inject.Named;
-
-import org.hl7.fhir.instance.model.api.IBaseResource;
-import org.hl7.fhir.r4.model.Parameters;
-import org.hl7.fhir.r4.model.Patient;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import ca.uhn.fhir.rest.annotation.Operation;
 import ca.uhn.fhir.rest.annotation.OperationParam;
 import ca.uhn.fhir.rest.param.StringAndListParam;
@@ -19,12 +7,24 @@ import ca.uhn.fhir.rest.param.StringOrListParam;
 import ca.uhn.fhir.rest.param.StringParam;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.server.IResourceProvider;
+import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import net.ihe.gazelle.app.patientregistryapi.application.SearchCrossReferenceException;
 import net.ihe.gazelle.app.patientregistryapi.business.EntityIdentifier;
 import net.ihe.gazelle.application.PatientRegistryXRefSearchClient;
 import net.ihe.gazelle.lib.annotations.Package;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.r4.model.Parameters;
+import org.hl7.fhir.r4.model.Patient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * This is a resource provider which stores Patient resources in memory using a HashMap. This is obviously not a production-ready solution for many
@@ -33,10 +33,12 @@ import net.ihe.gazelle.lib.annotations.Package;
  */
 @Named("ihePatientResourceProvider")
 public class IhePatientResourceProvider implements IResourceProvider {
-    public static final String URN_OID = "urn:oid:";
+    private static final String URN_OID = "urn:oid:";
     public static final String INVALID_REQUEST_BAD_SOURCE_IDENTIFIER = "Invalid request : bad sourceIdentifier";
-    public static final String INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY = "Invalid request : targetDomain can not be empty";
     private static final Logger patientLogger = LoggerFactory.getLogger(IhePatientResourceProvider.class);
+    public static final String SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND = "sourceIdentifier Patient Identifier not found";
+    public static final String SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND = "sourceIdentifier Assigning Authority not found";
+    public static final String TARGET_SYSTEM_NOT_FOUND = "targetSystem not found";
     @Inject
     private PatientRegistryXRefSearchClient patientRegistryXRefSearchClient;
 
@@ -50,12 +52,12 @@ public class IhePatientResourceProvider implements IResourceProvider {
 
     public IhePatientResourceProvider() {
     }
-    
+
     @Package
     public IhePatientResourceProvider(PatientRegistryXRefSearchClient client) {
-    	this.patientRegistryXRefSearchClient = client;
+        this.patientRegistryXRefSearchClient = client;
     }
-    
+
     /**
      * Search method for a Patient using the source identifier required parameter
      * and an optional list of target system
@@ -85,21 +87,21 @@ public class IhePatientResourceProvider implements IResourceProvider {
 
     private EntityIdentifier createEntityIdentifierFromSourceIdentifier(TokenParam sourceIdentifier) {
         if (sourceIdentifier == null) {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         String sourceIdentifierSystem = sourceIdentifier.getSystem();
         String sourceIdentifierValue = sourceIdentifier.getValue();
         if (sourceIdentifierSystem == null) {
             patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " null system");
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         if (sourceIdentifierValue == null) {
             patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " null value");
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new InvalidRequestException(SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND);
         }
         if (sourceIdentifierSystem.isBlank() || sourceIdentifierValue.isBlank()) {
             patientLogger.error(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER + " empty system or value");
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new InvalidRequestException(SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND);
         }
         patientLogger.info("Searching Patient with given sourceIdentifier");
 
@@ -113,7 +115,7 @@ public class IhePatientResourceProvider implements IResourceProvider {
             wellFormedEntityIdentifier.setType("ISO");
             wellFormedEntityIdentifier.setValue(sourceIdentifierValue);
         } else {
-            throw new InvalidRequestException(INVALID_REQUEST_BAD_SOURCE_IDENTIFIER);
+            throw new ResourceNotFoundException(SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND);
         }
         return wellFormedEntityIdentifier;
     }
@@ -129,7 +131,7 @@ public class IhePatientResourceProvider implements IResourceProvider {
                         singleParamValue = singleParamValue.replace(URN_OID, "");
                     }
                     if (singleParamValue.equals("")) {
-                        throw new InvalidRequestException(INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY);
+                        throw new ForbiddenOperationException(TARGET_SYSTEM_NOT_FOUND);
                     }
                     targetSystemList.add(singleParamValue);
                 }
diff --git a/src/test/java/net/ihe/gazelle/business/provider/ChPatientResourceProviderTest.java b/src/test/java/net/ihe/gazelle/business/provider/ChPatientResourceProviderTest.java
index 925d4a6..b7f67b8 100644
--- a/src/test/java/net/ihe/gazelle/business/provider/ChPatientResourceProviderTest.java
+++ b/src/test/java/net/ihe/gazelle/business/provider/ChPatientResourceProviderTest.java
@@ -2,6 +2,8 @@ package net.ihe.gazelle.business.provider;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import org.hl7.fhir.r4.model.Identifier;
 import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Parameters.ParametersParameterComponent;
@@ -84,8 +86,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(ChPatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -102,8 +104,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(ChPatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -121,7 +123,7 @@ class ChPatientResourceProviderTest {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
 		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+			assertEquals(ChPatientResourceProvider.SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -138,7 +140,7 @@ class ChPatientResourceProviderTest {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
 		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+			assertEquals(ChPatientResourceProvider.SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -154,8 +156,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(ChPatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -171,8 +173,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY, e.getMessage());
+		catch (ForbiddenOperationException e) {
+			assertEquals(ChPatientResourceProvider.TARGET_SYSTEM_NOT_FOUND, e.getMessage());
 		}
 	}
 	
@@ -186,8 +188,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_THE_REQUEST_SHALL_CONTAIN_ONE_OR_TWO_TARGET_DOMAIN, e.getMessage());
+		catch (ForbiddenOperationException e) {
+			assertEquals(ChPatientResourceProvider.TARGET_SYSTEM_NOT_FOUND, e.getMessage());
 		}
 	}
 	
@@ -204,8 +206,8 @@ class ChPatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(ChPatientResourceProvider.INVALID_REQUEST_THE_REQUEST_SHALL_CONTAIN_ONE_OR_TWO_TARGET_DOMAIN, e.getMessage());
+		catch (ForbiddenOperationException e) {
+			assertEquals(ChPatientResourceProvider.TARGET_SYSTEM_NOT_FOUND, e.getMessage());
 		}
 	}
 
diff --git a/src/test/java/net/ihe/gazelle/business/provider/IhePatientResourceProviderTest.java b/src/test/java/net/ihe/gazelle/business/provider/IhePatientResourceProviderTest.java
index d2b15a4..08d33ab 100644
--- a/src/test/java/net/ihe/gazelle/business/provider/IhePatientResourceProviderTest.java
+++ b/src/test/java/net/ihe/gazelle/business/provider/IhePatientResourceProviderTest.java
@@ -2,6 +2,8 @@ package net.ihe.gazelle.business.provider;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import org.hl7.fhir.r4.model.Identifier;
 import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Parameters.ParametersParameterComponent;
@@ -67,8 +69,8 @@ class IhePatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(IhePatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -85,8 +87,8 @@ class IhePatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(IhePatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -104,7 +106,7 @@ class IhePatientResourceProviderTest {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
 		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+			assertEquals(IhePatientResourceProvider.SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -121,7 +123,7 @@ class IhePatientResourceProviderTest {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
 		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+			assertEquals(IhePatientResourceProvider.SOURCE_IDENTIFIER_ASSIGNING_AUTHORITY_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -137,8 +139,8 @@ class IhePatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_BAD_SOURCE_IDENTIFIER, e.getMessage());
+		catch (ResourceNotFoundException e) {
+			assertEquals(IhePatientResourceProvider.SOURCE_IDENTIFIER_PATIENT_IDENTIFIER_NOT_FOUND, e.getMessage());
 		}
 
 	}
@@ -154,8 +156,8 @@ class IhePatientResourceProviderTest {
 		try {
 			provider.findPatientsByIdentifier(sourceId, targetDomains);
 		}
-		catch (InvalidRequestException e) {
-			assertEquals(IhePatientResourceProvider.INVALID_REQUEST_TARGET_DOMAIN_CAN_NOT_BE_EMPTY, e.getMessage());
+		catch (ForbiddenOperationException e) {
+			assertEquals(IhePatientResourceProvider.TARGET_SYSTEM_NOT_FOUND, e.getMessage());
 		}
 
 	}
-- 
GitLab