issue #3242 - address review feedback

mostly javadoc updates and a couple minor refactorings Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
LinuxForHealth · Feb 2, 2022 · 053f2ea · 053f2ea
1 parent 2430a86
commit 053f2ea
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 48 deletions.
diff --git a/docs/src/pages/guides/FHIRServerUsersGuide.md b/docs/src/pages/guides/FHIRServerUsersGuide.md
@@ -2267,7 +2267,7 @@ This section contains reference information about each of the configuration prop
 |`fhirServer/core/capabilitiesUrl`|null|
 |`fhirServer/core/externalBaseUrl`|null|
 |`fhirServer/core/ifNoneMatchReturnsNotModified`|false|
-|`fhirServer/core/defaultFhirVersion`|null|
+|`fhirServer/core/defaultFhirVersion`|4.0|
 |`fhirServer/core/useImplicitTypeScopingForWholeSystemInteractions`|true|
 |`fhirServer/validation/failFast`|false|
 |`fhirServer/term/capabilitiesUrl`|null|

diff --git a/fhir-config/src/main/java/com/ibm/fhir/config/FHIRConfigHelper.java b/fhir-config/src/main/java/com/ibm/fhir/config/FHIRConfigHelper.java
@@ -175,9 +175,9 @@ public static List<String> getSupportedResourceTypes() throws FHIRException {
      * @return a non-null list of supported resource types for the given fhirVersion
      */
     public static Set<String> getSupportedResourceTypes(FHIRVersionParam fhirVersion) throws FHIRException {
-        PropertyGroup rsrcsGroup = FHIRConfigHelper.getPropertyGroup(FHIRConfiguration.PROPERTY_RESOURCES);
+        PropertyGroup resourcesGroup = FHIRConfigHelper.getPropertyGroup(FHIRConfiguration.PROPERTY_RESOURCES);
         try {
-            ResourcesConfigAdapter configAdapter = new ResourcesConfigAdapter(rsrcsGroup, fhirVersion);
+            ResourcesConfigAdapter configAdapter = new ResourcesConfigAdapter(resourcesGroup, fhirVersion);
             return configAdapter.getSupportedResourceTypes();
         } catch (Exception e) {
             log.log(Level.SEVERE, "Unexpected exception while constructing a ResourcesConfigAdapter for fhirServer/resources", e);

diff --git a/fhir-config/src/main/java/com/ibm/fhir/config/Interaction.java b/fhir-config/src/main/java/com/ibm/fhir/config/Interaction.java
@@ -5,6 +5,10 @@
  */
 package com.ibm.fhir.config;
 
+/**
+ * Interaction constants to the allowed values of the
+ * fhirServer/resources/[resourceType]/interactions config property
+ */
 public enum Interaction {
     CREATE("create"),
     DELETE("delete"),

diff --git a/fhir-config/src/main/java/com/ibm/fhir/config/ResourcesConfigAdapter.java b/fhir-config/src/main/java/com/ibm/fhir/config/ResourcesConfigAdapter.java
@@ -10,6 +10,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -25,36 +26,18 @@ public class ResourcesConfigAdapter {
     public static final Logger log = Logger.getLogger(ResourcesConfigAdapter.class.getName());
 
     private final Set<String> supportedTypes;
-    private final Map<Interaction, Set<String>> typesByInteraction = new HashMap<>();
+    private final Map<Interaction, Set<String>> typesByInteraction;
 
+    /**
+     * Public constructor
+     *
+     * @param resourcesConfig a PropertyGroup instance for the fhirServer/resources property group
+     * @param fhirVersion a FHIRVersionParam with the fhirVersion to use for computing the applicable resource types
+     * @throws Exception
+     */
     public ResourcesConfigAdapter(PropertyGroup resourcesConfig, FHIRVersionParam fhirVersion) throws Exception {
         supportedTypes = computeSupportedResourceTypes(resourcesConfig, fhirVersion);
-
-        if (resourcesConfig == null) {
-            for (Interaction interaction : Interaction.values()) {
-                typesByInteraction.put(interaction, supportedTypes);
-            }
-            return;
-        }
-
-        for (String resourceType : supportedTypes) {
-            List<String> interactions = resourcesConfig.getStringListProperty(resourceType + "/" + FHIRConfiguration.PROPERTY_FIELD_RESOURCES_INTERACTIONS);
-            if (interactions == null) {
-                interactions = resourcesConfig.getStringListProperty("Resource/" + FHIRConfiguration.PROPERTY_FIELD_RESOURCES_INTERACTIONS);
-            }
-
-            if (interactions == null) {
-                for (Interaction interaction : Interaction.values()) {
-                    typesByInteraction.computeIfAbsent(interaction, k -> new LinkedHashSet<>()).add(resourceType);
-                }
-                continue;
-            }
-
-            for (String interactionString : interactions) {
-                Interaction interaction = Interaction.from(interactionString);
-                typesByInteraction.computeIfAbsent(interaction, k -> new LinkedHashSet<>()).add(resourceType);
-            }
-        }
+        typesByInteraction = computeTypesByInteraction(resourcesConfig);
     }
 
     /**
@@ -83,30 +66,65 @@ public Set<String> getSupportedResourceTypes(Interaction interaction) {
     private Set<String> computeSupportedResourceTypes(PropertyGroup resourcesConfig, FHIRVersionParam fhirVersion) throws Exception {
         Set<String> applicableTypes = ResourceTypeHelper.getResourceTypesFor(fhirVersion);
 
+        Set<String> result;
         if (resourcesConfig == null || resourcesConfig.getBooleanProperty("open", true)) {
-            return applicableTypes;
+            result = applicableTypes;
+        } else {
+            result = new LinkedHashSet<String>();
+            for (PropertyEntry rsrcsEntry : resourcesConfig.getProperties()) {
+                String name = rsrcsEntry.getName();
+
+                // Ensure we skip over the special property "open"
+                // and skip the abstract types Resource and DomainResource
+                if (FHIRConfiguration.PROPERTY_FIELD_RESOURCES_OPEN.equals(name) ||
+                        ResourceTypeHelper.getAbstractResourceTypeNames().contains(name)) {
+                    continue;
+                }
+
+                if (applicableTypes.contains(name)) {
+                    result.add(name);
+                } else if (log.isLoggable(Level.FINE)) {
+                    log.fine("Configured resource type '" + name + "' is not valid "
+                            + "or not applicable for fhirVersion " + fhirVersion.value());
+                }
+            }
         }
 
-        Set<String> result = new LinkedHashSet<String>();
-        for (PropertyEntry rsrcsEntry : resourcesConfig.getProperties()) {
-            String name = rsrcsEntry.getName();
+        return Collections.unmodifiableSet(result);
+    }
 
-            // Ensure we skip over the special property "open"
-            // and skip the abstract types Resource and DomainResource
-            if (FHIRConfiguration.PROPERTY_FIELD_RESOURCES_OPEN.equals(name) ||
-                    "Resource".equals(name) ||
-                    "DomainResource".equals(name)) {
-                continue;
+    // note that this private method depends on the member supportedTypes having already been computed
+    private Map<Interaction, Set<String>> computeTypesByInteraction(PropertyGroup resourcesConfig) throws Exception {
+        Map<Interaction, Set<String>> typeMap = new HashMap<>();
+        if (resourcesConfig == null) {
+            for (Interaction interaction : Interaction.values()) {
+                typeMap.put(interaction, supportedTypes);
             }
+        } else {
+            for (String resourceType : supportedTypes) {
+                List<String> interactions = resourcesConfig.getStringListProperty(resourceType + "/" + FHIRConfiguration.PROPERTY_FIELD_RESOURCES_INTERACTIONS);
+                if (interactions == null) {
+                    interactions = resourcesConfig.getStringListProperty("Resource/" + FHIRConfiguration.PROPERTY_FIELD_RESOURCES_INTERACTIONS);
+                }
+
+                if (interactions == null) {
+                    for (Interaction interaction : Interaction.values()) {
+                        typeMap.computeIfAbsent(interaction, k -> new LinkedHashSet<>()).add(resourceType);
+                    }
+                    continue;
+                }
 
-            if (applicableTypes.contains(name)) {
-                result.add(name);
-            } else if (log.isLoggable(Level.FINE)) {
-                log.fine("Configured resource type '" + name + "' is not valid "
-                        + "or not applicable for fhirVersion " + fhirVersion.value());
+                for (String interactionString : interactions) {
+                    Interaction interaction = Interaction.from(interactionString);
+                    typeMap.computeIfAbsent(interaction, k -> new LinkedHashSet<>()).add(resourceType);
+                }
             }
         }
 
-        return Collections.unmodifiableSet(result);
+        Map<Interaction, Set<String>> finalMap = new HashMap<>();
+        for (Entry<Interaction, Set<String>> entry : typeMap.entrySet()) {
+            finalMap.put(entry.getKey(), Collections.unmodifiableSet(entry.getValue()));
+        }
+        return Collections.unmodifiableMap(finalMap);
     }
 }
diff --git a/fhir-core/src/main/java/com/ibm/fhir/core/FHIRVersionParam.java b/fhir-core/src/main/java/com/ibm/fhir/core/FHIRVersionParam.java
@@ -14,7 +14,12 @@ public enum FHIRVersionParam {
 
     private final String value;
 
-    FHIRVersionParam(String value) {
+    /**
+     * Private constructor
+     *
+     * @param value the fhirVersion value string
+     */
+    private FHIRVersionParam(String value) {
         this.value = value;
     }
 

diff --git a/fhir-core/src/main/java/com/ibm/fhir/core/util/ResourceTypeHelper.java b/fhir-core/src/main/java/com/ibm/fhir/core/util/ResourceTypeHelper.java
@@ -21,7 +21,6 @@
 public class ResourceTypeHelper {
     private static final Set<ResourceTypeName> REMOVED_RESOURCE_TYPES = collectRemovedResourceTypes();
     private static final Set<ResourceTypeName> R4B_ONLY_RESOURCE_TYPES = collectR4bOnlyResourceTypes();
-
     private static final Set<ResourceTypeName> ABSTRACT_TYPES = Collections.unmodifiableSet(new HashSet<>(
             Arrays.asList(
                 ResourceTypeName.RESOURCE,
@@ -48,6 +47,11 @@ public class ResourceTypeHelper {
                 .map(ResourceTypeName::value)
                 .collect(Collectors.toList())));
 
+    private static final Set<String> ABSTRACT_RESOURCES = Collections.unmodifiableSet(
+            ABSTRACT_TYPES.stream()
+                .map(ResourceTypeName::value)
+                .collect(Collectors.toSet()));
+
     /**
      * @param fhirVersion The value of the MIME-type parameter 'fhirVersion' for the current interaction
      *          (e.g. "4.3" not "4.3.0")
@@ -71,6 +75,14 @@ public static Set<String> getNewOrBreakingResourceTypeNames() {
         return R4B_ONLY_RESOURCES;
     }
 
+    /**
+     * @return the set of resource type names that were either introduced in 4.3.0 (e.g. Ingredient) or changed
+     *          in backwards-incompatible ways in the 4.3.0 release (e.g. Evidence and EvidenceVariable)
+     */
+    public static Set<String> getAbstractResourceTypeNames() {
+        return ABSTRACT_RESOURCES;
+    }
+
     private static Set<ResourceTypeName> collectRemovedResourceTypes() {
         Set<ResourceTypeName> set = new HashSet<>();
         set.add(ResourceTypeName.EFFECT_EVIDENCE_SYNTHESIS);

diff --git a/fhir-core/src/test/java/com/ibm/fhir/core/util/test/ResourceTypeHelperTest.java b/fhir-core/src/test/java/com/ibm/fhir/core/util/test/ResourceTypeHelperTest.java
@@ -15,6 +15,9 @@
 import com.ibm.fhir.core.FHIRVersionParam;
 import com.ibm.fhir.core.util.ResourceTypeHelper;
 
+/**
+ * Tests for the ResourceTypeHelper class
+ */
 public class ResourceTypeHelperTest {
     @Test
     public void testGetNewOrBreakingResourceTypeNames() {