From b9eb84768ffcd3085d019e1e70f74abe47d3a558 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Wed, 4 Dec 2024 08:42:51 -0500 Subject: [PATCH 1/9] Fix issue with large bundles by changing how Bundles get validated, add input to limit entries that are validated --- ...mart_access_brands_validate_brands_test.rb | 20 +-- ...mart_access_brands_validate_bundle_test.rb | 125 +++++++++++++++++- ...cess_brands_validate_endpoint_urls_test.rb | 14 +- ...t_access_brands_validate_endpoints_test.rb | 27 +--- 4 files changed, 137 insertions(+), 49 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 57550d6..4cca05f 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -57,24 +57,10 @@ def skip_message end run do - bundle_response = if user_access_brands_bundle.blank? - load_tagged_requests('smart_access_brands_bundle') - skip skip_message if requests.length != 1 - requests.first.response_body - else - user_access_brands_bundle - end - - skip_if bundle_response.blank?, 'No SMART Access Brands Bundle contained in the response' - - assert_valid_json(bundle_response) - bundle_resource = FHIR.from_contents(bundle_response) + bundle_resource = scratch[:bundle_resource] + skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' - assert_valid_bundle_entries(bundle: bundle_resource, - resource_types: { - Organization: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brand' - }) organization_resources = bundle_resource .entry @@ -82,6 +68,8 @@ def skip_message .select { |resource| resource.resourceType == 'Organization' } organization_resources.each do |organization| + assert_valid_resource(resource: organization, + profile_url: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brand') endpoint_references = organization.endpoint.map(&:reference) if organization.extension.present? diff --git a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb index 8700b1d..5d08395 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb @@ -9,6 +9,15 @@ class SMARTAccessBrandsValidateBundle < Inferno::Test This test also ensures the Bundle is the 'collection' type and that it is not empty. ) + input :resource_validation_limit, + title: 'Limit Validation to a Maximum Resource Count', + description: %( + Input a number to limit the number of Bundle entries that are validated. For very large bundles, it is + recommended to limit the number of Bundle entries to avoid long test run times. + To validate all, leave blank. + ), + optional: true + input :user_access_brands_bundle, optional: true @@ -20,6 +29,74 @@ def skip_message ) end + def get_resource_entries(bundle_resource, resource_type) + bundle_resource + .entry + .select { |entry| entry.resource.resourceType == resource_type } + .uniq + end + + def limit_bundle_entries(resource_validation_limit, bundle_resource) + new_entries = [] + + organization_entries = get_resource_entries(bundle_resource, 'Organization') + endpoint_entries = get_resource_entries(bundle_resource, 'Endpoint') + + organization_entries.each do |organization_entry| + break if resource_validation_limit <= 0 + + new_entries.append(organization_entry) + resource_validation_limit -= 1 + + found_endpoint_entries = [] + organization_resource = organization_entry.resource + + if organization_resource.endpoint.present? + found_endpoint_entries = find_referenced_endpoints(organization_resource.endpoint, endpoint_entries) + elsif organization_resource.partOf.present? + parent_org = find_parent_organization_entry(organization_entries, organization_resource.partOf.reference) + + unless resource_already_exists?(new_entries, parent_org, 'Organization') + new_entries.append(parent_org) + resource_validation_limit -= 1 + + parent_org_resource = parent_org.resource + found_endpoint_entries = find_referenced_endpoints(parent_org_resource.endpoint, endpoint_entries) + end + end + + found_endpoint_entries.each do |found_endpoint_entry| + unless resource_already_exists?(new_entries, found_endpoint_entry, 'Endpoint') + new_entries.append(found_endpoint_entry) + resource_validation_limit -= 1 + end + end + end + new_entries + end + + def find_parent_organization_entry(organization_entries, org_reference) + organization_entries + .find { |parent_org_entry| org_reference.include? parent_org_entry.resource.id } + end + + def find_referenced_endpoints(organization_endpoints, endpoint_entries) + endpoints = [] + organization_endpoints.each do |endpoint_ref| + found_endpoint = endpoint_entries.find do |endpoint_entry| + endpoint_ref.reference.include?(endpoint_entry.resource.id) + end + endpoints.append(found_endpoint) + end + endpoints + end + + def resource_already_exists?(new_entries, found_resource_entry, resource_type) + new_entries.any? do |entry| + entry.resource.resourceType == resource_type && (entry.resource.id == found_resource_entry.resource.id) + end + end + run do bundle_response = if user_access_brands_bundle.blank? load_tagged_requests('smart_access_brands_bundle') @@ -34,12 +111,58 @@ def skip_message assert_valid_json(bundle_response) bundle_resource = FHIR.from_contents(bundle_response) assert_resource_type(:bundle, resource: bundle_resource) - assert_valid_resource(resource: bundle_resource, profile_url: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brands-bundle') + if resource_validation_limit.present? + limited_entries = limit_bundle_entries(resource_validation_limit.to_i, + bundle_resource) + bundle_resource.entry = limited_entries + end + + scratch[:bundle_resource] = bundle_resource + + assert(bundle_resource.type.present?, 'The SMART Access Brands Bundle is missing the required `type` field') assert(bundle_resource.type == 'collection', 'The SMART Access Brands Bundle must be type `collection`') assert(bundle_resource.timestamp.present?, 'Bundle.timestamp must be populated to advertise the timestamp of the last change to the contents') assert !bundle_resource.entry.empty?, 'The given Bundle does not contain any brands or endpoints.' + assert(bundle_resource.total.blank?, 'The `total` field is not allowed in `collection` type Bundles') + + entry_full_urls = [] + + bundle_resource.entry.each_with_index do |entry, index| + assert(entry.resource.present?, %( + Bundle entry #{index} missing the `resource` field. For Bundles of type collection, all entries must contain + resources. + )) + + assert(entry.request.blank?, %( + Bundle entry #{index} contains the `request` field. For Bundles of type collection, all entries must not have + request or response elements + )) + assert(entry.response.blank?, %( + Bundle entry #{index} contains the `response` field. For Bundles of type collection, all entries must not have + request or response elements + )) + assert(entry.search.blank?, %( + Bundle entry #{index} contains the `search` field. Entry.search is allowed only for `search` type Bundles. + )) + + assert(entry.fullUrl.exclude?('/_history/'), %( + Bundle entry #{index} contains a version specific reference in the `fullUrl` field + )) + + full_url_exists = entry_full_urls.any? do |hash| + hash['fullUrl'] == entry.fullUrl && hash['versionId'] == entry.resource&.meta&.versionId + end + + assert(!full_url_exists, %( + The SMART Access Brands Bundle contains entries with duplicate fullUrls (#{entry.fullUrl}) and versionIds + (#{entry.resource&.meta&.versionId}). FullUrl must be unique in a bundle, or else entries with the same + fullUrl must have different meta.versionId + )) + + entry_full_urls.append({ 'fullUrl' => entry.fullUrl, 'versionId' => entry.resource&.meta&.versionId }) + end end end end diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb index 06c16f1..0d43186 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb @@ -58,19 +58,9 @@ def skip_message end run do - bundle_response = if user_access_brands_bundle.blank? - load_tagged_requests('smart_access_brands_bundle') - skip skip_message if requests.length != 1 - requests.first.response_body - else - user_access_brands_bundle - end - - skip_if bundle_response.blank?, 'No SMART Access Brands Bundle contained in the response' - - assert_valid_json(bundle_response) - bundle_resource = FHIR.from_contents(bundle_response) + bundle_resource = scratch[:bundle_resource] + skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' endpoint_list = bundle_resource diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb index 8c2a420..357ab09 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb @@ -33,34 +33,21 @@ def skip_message optional: true run do - bundle_response = if user_access_brands_bundle.blank? - load_tagged_requests('smart_access_brands_bundle') - skip skip_message if requests.length != 1 - requests.first.response_body - else - user_access_brands_bundle - end - - skip_if bundle_response.blank?, 'No SMART Access Brands Bundle contained in the response' - - assert_valid_json(bundle_response) - bundle_resource = FHIR.from_contents(bundle_response) + bundle_resource = scratch[:bundle_resource] + skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' - assert_valid_bundle_entries(bundle: bundle_resource, - resource_types: { - Endpoint: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-endpoint' - }) - - endpoint_ids = + endpoint_resources = bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Endpoint' } - .map(&:id) - endpoint_ids.each do |endpoint_id| + endpoint_resources.each do |endpoint| + assert_valid_resource(resource: endpoint, + profile_url: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-endpoint') + endpoint_id = endpoint.id endpoint_referenced_orgs = find_referenced_org(bundle_resource, endpoint_id) assert !endpoint_referenced_orgs.empty?, "Endpoint with id: #{endpoint_id} does not have any associated Organizations in the Bundle." From f644d9fc112e416374a66809d2e5e14a07d2c8b6 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Wed, 4 Dec 2024 10:27:12 -0500 Subject: [PATCH 2/9] Change the way Endpoint and Organizations are validated to use assert_valid_bundle_entries, and add filter to limit the number of warning/error messages that appear from the validator --- .../smart_access_brands_suite.rb | 29 ++++++++++++++++++- ...mart_access_brands_validate_brands_test.rb | 7 +++-- ...t_access_brands_validate_endpoints_test.rb | 13 +++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_suite.rb b/lib/smart_app_launch/smart_access_brands_suite.rb index ae8cf97..cf41fbd 100644 --- a/lib/smart_app_launch/smart_access_brands_suite.rb +++ b/lib/smart_app_launch/smart_access_brands_suite.rb @@ -50,8 +50,35 @@ class SMARTAccessBrandsSuite < Inferno::TestSuite message_filters = VALIDATION_MESSAGE_FILTERS + $num_messages = 0 + $capped_message = false + $num_errors = 0 + $capped_errors = false + exclude_message do |message| - message_filters.any? { |filter| filter.match? message.message } + matches_filter = message_filters.any? { |filter| filter.match? message.message } + + unless matches_filter + if message.type == 'error' + $num_errors += 1 + else + $num_messages += 1 + end + end + + matches_filter || + (message.type != 'error' && $num_messages > 50 && !message.message.include?('Inferno is only showing the first')) || + (message.type == 'error' && $num_errors > 20 && !message.message.include?('Inferno is only showing the first')) + end + + perform_additional_validation do + if $num_messages > 50 && !$capped_message + $capped_message = true + { type: 'info', message: 'Inferno is only showing the first 50 validation info and warning messages.' } + elsif $num_errors > 20 && !$capped_errors + $capped_errors = true + { type: 'error', message: 'Inferno is only showing the first 20 validation error messages.' } + end end end diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 4cca05f..2181152 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -62,14 +62,17 @@ def skip_message skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' + assert_valid_bundle_entries(bundle: bundle_resource, + resource_types: { + Organization: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brand' + }) + organization_resources = bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Organization' } organization_resources.each do |organization| - assert_valid_resource(resource: organization, - profile_url: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brand') endpoint_references = organization.endpoint.map(&:reference) if organization.extension.present? diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb index 357ab09..f6557d1 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb @@ -38,16 +38,19 @@ def skip_message skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' - endpoint_resources = + assert_valid_bundle_entries(bundle: bundle_resource, + resource_types: { + Endpoint: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-endpoint' + }) + + endpoint_ids = bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Endpoint' } + .map(&:id) - endpoint_resources.each do |endpoint| - assert_valid_resource(resource: endpoint, - profile_url: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-endpoint') - endpoint_id = endpoint.id + endpoint_ids.each do |endpoint_id| endpoint_referenced_orgs = find_referenced_org(bundle_resource, endpoint_id) assert !endpoint_referenced_orgs.empty?, "Endpoint with id: #{endpoint_id} does not have any associated Organizations in the Bundle." From 7dce55e1548ae8e0be32104c7b907b948798def8 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Wed, 4 Dec 2024 12:08:11 -0500 Subject: [PATCH 3/9] Fix spec tests and do some code cleanup as a result --- ...mart_access_brands_validate_brands_test.rb | 14 ++-- ...mart_access_brands_validate_bundle_test.rb | 22 ++++--- ...cess_brands_validate_endpoint_urls_test.rb | 14 ++-- ...t_access_brands_validate_endpoints_test.rb | 12 ++-- ...access_brands_validate_brands_test_spec.rb | 66 ++++--------------- ...access_brands_validate_bundle_test_spec.rb | 55 ++++++++-------- ...brands_validate_endpoint_urls_test_spec.rb | 40 ++++------- ...ess_brands_validate_endpoints_test_spec.rb | 53 ++++----------- 8 files changed, 103 insertions(+), 173 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 2181152..6a8dc82 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -13,9 +13,6 @@ class SMARTAccessBrandsValidateBrands < Inferno::Test This test does not currently validate availability or format of Brand or Portal logos. ) - input :user_access_brands_bundle, - optional: true - def find_referenced_endpoint(bundle_resource, endpoint_id_ref) bundle_resource .entry @@ -56,10 +53,17 @@ def skip_message ) end + def scratch_bundle_resource + scratch[:bundle_resource] ||= {} + end + run do - bundle_resource = scratch[:bundle_resource] + bundle_resource = scratch_bundle_resource - skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' + skip_if bundle_resource.blank?, %( + No successful User Access Brands request was made in the previous test, or no User Access Brands Bundle was + provided + ) skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' assert_valid_bundle_entries(bundle: bundle_resource, diff --git a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb index 5d08395..1ad75bf 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb @@ -106,7 +106,10 @@ def resource_already_exists?(new_entries, found_resource_entry, resource_type) user_access_brands_bundle end - skip_if bundle_response.blank?, 'No SMART Access Brands Bundle contained in the response' + skip_if bundle_response.blank?, %( + No successful User Access Brands request was made in the previous test, or no User Access Brands Bundle was + provided + ) assert_valid_json(bundle_response) bundle_resource = FHIR.from_contents(bundle_response) @@ -130,25 +133,26 @@ def resource_already_exists?(new_entries, found_resource_entry, resource_type) entry_full_urls = [] bundle_resource.entry.each_with_index do |entry, index| + entry_num = index + 1 assert(entry.resource.present?, %( - Bundle entry #{index} missing the `resource` field. For Bundles of type collection, all entries must contain - resources. + Bundle entry #{entry_num} missing the `resource` field. For Bundles of type collection, all entries must + contain resources. )) assert(entry.request.blank?, %( - Bundle entry #{index} contains the `request` field. For Bundles of type collection, all entries must not have - request or response elements + Bundle entry #{entry_num} contains the `request` field. For Bundles of type collection, all entries must not + have request or response elements )) assert(entry.response.blank?, %( - Bundle entry #{index} contains the `response` field. For Bundles of type collection, all entries must not have - request or response elements + Bundle entry #{entry_num} contains the `response` field. For Bundles of type collection, all entries must not + have request or response elements )) assert(entry.search.blank?, %( - Bundle entry #{index} contains the `search` field. Entry.search is allowed only for `search` type Bundles. + Bundle entry #{entry_num} contains the `search` field. Entry.search is allowed only for `search` type Bundles. )) assert(entry.fullUrl.exclude?('/_history/'), %( - Bundle entry #{index} contains a version specific reference in the `fullUrl` field + Bundle entry #{entry_num} contains a version specific reference in the `fullUrl` field )) full_url_exists = entry_full_urls.any? do |hash| diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb index 0d43186..1f15d19 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoint_urls_test.rb @@ -7,9 +7,6 @@ class SMARTAccessBrandsValidateEndpointURLs < Inferno::Test and available. ) - input :user_access_brands_bundle, - optional: true - input :endpoint_availability_limit, title: 'Endpoint Availability Limit', description: %( @@ -57,10 +54,17 @@ def skip_message ) end + def scratch_bundle_resource + scratch[:bundle_resource] ||= {} + end + run do - bundle_resource = scratch[:bundle_resource] + bundle_resource = scratch_bundle_resource - skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' + skip_if bundle_resource.blank?, %( + No successful User Access Brands request was made in the previous test, or no User Access Brands Bundle was + provided + ) skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' endpoint_list = bundle_resource diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb index f6557d1..5e9569e 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb @@ -29,13 +29,17 @@ def skip_message ) end - input :user_access_brands_bundle, - optional: true + def scratch_bundle_resource + scratch[:bundle_resource] ||= {} + end run do - bundle_resource = scratch[:bundle_resource] + bundle_resource = scratch_bundle_resource - skip_if bundle_resource.blank?, 'No SMART Access Brands Bundle contained in the response' + skip_if bundle_resource.blank?, %( + No successful User Access Brands request was made in the previous test, or no User Access Brands Bundle was + provided + ) skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' assert_valid_bundle_entries(bundle: bundle_resource, diff --git a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb index ae9fcc6..34d5d8f 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb @@ -8,9 +8,9 @@ let(:result) { repo_create(:result, test_session_id: test_session.id) } let(:smart_access_brands_bundle) do - JSON.parse(File.read(File.join( - __dir__, '..', 'fixtures', 'smart_access_brands_example.json' - ))) + FHIR.from_contents(File.read(File.join( + __dir__, '..', 'fixtures', 'smart_access_brands_example.json' + ))) end let(:operation_outcome_success) do @@ -35,21 +35,6 @@ end let(:validator_url) { ENV.fetch('FHIR_RESOURCE_VALIDATOR_URL') } - let(:user_access_brands_publication_url) { 'http://fhirserver.org/smart_access_brands_example.json' } - - def create_user_access_brands_request(url: user_access_brands_publication_url, body: nil, status: 200) - repo_create( - :request, - name: 'retrieve_smart_access_brands_bundle', - direction: 'outgoing', - url:, - result:, - test_session_id: test_session.id, - response_body: body.is_a?(Hash) ? body.to_json : body, - status:, - tags: ['smart_access_brands_bundle'] - ) - end def run(runnable, inputs = {}) test_run_params = { test_session_id: test_session.id }.merge(runnable.reference_hash) @@ -86,7 +71,7 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) @@ -94,45 +79,18 @@ def run(runnable, inputs = {}) expect(validation_request).to have_been_made end - it 'passes if inputed in User Access Brands Bundle contains valid Brands' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) - - result = run(test, user_access_brands_bundle: smart_access_brands_bundle.to_json) - - expect(result.result).to eq('pass') - expect(validation_request).to have_been_made - end - it 'skips if no User Access Brands Bundle requests were made' do result = run(test) expect(result.result).to eq('skip') expect(result.result_message).to match( - 'No User Access Brands request was made in the previous test, and no User Access Brands Bundle was provided' + 'No successful User Access Brands request was made in the previous test' ) end - it 'skips if User Access Brands Bundle request does not contain a response body' do - create_user_access_brands_request - result = run(test) - - expect(result.result).to eq('skip') - expect(result.result_message).to eq('No SMART Access Brands Bundle contained in the response') - end - - it 'fails if User Access Brands Bundle is an invalid JSON' do - create_user_access_brands_request(body: '[[') - - result = run(test) - - expect(result.result).to eq('fail') - expect(result.result_message).to eq('Invalid JSON. ') - end - it 'skips if User Access Brands Bundle is empty' do - smart_access_brands_bundle['entry'] = [] - create_user_access_brands_request(body: smart_access_brands_bundle) + smart_access_brands_bundle.entry = [] + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) expect(result.result).to eq('skip') @@ -143,7 +101,7 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_failure.to_json) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) @@ -156,8 +114,8 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) - smart_access_brands_bundle['entry'].first['resource']['endpoint'].shift - create_user_access_brands_request(body: smart_access_brands_bundle) + smart_access_brands_bundle.entry.first.resource.endpoint.shift + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) @@ -170,8 +128,8 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) - smart_access_brands_bundle['entry'].delete_at(1) - create_user_access_brands_request(body: smart_access_brands_bundle) + smart_access_brands_bundle.entry.delete_at(1) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) diff --git a/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb index 61b3d30..ca23b64 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb @@ -83,25 +83,17 @@ def run(runnable, inputs = {}) end it 'passes if User Access Brands Bundle is valid' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) - create_user_access_brands_request(body: smart_access_brands_bundle) result = run(test) expect(result.result).to eq('pass') - expect(validation_request).to have_been_made end it 'passes if inputted User Access Brands Bundle is valid' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) - result = run(test, user_access_brands_bundle: smart_access_brands_bundle.to_json) expect(result.result).to eq('pass') - expect(validation_request).to have_been_made end it 'skips if no User Access Brands Bundle requests were made' do @@ -118,7 +110,7 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('skip') - expect(result.result_message).to eq('No SMART Access Brands Bundle contained in the response') + expect(result.result_message).to match('No successful User Access Brands request was made in the previous test') end it 'fails if User Access Brands Bundle is an invalid JSON' do @@ -140,25 +132,43 @@ def run(runnable, inputs = {}) expect(result.result_message).to eq('Unexpected resource type: expected Bundle, but received Patient') end - it 'fails if the User Access Brands Bundle fails validation' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_failure.to_json) + it 'fails if the User Access Brands Bundle contains duplicate fullUrls' do + smart_access_brands_bundle['entry'].first['fullUrl'] = 'https://ehr1.fhirserver.com/Endpoint/examplehospital-ehr1' + create_user_access_brands_request(body: smart_access_brands_bundle) + + result = run(test) + + expect(result.result).to eq('fail') + expect(result.result_message).to match( + 'The SMART Access Brands Bundle contains entries with duplicate fullUrls' + ) + end + it 'fails if the User Access Brands Bundle contains an entry with the request field' do + smart_access_brands_bundle['entry'].first['request'] = { 'method' => 'GET', 'url' => 'examplerequest.com' } create_user_access_brands_request(body: smart_access_brands_bundle) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to eq( - 'Resource does not conform to the profile: http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brands-bundle' + expect(result.result_message).to match( + 'Bundle entry 1 contains the `request` field' ) - expect(validation_request).to have_been_made end - it 'fails if User Access Brands Bundle is not type collection' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) + it 'fails if the User Access Brands Bundle contains an entry with a version specific fullUrl reference' do + smart_access_brands_bundle['entry'].first['fullUrl'] = 'examplerequest.com/_history/2' + create_user_access_brands_request(body: smart_access_brands_bundle) + + result = run(test) + + expect(result.result).to eq('fail') + expect(result.result_message).to match( + 'Bundle entry 1 contains a version specific reference' + ) + end + it 'fails if User Access Brands Bundle is not type collection' do smart_access_brands_bundle['type'] = 'history' create_user_access_brands_request(body: smart_access_brands_bundle) @@ -166,13 +176,9 @@ def run(runnable, inputs = {}) expect(result.result).to eq('fail') expect(result.result_message).to eq('The SMART Access Brands Bundle must be type `collection`') - expect(validation_request).to have_been_made end it 'fails if User Access Brands Bundle does not have the timestamp field populated' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) - smart_access_brands_bundle.delete('timestamp') create_user_access_brands_request(body: smart_access_brands_bundle) @@ -182,20 +188,15 @@ def run(runnable, inputs = {}) expect(result.result_message).to eq( 'Bundle.timestamp must be populated to advertise the timestamp of the last change to the contents' ) - expect(validation_request).to have_been_made end it 'fails if User Access Brands Bundle is empty' do - validation_request = stub_request(:post, "#{validator_url}/validate") - .to_return(status: 200, body: operation_outcome_success.to_json) - smart_access_brands_bundle['entry'] = [] create_user_access_brands_request(body: smart_access_brands_bundle) result = run(test) expect(result.result).to eq('fail') expect(result.result_message).to eq('The given Bundle does not contain any brands or endpoints.') - expect(validation_request).to have_been_made end end end diff --git a/spec/smart_app_launch/smart_access_brands_validate_endpoint_urls_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_endpoint_urls_test_spec.rb index f8a15fc..4dbcde7 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_endpoint_urls_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_endpoint_urls_test_spec.rb @@ -8,9 +8,9 @@ let(:result) { repo_create(:result, test_session_id: test_session.id) } let(:smart_access_brands_bundle) do - JSON.parse(File.read(File.join( - __dir__, '..', 'fixtures', 'smart_access_brands_example.json' - ))) + FHIR.from_contents(File.read(File.join( + __dir__, '..', 'fixtures', 'smart_access_brands_example.json' + ))) end let(:capability_statement) do @@ -22,20 +22,6 @@ let(:user_access_brands_publication_url) { 'http://fhirserver.org/smart_access_brands_example.json' } let(:base_url) { 'http://example.com/api/FHIR/R4' } - def create_user_access_brands_request(url: user_access_brands_publication_url, body: nil, status: 200) - repo_create( - :request, - name: 'retrieve_smart_access_brands_bundle', - direction: 'outgoing', - url:, - result:, - test_session_id: test_session.id, - response_body: body.is_a?(Hash) ? body.to_json : body, - status:, - tags: ['smart_access_brands_bundle'] - ) - end - def entity_result_message(runnable) results_repo.current_results_for_test_session_and_runnables(test_session.id, [runnable]) .first @@ -83,7 +69,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 200, body: capability_statement.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all') @@ -92,9 +78,9 @@ def run(runnable, inputs = {}) end it 'fails if Endpoint in Bundle contains an invalid URLs' do - smart_access_brands_bundle['entry'][1]['resource']['address'] = 'invalid_url' + smart_access_brands_bundle.entry[1].resource.address = 'invalid_url' - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all') @@ -107,7 +93,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 404, body: capability_statement.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all') @@ -121,7 +107,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 200, body: { 'example' => 'example' }.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all') @@ -136,7 +122,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 200, body: capability_statement.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all') @@ -152,7 +138,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 200, body: capability_statement.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'all', endpoint_availability_limit: 1) @@ -162,7 +148,7 @@ def run(runnable, inputs = {}) end it 'passes if at least 1 endpoint is available when success rate input is set to at least 1' do - smart_access_brands_bundle['entry'][1]['resource']['address'] = "#{base_url}/fake/address/1" + smart_access_brands_bundle.entry[1].resource.address = "#{base_url}/fake/address/1" uri_template = Addressable::Template.new "#{base_url}/{id}/metadata" capability_statement_request_success = stub_request(:get, uri_template) @@ -172,7 +158,7 @@ def run(runnable, inputs = {}) capability_statement_request_fail = stub_request(:get, fake_uri_template) .to_return(status: 404, body: '', headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'at_least_1') @@ -188,7 +174,7 @@ def run(runnable, inputs = {}) capability_statement_request = stub_request(:get, uri_template) .to_return(status: 200, body: capability_statement.to_json, headers: {}) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_publication_url:, endpoint_availability_success_rate: 'none') diff --git a/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb index 88bd628..242f972 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb @@ -8,9 +8,9 @@ let(:result) { repo_create(:result, test_session_id: test_session.id) } let(:smart_access_brands_bundle) do - JSON.parse(File.read(File.join( - __dir__, '..', 'fixtures', 'smart_access_brands_example.json' - ))) + FHIR.from_contents(File.read(File.join( + __dir__, '..', 'fixtures', 'smart_access_brands_example.json' + ))) end let(:operation_outcome_success) do @@ -35,21 +35,6 @@ end let(:validator_url) { ENV.fetch('FHIR_RESOURCE_VALIDATOR_URL') } - let(:user_access_brands_publication_url) { 'http://fhirserver.org/smart_access_brands_example.json' } - - def create_user_access_brands_request(url: user_access_brands_publication_url, body: nil, status: 200) - repo_create( - :request, - name: 'retrieve_smart_access_brands_bundle', - direction: 'outgoing', - url:, - result:, - test_session_id: test_session.id, - response_body: body.is_a?(Hash) ? body.to_json : body, - status:, - tags: ['smart_access_brands_bundle'] - ) - end def run(runnable, inputs = {}) test_run_params = { test_session_id: test_session.id }.merge(runnable.reference_hash) @@ -86,6 +71,7 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test, user_access_brands_bundle: smart_access_brands_bundle.to_json) expect(result.result).to eq('pass') @@ -96,7 +82,7 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) @@ -109,30 +95,13 @@ def run(runnable, inputs = {}) expect(result.result).to eq('skip') expect(result.result_message).to match( - 'No User Access Brands request was made in the previous test, and no User Access Brands Bundle was provided' + 'No successful User Access Brands request was made in the previous test' ) end - it 'skips if User Access Brands Bundle request does not contain a response body' do - create_user_access_brands_request - result = run(test) - - expect(result.result).to eq('skip') - expect(result.result_message).to eq('No SMART Access Brands Bundle contained in the response') - end - - it 'fails if User Access Brands Bundle is an invalid JSON' do - create_user_access_brands_request(body: '[[') - - result = run(test) - - expect(result.result).to eq('fail') - expect(result.result_message).to eq('Invalid JSON. ') - end - it 'skips if User Access Brands Bundle is empty' do - smart_access_brands_bundle['entry'] = [] - create_user_access_brands_request(body: smart_access_brands_bundle) + smart_access_brands_bundle.entry = [] + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) expect(result.result).to eq('skip') @@ -143,7 +112,7 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_failure.to_json) - create_user_access_brands_request(body: smart_access_brands_bundle) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) @@ -158,8 +127,8 @@ def run(runnable, inputs = {}) validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) - smart_access_brands_bundle['entry'].first['resource']['endpoint'].delete_at(1) - create_user_access_brands_request(body: smart_access_brands_bundle) + smart_access_brands_bundle.entry.first.resource.endpoint.delete_at(1) + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) result = run(test) From bae8d5fbb07619645d364a6d2d88f399c8e72c37 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Fri, 6 Dec 2024 00:12:20 -0500 Subject: [PATCH 4/9] Move validation filtering from validator to within the running test --- .../smart_access_brands_suite.rb | 29 +------------- ...mart_access_brands_validate_brands_test.rb | 39 ++++++++++++++----- ...t_access_brands_validate_endpoints_test.rb | 36 ++++++++++++----- 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_suite.rb b/lib/smart_app_launch/smart_access_brands_suite.rb index cf41fbd..ae8cf97 100644 --- a/lib/smart_app_launch/smart_access_brands_suite.rb +++ b/lib/smart_app_launch/smart_access_brands_suite.rb @@ -50,35 +50,8 @@ class SMARTAccessBrandsSuite < Inferno::TestSuite message_filters = VALIDATION_MESSAGE_FILTERS - $num_messages = 0 - $capped_message = false - $num_errors = 0 - $capped_errors = false - exclude_message do |message| - matches_filter = message_filters.any? { |filter| filter.match? message.message } - - unless matches_filter - if message.type == 'error' - $num_errors += 1 - else - $num_messages += 1 - end - end - - matches_filter || - (message.type != 'error' && $num_messages > 50 && !message.message.include?('Inferno is only showing the first')) || - (message.type == 'error' && $num_errors > 20 && !message.message.include?('Inferno is only showing the first')) - end - - perform_additional_validation do - if $num_messages > 50 && !$capped_message - $capped_message = true - { type: 'info', message: 'Inferno is only showing the first 50 validation info and warning messages.' } - elsif $num_errors > 20 && !$capped_errors - $capped_errors = true - { type: 'error', message: 'Inferno is only showing the first 20 validation error messages.' } - end + message_filters.any? { |filter| filter.match? message.message } end end diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 6a8dc82..13dd124 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -39,9 +39,12 @@ def check_portal_endpoints(portal_endpoints, organization_endpoints) portal_endpoint_found = organization_endpoints.any? do |endpoint_reference| portal_endpoint.valueReference.reference == endpoint_reference end - assert(portal_endpoint_found, %( + next if portal_endpoint_found + + add_message('error', %( Portal endpoints must also appear at Organization.endpoint. The portal endpoint with reference - #{portal_endpoint.valueReference.reference} was not found at Organization.endpoint.)) + #{portal_endpoint.valueReference.reference} was not found at Organization.endpoint. + )) end end @@ -66,17 +69,14 @@ def scratch_bundle_resource ) skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' - assert_valid_bundle_entries(bundle: bundle_resource, - resource_types: { - Organization: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-brand' - }) - organization_resources = bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Organization' } organization_resources.each do |organization| + resource_is_valid?(resource: organization) + endpoint_references = organization.endpoint.map(&:reference) if organization.extension.present? @@ -89,11 +89,30 @@ def scratch_bundle_resource endpoint_references.each do |endpoint_id_ref| organization_referenced_endpts = find_referenced_endpoint(bundle_resource, endpoint_id_ref) - assert !organization_referenced_endpts.empty?, - "Organization with id: #{organization.id} references an Endpoint that is not contained in this - bundle." + next unless organization_referenced_endpts.empty? + + add_message('error', %( + Organization with id: #{organization.id} references an Endpoint that is not contained in this + bundle. + )) end end + + error_messages = messages.select { |msg| msg[:type] == 'error' } + non_error_messages = messages.reject { |msg| msg[:type] == 'error' } + + @messages = [] + @messages += error_messages.first(20) unless error_messages.empty? + @messages += non_error_messages.first(50) unless non_error_messages.empty? + + if error_messages.count > 20 || non_error_messages.count > 50 + info_message = 'Inferno is only showing the first 20 error and 50 warning/information validation messages' + add_message('info', info_message) + end + + assert messages.empty? || messages.none? { |msg| msg[:type] == 'error' }, %( + Some Organizations in the Service Base URL Bundle are invalid + ) end end end diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb index 5e9569e..8eeb6e8 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb @@ -42,23 +42,39 @@ def scratch_bundle_resource ) skip_if bundle_resource.entry.empty?, 'The given Bundle does not contain any resources' - assert_valid_bundle_entries(bundle: bundle_resource, - resource_types: { - Endpoint: 'http://hl7.org/fhir/smart-app-launch/StructureDefinition/user-access-endpoint' - }) - - endpoint_ids = + endpoint_resources = bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Endpoint' } - .map(&:id) - endpoint_ids.each do |endpoint_id| + endpoint_resources.each do |endpoint| + resource_is_valid?(resource: endpoint) + + endpoint_id = endpoint.id endpoint_referenced_orgs = find_referenced_org(bundle_resource, endpoint_id) - assert !endpoint_referenced_orgs.empty?, - "Endpoint with id: #{endpoint_id} does not have any associated Organizations in the Bundle." + next unless endpoint_referenced_orgs.empty? + + add_message('error', %( + Endpoint with id: #{endpoint_id} does not have any associated Organizations in the Bundle. + )) end + + error_messages = messages.select { |msg| msg[:type] == 'error' } + non_error_messages = messages.reject { |msg| msg[:type] == 'error' } + + @messages = [] + @messages += error_messages.first(20) unless error_messages.empty? + @messages += non_error_messages.first(50) unless non_error_messages.empty? + + if error_messages.count > 20 || non_error_messages.count > 50 + info_message = 'Inferno is only showing the first 20 error and 50 warning/information validation messages' + add_message('info', info_message) + end + + assert messages.empty? || messages.none? { |msg| msg[:type] == 'error' }, %( + Some Endpoints in the Service Base URL Bundle are invalid + ) end end end From dec87a8ab0cf8084f018388f1ec2931c3bcb5a53 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:53:07 -0500 Subject: [PATCH 5/9] Add some safety checks and add functionality to loop through remaining endpoints if all Organizations have been added and limit has not yet been reached in case there are endpoints that no Organization references --- ...mart_access_brands_validate_bundle_test.rb | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb index 1ad75bf..3a574fd 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb @@ -56,7 +56,7 @@ def limit_bundle_entries(resource_validation_limit, bundle_resource) elsif organization_resource.partOf.present? parent_org = find_parent_organization_entry(organization_entries, organization_resource.partOf.reference) - unless resource_already_exists?(new_entries, parent_org, 'Organization') + unless parent_org.blank? || resource_already_exists?(new_entries, parent_org, 'Organization') new_entries.append(parent_org) resource_validation_limit -= 1 @@ -66,12 +66,25 @@ def limit_bundle_entries(resource_validation_limit, bundle_resource) end found_endpoint_entries.each do |found_endpoint_entry| - unless resource_already_exists?(new_entries, found_endpoint_entry, 'Endpoint') - new_entries.append(found_endpoint_entry) - resource_validation_limit -= 1 + next if resource_already_exists?(new_entries, found_endpoint_entry, 'Endpoint') + + new_entries.append(found_endpoint_entry) + + endpoint_entries.delete_if do |entry| + entry.resource.resourceType == 'Endpoint' && entry.resource.id == found_endpoint_entry.resource.id end + + resource_validation_limit -= 1 end end + + endpoint_entries.each do |endpoint_entry| + break if resource_validation_limit <= 0 + + new_entries.append(endpoint_entry) + resource_validation_limit -= 1 + end + new_entries end @@ -86,7 +99,7 @@ def find_referenced_endpoints(organization_endpoints, endpoint_entries) found_endpoint = endpoint_entries.find do |endpoint_entry| endpoint_ref.reference.include?(endpoint_entry.resource.id) end - endpoints.append(found_endpoint) + endpoints.append(found_endpoint) if found_endpoint.present? end endpoints end From 10572b3805b27f7b672d7ae0a50d9ba5efe69459 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:05:11 -0500 Subject: [PATCH 6/9] Fix spec tests --- ...rt_access_brands_validate_brands_test_spec.rb | 16 +++++++++++++--- ...access_brands_validate_endpoints_test_spec.rb | 16 +++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb index 34d5d8f..41357a6 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb @@ -6,6 +6,7 @@ let(:results_repo) { Inferno::Repositories::Results.new } let(:test_session) { repo_create(:test_session, test_suite_id: 'smart_access_brands') } let(:result) { repo_create(:result, test_session_id: test_session.id) } + let(:runnable) { Inferno::Repositories::Tests.new.find('smart_access_brands_valid_brands') } let(:smart_access_brands_bundle) do FHIR.from_contents(File.read(File.join( @@ -36,6 +37,13 @@ let(:validator_url) { ENV.fetch('FHIR_RESOURCE_VALIDATOR_URL') } + def entity_result_message + results_repo.current_results_for_test_session_and_runnables(test_session.id, [runnable]) + .first + .messages + .first + end + def run(runnable, inputs = {}) test_run_params = { test_session_id: test_session.id }.merge(runnable.reference_hash) test_run = Inferno::Repositories::TestRuns.new.create(test_run_params) @@ -106,7 +114,9 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to eq('The following bundle entries are invalid: Organization#examplehospital') + expect(entity_result_message.message).to match( + 'Resource does not conform to profile' + ) expect(validation_request).to have_been_made end @@ -120,7 +130,7 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to match('Portal endpoints must also appear at Organization.endpoint') + expect(entity_result_message.message).to match('Portal endpoints must also appear at Organization.endpoint') expect(validation_request).to have_been_made end @@ -134,7 +144,7 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to match( + expect(entity_result_message.message).to match( 'Organization with id: examplehospital references an Endpoint that is not contained' ) expect(validation_request).to have_been_made diff --git a/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb index 242f972..1ce38b1 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_endpoints_test_spec.rb @@ -6,6 +6,7 @@ let(:results_repo) { Inferno::Repositories::Results.new } let(:test_session) { repo_create(:test_session, test_suite_id: 'smart_access_brands') } let(:result) { repo_create(:result, test_session_id: test_session.id) } + let(:runnable) { Inferno::Repositories::Tests.new.find('smart_access_brands_valid_endpoints') } let(:smart_access_brands_bundle) do FHIR.from_contents(File.read(File.join( @@ -36,6 +37,13 @@ let(:validator_url) { ENV.fetch('FHIR_RESOURCE_VALIDATOR_URL') } + def entity_result_message + results_repo.current_results_for_test_session_and_runnables(test_session.id, [runnable]) + .first + .messages + .first + end + def run(runnable, inputs = {}) test_run_params = { test_session_id: test_session.id }.merge(runnable.reference_hash) test_run = Inferno::Repositories::TestRuns.new.create(test_run_params) @@ -117,9 +125,11 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to eq( - 'The following bundle entries are invalid: Endpoint#examplehospital-ehr1, Endpoint#examplehospital-ehr2' + + expect(entity_result_message.message).to match( + 'Resource does not conform to profile' ) + expect(validation_request).to have_been_made.times(2) end @@ -133,7 +143,7 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('fail') - expect(result.result_message).to match( + expect(entity_result_message.message).to match( 'Endpoint with id: examplehospital-ehr2 does not have any associated Organizations in the Bundle' ) expect(validation_request).to have_been_made.times(2) From b667f7336e0d5a48b8f0a7a34924cd3310d1afca Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:12:09 -0500 Subject: [PATCH 7/9] Fix User Access Brands Brand validation to allow sub-organizations --- ...mart_access_brands_validate_brands_test.rb | 49 ++++++++++++++++--- .../fixtures/smart_access_brands_example.json | 40 +++++++++++++++ ...access_brands_validate_brands_test_spec.rb | 23 +++++++-- 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 13dd124..4ee1e12 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -22,6 +22,14 @@ def find_referenced_endpoint(bundle_resource, endpoint_id_ref) .select { |endpoint_id| endpoint_id_ref.include? endpoint_id } end + def find_parent_organization(bundle_resource, org_reference) + bundle_resource + .entry + .map(&:resource) + .select { |resource| resource.resourceType == 'Organization' } + .find { |parent_org| org_reference.include? parent_org.id } + end + def find_extension(extension_array, extension_name) extension_array.find do |extension| extension.url.ends_with?(extension_name) @@ -78,7 +86,6 @@ def scratch_bundle_resource resource_is_valid?(resource: organization) endpoint_references = organization.endpoint.map(&:reference) - if organization.extension.present? portal_extension = find_extension(organization.extension, '/organization-portal') if portal_extension.present? @@ -87,14 +94,40 @@ def scratch_bundle_resource end end - endpoint_references.each do |endpoint_id_ref| - organization_referenced_endpts = find_referenced_endpoint(bundle_resource, endpoint_id_ref) - next unless organization_referenced_endpts.empty? + if organization.endpoint.empty? + if organization.partOf.blank? + add_message('error', %( + Organization with id: #{organization.id} does not have the endpoint or partOf field populated + )) + next + end + + parent_organization = find_parent_organization(bundle_resource, organization.partOf.reference) + + if parent_organization.blank? + add_message('error', %( + Organization with id: #{organization.id} references parent Organization not found in the Bundle: + #{organization.partOf.reference} + )) + next + end - add_message('error', %( - Organization with id: #{organization.id} references an Endpoint that is not contained in this - bundle. - )) + if parent_organization.endpoint.empty? + add_message('error', %( + Organization with id: #{organization.id} has parent Organization with id: #{parent_organization.id} that + does not have the `endpoint` field populated. + )) + end + else + endpoint_references.each do |endpoint_id_ref| + organization_referenced_endpts = find_referenced_endpoint(bundle_resource, endpoint_id_ref) + next unless organization_referenced_endpts.empty? + + add_message('error', %( + Organization with id: #{organization.id} references an Endpoint that is not contained in this + bundle. + )) + end end end diff --git a/spec/fixtures/smart_access_brands_example.json b/spec/fixtures/smart_access_brands_example.json index 2e27837..224049a 100644 --- a/spec/fixtures/smart_access_brands_example.json +++ b/spec/fixtures/smart_access_brands_example.json @@ -193,6 +193,46 @@ ], "address" : "http://example.com/api/FHIR/R4/2" } + }, + { + "fullUrl" : "https://ehr.example.com/Organization/ehchospital", + "resource" : { + "resourceType" : "Organization", + "id" : "ehchospital", + "extension" : [{ + "extension" : [{ + "url" : "brandLogo", + "valueUrl" : "data:image/svg+xml;base64,PHN2ZyB0aXRsZT0iU3Rld2FydCBNZW1vcmlhbCBIb3Nw...+IDwvc3ZnPgo=" + }], + "url" : "http://hl7.org/fhir/StructureDefinition/organization-brand" + }], + "identifier" : [{ + "system" : "urn:ietf:rfc:3986", + "value" : "https://ehchospital.example.org" + }], + "active" : true, + "type" : [{ + "coding" : [{ + "system" : "http://terminology.hl7.org/CodeSystem/organization-type", + "code" : "prov", + "display" : "Healthcare Provider" + }] + }], + "name" : "ExampleHealth Community Hospital", + "alias" : ["GoodHealth Madison"], + "telecom" : [{ + "system" : "url", + "value" : "https://www.ehchospital.example.com" + }], + "address" : [{ + "city" : "Lake City", + "state" : "IA", + "country" : "US" + }], + "partOf" : { + "reference" : "Organization/examplehospital" + } + } } ] } \ No newline at end of file diff --git a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb index 41357a6..f77d37f 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb @@ -84,7 +84,7 @@ def run(runnable, inputs = {}) result = run(test) expect(result.result).to eq('pass') - expect(validation_request).to have_been_made + expect(validation_request).to have_been_made.times(2) end it 'skips if no User Access Brands Bundle requests were made' do @@ -117,7 +117,22 @@ def run(runnable, inputs = {}) expect(entity_result_message.message).to match( 'Resource does not conform to profile' ) - expect(validation_request).to have_been_made + expect(validation_request).to have_been_made.times(2) + end + + it 'fails if Brand missing endpoint and partOf fields' do + validation_request = stub_request(:post, "#{validator_url}/validate") + .to_return(status: 200, body: operation_outcome_success.to_json) + smart_access_brands_bundle.entry.last.resource.partOf = nil + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) + + result = run(test) + + expect(result.result).to eq('fail') + expect(entity_result_message.message).to match( + 'Organization with id: ehchospital does not have the endpoint or partOf field populated' + ) + expect(validation_request).to have_been_made.times(2) end it 'fails if Brand contains Endpoint in portal extension but not Organization.endpoint' do @@ -131,7 +146,7 @@ def run(runnable, inputs = {}) expect(result.result).to eq('fail') expect(entity_result_message.message).to match('Portal endpoints must also appear at Organization.endpoint') - expect(validation_request).to have_been_made + expect(validation_request).to have_been_made.times(2) end it 'fails if Brand contains Endpoint reference not found in Bundle' do @@ -147,7 +162,7 @@ def run(runnable, inputs = {}) expect(entity_result_message.message).to match( 'Organization with id: examplehospital references an Endpoint that is not contained' ) - expect(validation_request).to have_been_made + expect(validation_request).to have_been_made.times(2) end end end From dcb14f7173bce4fd3855e8d4ccedc7e9a1a2dd9a Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:16:56 -0500 Subject: [PATCH 8/9] Add additional spec test to ensure sub-org changes work as expected --- ...rt_access_brands_validate_brands_test_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb index f77d37f..5aa3e59 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb @@ -135,6 +135,22 @@ def run(runnable, inputs = {}) expect(validation_request).to have_been_made.times(2) end + it 'fails if Brand partOf references an Organization that does not exist' do + validation_request = stub_request(:post, "#{validator_url}/validate") + .to_return(status: 200, body: operation_outcome_success.to_json) + + smart_access_brands_bundle.entry.shift + allow_any_instance_of(test).to receive(:scratch_bundle_resource).and_return(smart_access_brands_bundle) + + result = run(test) + + expect(result.result).to eq('fail') + expect(entity_result_message.message).to match( + 'Organization with id: ehchospital references parent Organization not found in the Bundle' + ) + expect(validation_request).to have_been_made.times(1) + end + it 'fails if Brand contains Endpoint in portal extension but not Organization.endpoint' do validation_request = stub_request(:post, "#{validator_url}/validate") .to_return(status: 200, body: operation_outcome_success.to_json) From 7293d96d6a299b6a9a7174199263c5e5437bec57 Mon Sep 17 00:00:00 2001 From: Emily Michaud <59289146+emichaud998@users.noreply.github.com> Date: Mon, 9 Dec 2024 14:04:42 -0500 Subject: [PATCH 9/9] Fix how resource ids are checked in resource references --- .../smart_access_brands_validate_brands_test.rb | 14 ++++++++++---- .../smart_access_brands_validate_bundle_test.rb | 10 ++++++++-- .../smart_access_brands_validate_endpoints_test.rb | 8 +++++++- ...mart_access_brands_validate_brands_test_spec.rb | 2 +- ...mart_access_brands_validate_bundle_test_spec.rb | 8 ++++++++ 5 files changed, 34 insertions(+), 8 deletions(-) diff --git a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb index 4ee1e12..5678609 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_brands_test.rb @@ -13,13 +13,19 @@ class SMARTAccessBrandsValidateBrands < Inferno::Test This test does not currently validate availability or format of Brand or Portal logos. ) + def regex_match?(resource_id, reference) + return false if resource_id.blank? + + %r{#{resource_id}(?:/[^\/]*|\|[^\/]*)*/?$}.match?(reference) + end + def find_referenced_endpoint(bundle_resource, endpoint_id_ref) bundle_resource .entry .map(&:resource) .select { |resource| resource.resourceType == 'Endpoint' } .map(&:id) - .select { |endpoint_id| endpoint_id_ref.include? endpoint_id } + .select { |endpoint_id| regex_match?(endpoint_id, endpoint_id_ref) } end def find_parent_organization(bundle_resource, org_reference) @@ -27,7 +33,7 @@ def find_parent_organization(bundle_resource, org_reference) .entry .map(&:resource) .select { |resource| resource.resourceType == 'Organization' } - .find { |parent_org| org_reference.include? parent_org.id } + .find { |parent_org| regex_match?(parent_org.id, org_reference) } end def find_extension(extension_array, extension_name) @@ -124,8 +130,8 @@ def scratch_bundle_resource next unless organization_referenced_endpts.empty? add_message('error', %( - Organization with id: #{organization.id} references an Endpoint that is not contained in this - bundle. + Organization with id: #{organization.id} references an Endpoint endpoint_id_ref that is not contained in + this bundle. )) end end diff --git a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb index 3a574fd..42929f7 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_bundle_test.rb @@ -88,16 +88,22 @@ def limit_bundle_entries(resource_validation_limit, bundle_resource) new_entries end + def regex_match?(resource_id, reference) + return false if resource_id.blank? + + %r{#{resource_id}(?:/[^\/]*|\|[^\/]*)*/?$}.match?(reference) + end + def find_parent_organization_entry(organization_entries, org_reference) organization_entries - .find { |parent_org_entry| org_reference.include? parent_org_entry.resource.id } + .find { |parent_org_entry| regex_match?(parent_org_entry.resource.id, org_reference) } end def find_referenced_endpoints(organization_endpoints, endpoint_entries) endpoints = [] organization_endpoints.each do |endpoint_ref| found_endpoint = endpoint_entries.find do |endpoint_entry| - endpoint_ref.reference.include?(endpoint_entry.resource.id) + regex_match?(endpoint_entry.resource.id, endpoint_ref.reference) end endpoints.append(found_endpoint) if found_endpoint.present? end diff --git a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb index 8eeb6e8..28d550b 100644 --- a/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb +++ b/lib/smart_app_launch/smart_access_brands_validate_endpoints_test.rb @@ -10,6 +10,12 @@ class SMARTAccessBrandsValidateEndpoints < Inferno::Test by checking if it is referenced by at least 1 Organization resource. ) + def regex_match?(resource_id, reference) + return false if resource_id.blank? + + %r{#{resource_id}(?:/[^\/]*|\|[^\/]*)*/?$}.match?(reference) + end + def find_referenced_org(bundle_resource, endpoint_id) bundle_resource .entry @@ -18,7 +24,7 @@ def find_referenced_org(bundle_resource, endpoint_id) .map(&:endpoint) .flatten .map(&:reference) - .select { |reference| reference.include? endpoint_id } + .select { |reference| regex_match?(endpoint_id, reference) } end def skip_message diff --git a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb index 5aa3e59..aacfacb 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_brands_test_spec.rb @@ -176,7 +176,7 @@ def run(runnable, inputs = {}) expect(result.result).to eq('fail') expect(entity_result_message.message).to match( - 'Organization with id: examplehospital references an Endpoint that is not contained' + 'Organization with id: examplehospital references an Endpoint' ) expect(validation_request).to have_been_made.times(2) end diff --git a/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb b/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb index ca23b64..1f344cd 100644 --- a/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb +++ b/spec/smart_app_launch/smart_access_brands_validate_bundle_test_spec.rb @@ -96,6 +96,14 @@ def run(runnable, inputs = {}) expect(result.result).to eq('pass') end + it 'passes if a valid User Access Brands Bundle was received and resource_validation_limit input is entered' do + create_user_access_brands_request(body: smart_access_brands_bundle) + + result = run(test, resource_validation_limit: 2) + + expect(result.result).to eq('pass') + end + it 'skips if no User Access Brands Bundle requests were made' do result = run(test)