From 913467e466861468bb8533dc7136c2eb5a64f733 Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Mon, 13 Nov 2023 09:55:23 -0500 Subject: [PATCH] FI-2233: New validation module for HL7 validator wrapper (#401) * FI-2233: step 1, clone Inferno::DSL::FHIRValidation to ::FHIRResourceValidation * reapply changed from PoC * update unit tests * wire everything up * added commented-out nginx conf for hl7 validator endpoint * review feedback * update tests with new validator filename format --- .env | 1 + config/nginx.background.conf | 18 ++ docker-compose.background.yml | 7 + lib/inferno/dsl.rb | 4 +- lib/inferno/dsl/fhir_resource_validation.rb | 298 ++++++++++++++++++ lib/inferno/entities/test_suite.rb | 1 + .../dsl/fhir_resource_validation_spec.rb | 198 ++++++++++++ 7 files changed, 526 insertions(+), 1 deletion(-) create mode 100644 lib/inferno/dsl/fhir_resource_validation.rb create mode 100644 spec/inferno/dsl/fhir_resource_validation_spec.rb diff --git a/.env b/.env index 99ea9dc05..fa0d6327f 100644 --- a/.env +++ b/.env @@ -2,6 +2,7 @@ VALIDATOR_URL=http://localhost/validatorapi REDIS_URL=redis://localhost:6379/0 G10_VALIDATOR_URL=http://localhost/validatorapi +FHIR_RESOURCE_VALIDATOR_URL=http://localhost/hl7validatorapi # The base path where inferno will be hosted. Leave blank to host inferno at the # root of its host. diff --git a/config/nginx.background.conf b/config/nginx.background.conf index f89747207..f69287fd1 100644 --- a/config/nginx.background.conf +++ b/config/nginx.background.conf @@ -82,5 +82,23 @@ http { proxy_pass http://validator_service:4567/; } + +# To enable the HL7 Validator Wrapper, both the section below and +# the section in docker-compose.background.yml need to be uncommented +# location /hl7validatorapi/ { +# proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; +# proxy_set_header Host $http_host; +# proxy_set_header X-Forwarded-Proto $scheme; +# proxy_set_header X-Forwarded-Port $server_port; +# proxy_redirect off; +# proxy_set_header Connection ''; +# proxy_http_version 1.1; +# chunked_transfer_encoding off; +# proxy_buffering off; +# proxy_cache off; +# proxy_read_timeout 600s; +# +# proxy_pass http://hl7_validator_service:3500/; +# } } } diff --git a/docker-compose.background.yml b/docker-compose.background.yml index 33e8ff536..c03de0b16 100644 --- a/docker-compose.background.yml +++ b/docker-compose.background.yml @@ -28,3 +28,10 @@ services: volumes: - ./data/redis:/data command: redis-server --appendonly yes + # To enable the HL7 Validator Wrapper, both the section below and + # the section in nginx.background.conf need to be uncommented + # hl7_validator_service: + # image: markiantorno/validator-wrapper + # # Update this path to match your directory structure + # volumes: + # - ./igs:/home/igs \ No newline at end of file diff --git a/lib/inferno/dsl.rb b/lib/inferno/dsl.rb index b50a9cb08..712425e87 100644 --- a/lib/inferno/dsl.rb +++ b/lib/inferno/dsl.rb @@ -1,6 +1,7 @@ require_relative 'dsl/assertions' require_relative 'dsl/fhir_client' require_relative 'dsl/fhir_validation' +require_relative 'dsl/fhir_resource_validation' require_relative 'dsl/http_client' require_relative 'dsl/results' require_relative 'dsl/runnable' @@ -13,7 +14,8 @@ module DSL FHIRClient, HTTPClient, Results, - FHIRValidation + FHIRValidation, + FHIRResourceValidation ].freeze EXTENDABLE_DSL_MODULES = [ diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb new file mode 100644 index 000000000..3d9d5bb03 --- /dev/null +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -0,0 +1,298 @@ +require_relative '../ext/fhir_models' +module Inferno + module DSL + # This module contains the methods needed to configure a validator to + # perform validation of FHIR resources. The actual validation is performed + # by an external FHIR validation service. Tests will typically rely on + # `assert_valid_resource` for validation rather than directly calling + # methods on a validator. + # + # @example + # + # validator do + # url 'http://example.com/validator' + # exclude_message { |message| message.type == 'info' } + # perform_additional_validation do |resource, profile_url| + # if something_is_wrong + # { type: 'error', message: 'something is wrong' } + # else + # { type: 'info', message: 'everything is ok' } + # end + # end + # end + module FHIRResourceValidation + def self.included(klass) + klass.extend ClassMethods + end + + class Validator + attr_reader :requirements + + # @private + def initialize(requirements = nil, &) + instance_eval(&) + @requirements = requirements + end + + # @private + def default_validator_url + ENV.fetch('FHIR_RESOURCE_VALIDATOR_URL') + end + + # Set the url of the validator service + # + # @param validator_url [String] + def url(validator_url = nil) + @url = validator_url if validator_url + + @url + end + + # Set the IGs that the validator will need to load + # Example: ["hl7.fhir.us.core#4.0.0"] + # @param igs [Array] + def igs(validator_igs = nil) + @igs = validator_igs if validator_igs + + @igs + end + + # @private + def additional_validations + @additional_validations ||= [] + end + + # Perform validation steps in addition to FHIR validation. + # + # @example + # perform_additional_validation do |resource, profile_url| + # if something_is_wrong + # { type: 'error', message: 'something is wrong' } + # else + # { type: 'info', message: 'everything is ok' } + # end + # end + # @yieldparam resource [FHIR::Model] the resource being validated + # @yieldparam profile_url [String] the profile the resource is being + # validated against + # @yieldreturn [Array>,Hash] The + # block should return a Hash or an Array of Hashes if any validation + # messages should be added. The Hash must contain two keys: `:type` + # and `:message`. `:type` can have a value of `'info'`, `'warning'`, + # or `'error'`. A type of `'error'` means the resource is invalid. + # `:message` contains the message string itself. + def perform_additional_validation(&block) + additional_validations << block + end + + # @private + def additional_validation_messages(resource, profile_url) + additional_validations + .flat_map { |step| step.call(resource, profile_url) } + .select { |message| message.is_a? Hash } + end + + # Filter out unwanted validation messages. Any messages for which the + # block evalutates to a truthy value will be excluded. + # + # @example + # validator do + # exclude_message { |message| message.type == 'info' } + # end + # @yieldparam message [Inferno::Entities::Message] + def exclude_message(&block) + @exclude_message = block if block_given? + @exclude_message + end + + # @see Inferno::DSL::FHIRResourceValidation#resource_is_valid? + def resource_is_valid?(resource, profile_url, runnable) + profile_url ||= FHIR::Definitions.resource_definition(resource.resourceType).url + + begin + response = call_validator(resource, profile_url) + rescue StandardError => e + # This could be a complete failure to connect (validator isn't running) + # or a timeout (validator took too long to respond). + runnable.add_message('error', e.message) + raise Inferno::Exceptions::ErrorInValidatorException, "Unable to connect to validator at #{url}." + end + outcome = operation_outcome_from_validator_response(response, runnable) + + message_hashes = message_hashes_from_outcome(outcome, resource, profile_url) + + message_hashes + .each { |message_hash| runnable.add_message(message_hash[:type], message_hash[:message]) } + + unless response.status == 200 + raise Inferno::Exceptions::ErrorInValidatorException, + 'Error occurred in the validator. Review Messages tab or validator service logs for more information.' + end + + message_hashes + .none? { |message_hash| message_hash[:type] == 'error' } + rescue Inferno::Exceptions::ErrorInValidatorException + raise + rescue StandardError => e + runnable.add_message('error', e.message) + raise Inferno::Exceptions::ErrorInValidatorException, + 'Error occurred in the validator. Review Messages tab or validator service logs for more information.' + end + + # @private + def filter_messages(message_hashes) + message_hashes.reject! { |message| exclude_message.call(Entities::Message.new(message)) } if exclude_message + end + + # @private + def message_hashes_from_outcome(outcome, resource, profile_url) + message_hashes = outcome.issue&.map { |issue| message_hash_from_issue(issue, resource) } || [] + + message_hashes.concat(additional_validation_messages(resource, profile_url)) + + filter_messages(message_hashes) + + message_hashes + end + + # @private + def message_hash_from_issue(issue, resource) + { + type: issue_severity(issue), + message: issue_message(issue, resource) + } + end + + # @private + def issue_severity(issue) + case issue.severity + when 'warning' + 'warning' + when 'information' + 'info' + else + 'error' + end + end + + # @private + def issue_message(issue, resource) + location = if issue.respond_to?(:expression) + issue.expression&.join(', ') + else + issue.location&.join(', ') + end + + location_prefix = resource.id ? "#{resource.resourceType}/#{resource.id}" : resource.resourceType + + "#{location_prefix}: #{location}: #{issue&.details&.text}" + end + + # @private + def wrap_resource_for_hl7_wrapper(resource, profile_url) + wrapped_resource = { + cliContext: { + # TODO: these should be configurable as well + sv: '4.0.1', + # displayWarnings: true, # -display-issues-are-warnings + # txServer: nil, # -tx n/a + igs: @igs || [], + # NOTE: this profile must be part of a loaded IG, + # otherwise the response is an HTTP 500 with no content + profiles: [profile_url] + }, + filesToValidate: [ + { + fileName: "#{resource.resourceType}/#{resource.id}.json", + fileContent: resource.to_json, + fileType: 'json' + } + ], + sessionId: @session_id + } + wrapped_resource.to_json + end + + # Post a resource to the validation service for validating. + # + # @param resource [FHIR::Model] + # @param profile_url [String] + # @return [String] the body of the validation response + def validate(resource, profile_url) + call_validator(resource, profile_url).body + end + + # @private + def call_validator(resource, profile_url) + request_body = wrap_resource_for_hl7_wrapper(resource, profile_url) + Faraday.new( + url, + request: { timeout: 600 } + ).post('validate', request_body, content_type: 'application/json') + end + + # @private + def operation_outcome_from_hl7_wrapped_response(response) + res = JSON.parse(response) + + @session_id = res['sessionId'] + + # assume for now that one resource -> one request + issues = res['outcomes'][0]['issues']&.map do |i| + { severity: i['level'].downcase, expression: i['location'], details: { text: i['message'] } } + end + # this is circuitous, ideally we would map this response directly to message_hashes + FHIR::OperationOutcome.new(issue: issues) + end + + # @private + def operation_outcome_from_validator_response(response, runnable) + if response.body.start_with? '{' + operation_outcome_from_hl7_wrapped_response(response.body) + else + runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") + raise Inferno::Exceptions::ErrorInValidatorException, + 'Validator response was an unexpected format. '\ + 'Review Messages tab or validator service logs for more information.' + end + end + end + + module ClassMethods + # @private + def fhir_validators + @fhir_validators ||= {} + end + + # Define a validator + # @example + # fhir_resource_validator do + # url 'http://example.com/validator' + # exclude_message { |message| message.type == 'info' } + # perform_additional_validation do |resource, profile_url| + # if something_is_wrong + # { type: 'error', message: 'something is wrong' } + # else + # { type: 'info', message: 'everything is ok' } + # end + # end + # end + # + # @param name [Symbol] the name of the validator, only needed if you are + # using multiple validators + # @param required_suite_options [Hash] suite options that must be + # selected in order to use this validator + def fhir_resource_validator(name = :default, required_suite_options: nil, &block) + current_validators = fhir_validators[name] || [] + + new_validator = Inferno::DSL::FHIRResourceValidation::Validator.new(required_suite_options, &block) + + current_validators.reject! { |validator| validator.requirements == required_suite_options } + current_validators << new_validator + + fhir_validators[name] = current_validators + end + end + end + end +end diff --git a/lib/inferno/entities/test_suite.rb b/lib/inferno/entities/test_suite.rb index 20af6108b..64f3225ca 100644 --- a/lib/inferno/entities/test_suite.rb +++ b/lib/inferno/entities/test_suite.rb @@ -13,6 +13,7 @@ class TestSuite extend DSL::FHIRClient::ClassMethods extend DSL::HTTPClient::ClassMethods include DSL::FHIRValidation + include DSL::FHIRResourceValidation class << self extend Forwardable diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb new file mode 100644 index 000000000..e039452bb --- /dev/null +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -0,0 +1,198 @@ +RSpec.describe Inferno::DSL::FHIRResourceValidation do + let(:validation_url) { 'http://example.com' } + let(:profile_url) { 'PROFILE' } + let(:validator) do + Inferno::DSL::FHIRResourceValidation::Validator.new do + url 'http://example.com' + end + end + let(:runnable) do + Inferno::Entities::Test.new + end + let(:resource) { FHIR::Patient.new } + + describe '#perform_additional_validation' do + before do + stub_request(:post, "#{validation_url}/validate") + .to_return(status: 200, body: { + outcomes: [{ + fileInfo: { + fileName: 'Patient/id.json', + fileContent: resource.to_json, + fileType: 'json' + }, + issues: [] + }], + sessionId: '5c7f903b-7e46-4e83-bdc9-0248ad2ba5f5' + }.to_json) + end + + context 'when the step does not return a hash' do + it 'does not add any messages to the runnable' do + validator.perform_additional_validation { 1 } + validator.perform_additional_validation { nil } + + expect(validator.resource_is_valid?(resource, profile_url, runnable)).to be(true) + expect(runnable.messages).to eq([]) + end + end + + context 'when the step returns an hash' do + let(:extra_message) do + { type: 'info', message: 'INFO' } + end + + it 'adds the messages to the runnable' do + validator.perform_additional_validation { extra_message } + + expect(validator.resource_is_valid?(resource, profile_url, runnable)).to be(true) + expect(runnable.messages).to eq([extra_message]) + end + end + + context 'when the step returns an array of hashes' do + let(:extra_messages) do + [ + { type: 'info', message: 'INFO' }, + { type: 'warning', message: 'WARNING' } + ] + end + + it 'adds the messages to the runnable' do + validator.perform_additional_validation { extra_messages } + + expect(validator.resource_is_valid?(resource, profile_url, runnable)).to be(true) + expect(runnable.messages).to eq(extra_messages) + end + end + + context 'when the step returns an error message' do + let(:extra_messages) do + [ + { type: 'info', message: 'INFO' }, + { type: 'error', message: 'ERROR' } + ] + end + + it 'fails validation' do + validator.perform_additional_validation { extra_messages } + + expect(validator.resource_is_valid?(resource, profile_url, runnable)).to be(false) + expect(runnable.messages).to eq(extra_messages) + end + end + end + + describe '#resource_is_valid?' do + let(:resource_string) do + { + resourceType: 'Patient', + id: '0000', + _gender: { + extension: [ + { + url: 'http: //hl7.org/fhir/StructureDefinition/data-absent-reason', + valueCode: 'unknown' + } + ] + } + }.to_json + end + let(:resource2) { FHIR.from_contents(resource_string) } + + let(:wrapped_resource_string) do + { + cliContext: { + sv: '4.0.1', + igs: [], + profiles: [profile_url] + }, + filesToValidate: [ + { + fileName: 'Patient/0000.json', + fileContent: resource2.to_json, + fileType: 'json' + } + ], + sessionId: nil + }.to_json + end + + context 'with invalid resource' do + let(:invalid_outcome) do + { + outcomes: [{ + fileInfo: { + fileName: 'Patient/0000.json', + fileContent: resource_string.to_json, + fileType: 'json' + }, + issues: [{ + source: 'InstanceValidator', + line: 4, + col: 4, + location: 'Patient.identifier[0]', + message: 'Identifier.system must be an absolute reference, not a local reference', + messageId: 'Type_Specific_Checks_DT_Identifier_System', + type: 'CODEINVALID', + level: 'ERROR', + html: 'Identifier.system must be an absolute reference, not a local reference', + display: 'ERROR: Patient.identifier[0]: Identifier.system must be an absolute reference, ', + error: true + }] + }], + sessionId: 'b8cf5547-1dc7-4714-a797-dc2347b93fe2' + }.to_json + end + + before do + stub_request(:post, "#{validation_url}/validate") + .with(body: wrapped_resource_string) + .to_return(status: 200, body: invalid_outcome) + end + + it 'includes resourceType/id in error message' do + result = validator.resource_is_valid?(resource2, profile_url, runnable) + + expect(result).to be(false) + expect(runnable.messages.first[:message]).to start_with("#{resource2.resourceType}/#{resource2.id}:") + end + end + + it 'throws ErrorInValidatorException for non-JSON response' do + stub_request(:post, "#{validation_url}/validate") + .with(body: wrapped_resource_string) + .to_return(status: 500, body: 'Internal Server Error') + + expect do + validator.resource_is_valid?(resource2, profile_url, runnable) + end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) + end + end + + describe '.find_validator' do + it 'finds the correct validator based on suite options' do + suite = OptionsSuite::Suite + + v1_validator = suite.find_validator(:default, ig_version: '1') + v2_validator = suite.find_validator(:default, ig_version: '2') + + expect(v1_validator.url).to eq('v1_validator') + expect(v2_validator.url).to eq('v2_validator') + end + end + + describe '#find_validator' do + it 'finds the correct validator based on suite options' do + test_class = OptionsSuite::Suite.groups.first.tests.first + v1_test = test_class.new(suite_options: { ig_version: '1' }) + v2_test = test_class.new(suite_options: { ig_version: '2' }) + + v1_validator = v1_test.find_validator(:default) + v2_validator = v2_test.find_validator(:default) + + expect(v1_validator.url).to eq('v1_validator') + expect(v2_validator.url).to eq('v2_validator') + end + end +end