9: Code cleanup and Addressing Test Debt - Adding Unit Tests for Custom Endpoints PractitionerDetails and LocationHierarchy

README.md

@@ -160,6 +160,36 @@ $ mvn clean test jacoco:report
 
 The test report is located at `/plugins/target/site/jacoco/index.html`
 
+## Accessing FHIR and Custom Endpoints with the New Gateway
+
+With the recent refactor in the gateway-plugin repository, accessing FHIR and
+custom endpoints through the new gateway has undergone changes. This section
+outlines the updated approach for accessing different types of endpoints.
+
+### FHIR Endpoints
+
+When utilizing the (new) gateway, it is now mandatory to include the `/fhir/`
+part in the URL when accessing FHIR endpoints. This adjustment aligns our
+structure with Google's gateway.
+
+Example:
+
+`https://gateway.example.com/fhir/Patient`
+
+### Custom Endpoints
+
+For custom endpoints such as `/Practitioner-Detail` and `/LocationHierarchy`,
+there is no need to include the `/fhir/` part. Directly use the endpoint in the
+URL:
+
+This approach ensures consistency and clarity when accessing various endpoint
+types through the gateway.
+
+#### Important Note:
+
+Developers, please update your client applications accordingly to accommodate
+these changes in the endpoint structure.
+
 ## Documentation
 
 - HAPI FHIR JPA Starter project

exec/src/main/java/org/smartregister/fhir/gateway/MainApp.java

@@ -6,7 +6,7 @@
 
 /**
  * This class shows the minimum that is required to create a FHIR Gateway with all AccessChecker
- * plugins defined in "com.google.fhir.gateway.plugin".
+ * plugins defined in "com.google.fhir.gateway.plugin" and "org.smartregister.fhir.gateway.plugins.
  */
 @SpringBootApplication(
         scanBasePackages = {"org.smartregister.fhir.gateway", "com.google.fhir.gateway.plugin"})

plugins/src/main/java/org/smartregister/fhir/gateway/plugins/LocationHierarchyEndpointHelper.java

@@ -10,7 +10,6 @@
 import javax.annotation.Nullable;
 
 import org.apache.commons.lang3.StringUtils;
-import org.hl7.fhir.r4.model.BaseResource;
 import org.hl7.fhir.r4.model.Bundle;
 import org.hl7.fhir.r4.model.Location;
 import org.hl7.fhir.r4.model.StringType;
@@ -92,26 +91,6 @@ public List<Location> descendants(String locationId, Location parentLocation) {
         return allLocations;
     }
 
-    private @Nullable List<Location> getLocationsByIds(List<String> locationIds) {
-        if (locationIds == null || locationIds.isEmpty()) {
-            return new ArrayList<>();
-        }
-
-        Bundle locationsBundle =
-                getFhirClientForR4()
-                        .search()
-                        .forResource(Location.class)
-                        .where(
-                                new ReferenceClientParam(BaseResource.SP_RES_ID)
-                                        .hasAnyOfIds(locationIds))
-                        .returnBundle(Bundle.class)
-                        .execute();
-
-        return locationsBundle.getEntry().stream()
-                .map(bundleEntryComponent -> ((Location) bundleEntryComponent.getResource()))
-                .collect(Collectors.toList());
-    }
-
     private @Nullable Location getLocationsByIdentifier(String identifier) {
         Bundle locationsBundle =
                 getFhirClientForR4()

plugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java

@@ -209,23 +209,6 @@ private IGenericClient createFhirClientForR4(FhirContext fhirContext) {
             return client;
         }
 
-        private Composition readCompositionResource(String applicationId, FhirContext fhirContext) {
-            IGenericClient client = createFhirClientForR4(fhirContext);
-            Bundle compositionBundle =
-                    client.search()
-                            .forResource(Composition.class)
-                            .where(Composition.IDENTIFIER.exactly().identifier(applicationId))
-                            .returnBundle(Bundle.class)
-                            .execute();
-            List<Bundle.BundleEntryComponent> compositionEntries =
-                    compositionBundle != null
-                            ? compositionBundle.getEntry()
-                            : Collections.singletonList(new Bundle.BundleEntryComponent());
-            Bundle.BundleEntryComponent compositionEntry =
-                    !compositionEntries.isEmpty() ? compositionEntries.get(0) : null;
-            return compositionEntry != null ? (Composition) compositionEntry.getResource() : null;
-        }
-
         private String getBinaryResourceReference(Composition composition) {
 
             String id = "";

plugins/src/main/java/org/smartregister/fhir/gateway/plugins/PractitionerDetailsEndpointHelper.java

@@ -1,10 +1,6 @@
 package org.smartregister.fhir.gateway.plugins;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -489,31 +485,40 @@ private List<String> getManagingOrganizationsOfCareTeamIds(List<CareTeam> careTe
     }
 
     private List<CareTeam> mapBundleToCareTeams(Bundle careTeams) {
-        return careTeams.getEntry().stream()
-                .map(bundleEntryComponent -> (CareTeam) bundleEntryComponent.getResource())
-                .collect(Collectors.toList());
+        return careTeams != null
+                ? careTeams.getEntry().stream()
+                        .map(bundleEntryComponent -> (CareTeam) bundleEntryComponent.getResource())
+                        .collect(Collectors.toList())
+                : Collections.emptyList();
     }
 
     private List<PractitionerRole> mapBundleToPractitionerRolesWithOrganization(
             Bundle practitionerRoles) {
-        return practitionerRoles.getEntry().stream()
-                .map(it -> (PractitionerRole) it.getResource())
-                .collect(Collectors.toList());
+        return practitionerRoles != null
+                ? practitionerRoles.getEntry().stream()
+                        .map(it -> (PractitionerRole) it.getResource())
+                        .collect(Collectors.toList())
+                : Collections.emptyList();
     }
 
     private List<Group> mapBundleToGroups(Bundle groupsBundle) {
-        return groupsBundle.getEntry().stream()
-                .map(bundleEntryComponent -> (Group) bundleEntryComponent.getResource())
-                .collect(Collectors.toList());
+        return groupsBundle != null
+                ? groupsBundle.getEntry().stream()
+                        .map(bundleEntryComponent -> (Group) bundleEntryComponent.getResource())
+                        .collect(Collectors.toList())
+                : Collections.emptyList();
     }
 
     private List<OrganizationAffiliation> mapBundleToOrganizationAffiliation(
             Bundle organizationAffiliationBundle) {
-        return organizationAffiliationBundle.getEntry().stream()
-                .map(
-                        bundleEntryComponent ->
-                                (OrganizationAffiliation) bundleEntryComponent.getResource())
-                .collect(Collectors.toList());
+        return organizationAffiliationBundle != null
+                ? organizationAffiliationBundle.getEntry().stream()
+                        .map(
+                                bundleEntryComponent ->
+                                        (OrganizationAffiliation)
+                                                bundleEntryComponent.getResource())
+                        .collect(Collectors.toList())
+                : Collections.emptyList();
     }
 
     private List<LocationHierarchy> getLocationsHierarchy(List<String> locationsIdentifiers) {

plugins/src/test/java/org/smartregister/fhir/gateway/plugins/LocationHeirarchyEndpointHelperTest.java

@@ -0,0 +1,66 @@
+package org.smartregister.fhir.gateway.plugins;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.Identifier;
+import org.hl7.fhir.r4.model.Location;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.internal.stubbing.defaultanswers.ReturnsDeepStubs;
+import org.smartregister.model.location.LocationHierarchy;
+
+import ca.uhn.fhir.rest.client.api.IGenericClient;
+import ca.uhn.fhir.rest.gclient.ICriterion;
+
+public class LocationHeirarchyEndpointHelperTest {
+
+    private LocationHierarchyEndpointHelper locationHierarchyEndpointHelper;
+    IGenericClient client;
+
+    @Before
+    public void setUp() {
+        client = mock(IGenericClient.class, new ReturnsDeepStubs());
+        locationHierarchyEndpointHelper = new LocationHierarchyEndpointHelper(client);
+    }
+
+    @Test
+    public void testGetLocationHierarchyNotFound() {
+        Bundle bundleLocation = new Bundle();
+        Object whenSearchLocation =
+                client.search()
+                        .forResource(Location.class)
+                        .where(any(ICriterion.class))
+                        .returnBundle(Bundle.class)
+                        .execute();
+
+        when(whenSearchLocation).thenReturn(bundleLocation);
+        LocationHierarchy locationHierarchy =
+                locationHierarchyEndpointHelper.getLocationHierarchy("12345");
+        assertEquals(
+                org.smartregister.utils.Constants.LOCATION_RESOURCE_NOT_FOUND,
+                locationHierarchy.getId());
+    }
+
+    private Bundle getLocationBundle() {
+        Bundle bundleLocation = new Bundle();
+        bundleLocation.setId("Location/1234");
+        Bundle.BundleEntryComponent bundleEntryComponent = new Bundle.BundleEntryComponent();
+        Location location = new Location();
+        location.setId("Location/1234");
+        Identifier identifier = new Identifier();
+        identifier.setValue("location-1234-abcd");
+        List<Identifier> identifiers = new ArrayList<Identifier>();
+        identifiers.add(identifier);
+        location.setIdentifier(identifiers);
+        bundleEntryComponent.setResource(location);
+        bundleLocation.addEntry(bundleEntryComponent);
+        return bundleLocation;
+    }
+}

plugins/src/test/java/org/smartregister/fhir/gateway/plugins/PermissionAccessCheckerTest.java

@@ -8,6 +8,7 @@
 import java.net.URL;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.hl7.fhir.r4.model.Enumerations;
@@ -73,7 +74,7 @@ public void testManagePatientRoleCanAccessGetPatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("MANAGE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("MANAGE_PATIENT"));
         when(claimMock.asMap()).thenReturn(map);
         when(claimMock.asString()).thenReturn("ecbis-saa");
 
@@ -92,7 +93,7 @@ public void testGetPatientRoleCanAccessGetPatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("GET_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("GET_PATIENT"));
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
         when(requestMock.getRequestType()).thenReturn(RequestTypeEnum.GET);
@@ -109,7 +110,7 @@ public void testGetPatientWithoutRoleCannotAccessGetPatient() throws IOException
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList(""));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of(""));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -127,7 +128,7 @@ public void testDeletePatientRoleCanAccessDeletePatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("DELETE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("DELETE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -145,7 +146,7 @@ public void testManagePatientRoleCanAccessDeletePatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("MANAGE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("MANAGE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -163,7 +164,7 @@ public void testDeletePatientWithoutRoleCannotAccessDeletePatient() throws IOExc
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList(""));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of(""));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -181,7 +182,7 @@ public void testPutWithManagePatientRoleCanAccessPutPatient() throws IOException
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("MANAGE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("MANAGE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -198,7 +199,7 @@ public void testPutPatientWithRoleCanAccessPutPatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("PUT_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("PUT_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -215,7 +216,7 @@ public void testPutPatientWithoutRoleCannotAccessPutPatient() throws IOException
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList(""));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of(""));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -232,7 +233,7 @@ public void testPostPatientWithRoleCanAccessPostPatient() throws IOException {
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("POST_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("POST_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -249,7 +250,7 @@ public void testPostPatientWithoutRoleCannotAccessPostPatient() throws IOExcepti
         setUpFhirBundle("test_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList(""));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of(""));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
         when(requestMock.getResourceName()).thenReturn(Enumerations.ResourceType.PATIENT.name());
@@ -265,7 +266,7 @@ public void testManageResourceRoleCanAccessBundlePutResources() throws IOExcepti
         setUpFhirBundle("bundle_transaction_put_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("MANAGE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("MANAGE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
 
@@ -283,7 +284,7 @@ public void testPutResourceRoleCanAccessBundlePutResources() throws IOException
         setUpFhirBundle("bundle_transaction_put_patient.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("PUT_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("PUT_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
 
@@ -301,7 +302,7 @@ public void testDeleteResourceRoleCanAccessBundleDeleteResources() throws IOExce
         setUpFhirBundle("bundle_transaction_delete.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("DELETE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("DELETE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
 
@@ -380,7 +381,7 @@ public void testBundleResourceNonTransactionTypeThrowsException() throws IOExcep
         setUpFhirBundle("bundle_empty.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList());
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of());
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
 
@@ -394,7 +395,7 @@ public void testAccessGrantedWhenManageResourcePresentForTypeBundleResources()
         setUpFhirBundle("test_bundle_transaction.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("MANAGE_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("MANAGE_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);
 
@@ -436,7 +437,7 @@ public void testAccessDeniedWhenSingleRoleMissingForTypeBundleResources() throws
         setUpFhirBundle("test_bundle_transaction.json");
 
         Map<String, Object> map = new HashMap<>();
-        map.put(PermissionAccessChecker.Factory.ROLES, Arrays.asList("PUT_PATIENT"));
+        map.put(PermissionAccessChecker.Factory.ROLES, List.of("PUT_PATIENT"));
         map.put(PermissionAccessChecker.Factory.FHIR_CORE_APPLICATION_ID_CLAIM, "ecbis-saa");
         when(claimMock.asMap()).thenReturn(map);