Issue #2834 - address review comments

Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
LinuxForHealth · Oct 21, 2021 · d656e75 · d656e75
1 parent 07d8917
commit d656e75
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 39 deletions.
diff --git a/docs/src/pages/guides/FHIRServerUsersGuide.md b/docs/src/pages/guides/FHIRServerUsersGuide.md
@@ -3,7 +3,7 @@ layout: post
 title:  IBM FHIR Server User's Guide
 description: IBM FHIR Server User's Guide
 Copyright: years 2017, 2021
-lastupdated: "2021-10-20"
+lastupdated: "2021-10-21"
 permalink: /FHIRServerUsersGuide/
 ---
 
@@ -801,10 +801,10 @@ For example, you can configure a set of FHIRPath Constraints to run for resource
 
 The following configuration parameters can be used to specify rules relating to the set of profiles that are specified in a resource's `meta.profile` element:
 * `fhirServer/resources/<resourceType>/profiles/atLeastOne` - this configuration parameter is used to specify a set of profiles, at least one of which a resource must claim conformance to and be successfully validated against in order to be persisted to the FHIR server.
-* `fhirServer/resources/<resourceType>/profiles/notAllowed`- this configuration parameter is used to specify a set of profiles to which a resource is *not allowed* to claim conformance and be successfully validated against in order to be persisted to the FHIR server.
+* `fhirServer/resources/<resourceType>/profiles/notAllowed`- this configuration parameter is used to specify a set of profiles to which a resource is *not allowed* to claim conformance.
 * `fhirServer/resources/<resourceType>/profiles/allowUnknown`- this configuration parameter is used to indicate whether a warning or an error is issued if a profile specified in a resource's `meta.profile`element is not loaded in the FHIR server. The default value is `true`, meaning unknown profiles are allowed to be specified. The profile will be ignored and just a warning will be returned. If set to `false`, this means unknown profiles are not allowed to be specified. An error will be returned and resource validation will fail.
 
-Before calling the FHIR validator to validate a resource against the set of profiles specified in its `meta.profile` element that it is claiming conformance to, the following pre-validation will be done for that set of profiles based on the configuration parameters listed above:
+Before calling the FHIR validator to validate a resource against the set of profiles specified in its `meta.profile` element that it is claiming conformance to, the following pre-validation will be performed for that set of profiles based on the configuration parameters listed above:
 1.  If the `fhirServer/resources/<resourceType>/profiles/notAllowed` configuration parameter is set to a non-empty list, an error will be returned for any specified profile that is in the list, and validation will fail.
 2. If the `fhirServer/resources/<resourceType>/profiles/allowUnknown` configuration parameter is set to `false`,  an error will be returned for any specified profile that is not loaded in the FHIR server, and validation will fail.
 3. If the `fhirServer/resources/<resourceType>/profiles/atLeastOne` configuration parameter is set to a non-empty list, an error will be returned if none of the specified profiles is in the list, and validation will fail.

diff --git a/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java
@@ -2561,10 +2561,10 @@ public int doReindexSingle(OperationOutcome.Builder operationOutcomeResult, Inst
      * @throws FHIRValidationException
      */
     private List<Issue> validateResource(Resource resource) throws FHIRValidationException {
-        List<String> atLeastOneProfiles = new ArrayList<>();
-        List<String> atLeastOneProfilesWithoutVersion = new ArrayList<>();
-        List<String> notAllowedProfiles = new ArrayList<>();
-        List<String> notAllowedProfilesWithoutVersion = new ArrayList<>();
+        Set<String> atLeastOneProfiles = new HashSet<>();
+        Set<String> atLeastOneProfilesWithoutVersion = new HashSet<>();
+        Set<String> notAllowedProfiles = new HashSet<>();
+        Set<String> notAllowedProfilesWithoutVersion = new HashSet<>();
         boolean allowUnknown;
 
         // Retrieve the profile configuration
@@ -2580,13 +2580,13 @@ private List<Issue> validateResource(Resource resource) throws FHIRValidationExc
                     FHIRConfigHelper.getStringListProperty(resourceSpecificProfileConfigPath.toString() +
                         FHIRConfiguration.PROPERTY_FIELD_RESOURCES_PROFILES_AT_LEAST_ONE);
             if (resourceSpecificAtLeastOneProfiles != null) {
-                atLeastOneProfiles = resourceSpecificAtLeastOneProfiles;
+                atLeastOneProfiles.addAll(resourceSpecificAtLeastOneProfiles);
             } else {
                 List<String> defaultAtLeastOneProfiles =
                         FHIRConfigHelper.getStringListProperty(defaultProfileConfigPath.toString() +
                             FHIRConfiguration.PROPERTY_FIELD_RESOURCES_PROFILES_AT_LEAST_ONE);
                 if (defaultAtLeastOneProfiles != null) {
-                    atLeastOneProfiles = defaultAtLeastOneProfiles;
+                    atLeastOneProfiles.addAll(defaultAtLeastOneProfiles);
                 }
             }
 
@@ -2606,13 +2606,13 @@ private List<Issue> validateResource(Resource resource) throws FHIRValidationExc
                     FHIRConfigHelper.getStringListProperty(resourceSpecificProfileConfigPath.toString() +
                         FHIRConfiguration.PROPERTY_FIELD_RESOURCES_PROFILES_NOT_ALLOWED);
             if (resourceSpecificNotAllowedProfiles != null) {
-                notAllowedProfiles = resourceSpecificNotAllowedProfiles;
+                notAllowedProfiles.addAll(resourceSpecificNotAllowedProfiles);
             } else {
                 List<String> defaultNotAllowedProfiles =
                         FHIRConfigHelper.getStringListProperty(defaultProfileConfigPath.toString() +
                             FHIRConfiguration.PROPERTY_FIELD_RESOURCES_PROFILES_NOT_ALLOWED);
                 if (defaultNotAllowedProfiles != null) {
-                    notAllowedProfiles = defaultNotAllowedProfiles;
+                    notAllowedProfiles.addAll(defaultNotAllowedProfiles);
                 }
             }
 

diff --git a/fhir-server/src/test/java/com/ibm/fhir/server/test/ProfileValidationConfigTest.java b/fhir-server/src/test/java/com/ibm/fhir/server/test/ProfileValidationConfigTest.java
@@ -7,6 +7,7 @@
 
 import static com.ibm.fhir.model.type.String.string;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.util.List;
@@ -119,8 +120,8 @@ public void testCreateWithNoProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -154,8 +155,8 @@ public void testCreateWithNonRequiredProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -189,8 +190,8 @@ public void testCreateWithRequiredProfileSpecifiedButNoVersion() throws Exceptio
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -514,8 +515,8 @@ public void testUpdateWithNoProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -550,8 +551,8 @@ public void testUpdateWithNonRequiredProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -666,8 +667,8 @@ public void testBundleWithNoProfileSpecified() throws Exception {
             assertEquals(issues.get(0).getDetails().getText().getValue(), "One or more errors were encountered while validating a 'transaction' request bundle.");
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.FATAL);
             assertEquals(issues.get(0).getCode(), IssueType.INVALID);
-            assertEquals(issues.get(1).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(1).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(1).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(1).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -719,8 +720,8 @@ public void testBundleWithNonRequiredProfileSpecified() throws Exception {
             assertEquals(issues.get(0).getDetails().getText().getValue(), "One or more errors were encountered while validating a 'transaction' request bundle.");
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.FATAL);
             assertEquals(issues.get(0).getCode(), IssueType.INVALID);
-            assertEquals(issues.get(1).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(1).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(1).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(1).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -852,8 +853,8 @@ public void testCreateWithNonAllowedProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -889,8 +890,8 @@ public void testCreateWithNonAllowedVersionedProfileSpecified() throws Exception
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -1019,8 +1020,8 @@ public void testUpdateWithNonAllowedProfileSpecified() throws Exception {
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -1057,8 +1058,8 @@ public void testUpdateWithNonAllowedVersionedProfileSpecified() throws Exception
             // Validate results
             List<Issue> issues = e.getIssues();
             assertEquals(issues.size(), 1);
-            assertEquals(issues.get(0).getDetails().getText().getValue(),
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(0).getDetails().getText().getValue().startsWith(
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -1207,8 +1208,8 @@ public void testBundleWithNonAllowedProfileSpecified() throws Exception {
             assertEquals(issues.get(0).getDetails().getText().getValue(), "One or more errors were encountered while validating a 'transaction' request bundle.");
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.FATAL);
             assertEquals(issues.get(0).getCode(), IssueType.INVALID);
-            assertEquals(issues.get(1).getDetails().getText().getValue(), 
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(1).getDetails().getText().getValue().startsWith( 
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(1).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(1).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -1262,8 +1263,8 @@ public void testBundleWithNonAllowedVersionedProfileSpecified() throws Exception
             assertEquals(issues.get(0).getDetails().getText().getValue(), "One or more errors were encountered while validating a 'transaction' request bundle.");
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.FATAL);
             assertEquals(issues.get(0).getCode(), IssueType.INVALID);
-            assertEquals(issues.get(1).getDetails().getText().getValue(),
-                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: [profile8, profile6|1]");
+            assertTrue(issues.get(1).getDetails().getText().getValue().startsWith(
+                "A profile was specified which is not allowed. Resources of type 'CarePlan' are not allowed to declare conformance to any of the following profiles: ["));
             assertEquals(issues.get(1).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(1).getCode(), IssueType.BUSINESS_RULE);
         }
@@ -1445,8 +1446,8 @@ public void testCreateWithNonAllowedNonRequiredProfileSpecified() throws Excepti
                 "A profile was specified which is not allowed. Resources of type 'Patient' are not allowed to declare conformance to any of the following profiles: [profile5]");
             assertEquals(issues.get(0).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(0).getCode(), IssueType.BUSINESS_RULE);
-            assertEquals(issues.get(1).getDetails().getText().getValue(),
-                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: [profile1, profile2|1, profile3]");
+            assertTrue(issues.get(1).getDetails().getText().getValue().startsWith(
+                "A required profile was not specified. Resources of type 'Patient' must declare conformance to at least one of the following profiles: ["));
             assertEquals(issues.get(1).getSeverity(), IssueSeverity.ERROR);
             assertEquals(issues.get(1).getCode(), IssueType.BUSINESS_RULE);
         }