From c899bc2ee7ed66514a0e151b798343102af147e1 Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Mon, 4 Dec 2023 11:57:14 -0500 Subject: [PATCH 1/4] FI-2236: support arbitrary fields on hl7 validator cliContext --- lib/inferno/dsl/fhir_resource_validation.rb | 37 +++++++---- .../dsl/fhir_resource_validation_spec.rb | 61 +++++++++++++++++++ 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index 86fbd0526..bfc79652f 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -49,13 +49,30 @@ def url(validator_url = nil) @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 + # @private + def cli_context + @cli_context ||= { + sv: '4.0.1', + igs: [], + doNative: false, + extensions: ['any'] + } + end - @igs + # @private + def method_missing(method_name, *args) + # Interpret any other method as setting a field on cliContext. + # Follow the same format as `url` here: + # only set the value if one is provided. + # args will be an empty array if no value is provided. + cli_context[method_name] = args[0] unless args.empty? + + cli_context[method_name] + end + + # @private + def respond_to_missing?(_method_name, _include_private = false) + true end # @private @@ -193,13 +210,7 @@ def issue_message(issue, resource) 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 + **cli_context, profiles: [profile_url] }, filesToValidate: [ diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index e039452bb..0759fac62 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -105,6 +105,8 @@ cliContext: { sv: '4.0.1', igs: [], + doNative: false, + extensions: ['any'], profiles: [profile_url] }, filesToValidate: [ @@ -170,6 +172,65 @@ end end + describe '.method_missing' do + it 'applies the correct settings to cli_context' do + v1 = Inferno::DSL::FHIRResourceValidation::Validator.new do + url validation_url + txServer nil + end + + v2 = Inferno::DSL::FHIRResourceValidation::Validator.new do + url validation_url + displayWarnings true + end + + expect(v1.cli_context.fetch(:txServer, :missing)).to eq(nil) + expect(v1.cli_context.fetch(:displayWarnings, :missing)).to eq(:missing) + expect(v1.txServer).to eq(nil) + + expect(v2.cli_context.fetch(:txServer, :missing)).to eq(:missing) + expect(v2.cli_context[:displayWarnings]).to eq(true) + expect(v2.displayWarnings).to eq(true) + end + + it 'uses the right cli_context when submitting the validation request' do + v3 = Inferno::DSL::FHIRResourceValidation::Validator.new do + url 'http://example.com' + txServer nil + displayWarnings true + igs ['hl7.fhir.us.core#1.0.1'] + end + + expected_request_body = { + cliContext: { + sv: '4.0.1', + igs: ['hl7.fhir.us.core#1.0.1'], + doNative: false, + extensions: ['any'], + txServer: nil, + displayWarnings: true, + profiles: [profile_url] + }, + filesToValidate: [ + { + fileName: 'Patient/.json', + fileContent: resource.to_json, + fileType: 'json' + } + ], + sessionId: nil + }.to_json + + stub_request(:post, 'http://example.com/validate') + .with(body: expected_request_body) + .to_return(status: 200, body: '{}') + + expect(v3.validate(resource, profile_url)).to eq('{}') + # if the request body doesn't match the stub, + # validate will throw an exception + end + end + describe '.find_validator' do it 'finds the correct validator based on suite options' do suite = OptionsSuite::Suite From 205700d0b327837fd76e0d224a028b7f4696aab3 Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Fri, 8 Dec 2023 15:24:02 -0500 Subject: [PATCH 2/4] Rework method_missing into a separate CliContext class --- lib/inferno/dsl/fhir_resource_validation.rb | 88 ++++++++++++++----- .../dsl/fhir_resource_validation_spec.rb | 38 ++++---- 2 files changed, 88 insertions(+), 38 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index bfc79652f..24d796925 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -49,30 +49,41 @@ def url(validator_url = nil) @url end - # @private - def cli_context - @cli_context ||= { - sv: '4.0.1', - igs: [], - doNative: false, - extensions: ['any'] - } - end - - # @private - def method_missing(method_name, *args) - # Interpret any other method as setting a field on cliContext. - # Follow the same format as `url` here: - # only set the value if one is provided. - # args will be an empty array if no value is provided. - cli_context[method_name] = args[0] unless args.empty? + # Set the IGs that the validator will need to load + # Example: ["hl7.fhir.us.core#4.0.0"] + # @param validator_igs [Array] + def igs(validator_igs = nil) + cli_context.igs(validator_igs) if validator_igs - cli_context[method_name] + cli_context.igs end - # @private - def respond_to_missing?(_method_name, _include_private = false) - true + # Set the cliContext used as part of each validation request. + # Fields may be passed as either a Hash or block. + # Note that all fields included here will be sent directly in requests, + # there is no check that the fields are correct. + # + # @example + # fhir_resource_validator do + # url 'http://example.com/validator' + # cli_context do + # noExtensibleBindingMessages true + # txServer nil + # end + # end + # + # @param definition [Hash] raw fields to set, optional + def cli_context(definition = nil, &) + if @cli_context + if definition + @cli_context.definition.merge!(definition) + elsif block_given? + @cli_context.instance_eval(&) + end + else + @cli_context = CliContext.new(definition || {}, &) + end + @cli_context end # @private @@ -210,7 +221,7 @@ def issue_message(issue, resource) def wrap_resource_for_hl7_wrapper(resource, profile_url) wrapped_resource = { cliContext: { - **cli_context, + **cli_context.definition, profiles: [profile_url] }, filesToValidate: [ @@ -270,6 +281,39 @@ def operation_outcome_from_validator_response(response, runnable) end end + # @private + class CliContext + attr_reader :definition + + CLICONTEXT_DEFAULTS = { + sv: '4.0.1', + doNative: false, + extensions: ['any'] + }.freeze + + # @private + def initialize(definition, &) + @definition = CLICONTEXT_DEFAULTS.merge(definition) + instance_eval(&) if block_given? + end + + # @private + def method_missing(method_name, *args) + # Interpret any other method as setting a field on cliContext. + # Follow the same format as `Validator.url` here: + # only set the value if one is provided. + # args will be an empty array if no value is provided. + definition[method_name] = args[0] unless args.empty? + + definition[method_name] + end + + # @private + def respond_to_missing?(_method_name, _include_private = false) + true + end + end + module ClassMethods # @private def fhir_validators diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index 0759fac62..e817f3eef 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -104,7 +104,6 @@ { cliContext: { sv: '4.0.1', - igs: [], doNative: false, extensions: ['any'], profiles: [profile_url] @@ -172,41 +171,48 @@ end end - describe '.method_missing' do + describe '.cli_context' do it 'applies the correct settings to cli_context' do v1 = Inferno::DSL::FHIRResourceValidation::Validator.new do - url validation_url - txServer nil + url 'http://example.com' + cli_context do + txServer nil + end end v2 = Inferno::DSL::FHIRResourceValidation::Validator.new do - url validation_url - displayWarnings true + url 'http://example.com' + cli_context({ + displayWarnings: true + }) end - expect(v1.cli_context.fetch(:txServer, :missing)).to eq(nil) - expect(v1.cli_context.fetch(:displayWarnings, :missing)).to eq(:missing) - expect(v1.txServer).to eq(nil) + expect(v1.cli_context.definition.fetch(:txServer, :missing)).to eq(nil) + expect(v1.cli_context.definition.fetch(:displayWarnings, :missing)).to eq(:missing) + expect(v1.cli_context.txServer).to eq(nil) - expect(v2.cli_context.fetch(:txServer, :missing)).to eq(:missing) - expect(v2.cli_context[:displayWarnings]).to eq(true) - expect(v2.displayWarnings).to eq(true) + expect(v2.cli_context.definition.fetch(:txServer, :missing)).to eq(:missing) + expect(v2.cli_context.definition[:displayWarnings]).to eq(true) + expect(v2.cli_context.displayWarnings).to eq(true) end it 'uses the right cli_context when submitting the validation request' do v3 = Inferno::DSL::FHIRResourceValidation::Validator.new do url 'http://example.com' - txServer nil - displayWarnings true igs ['hl7.fhir.us.core#1.0.1'] + cli_context do + txServer nil + displayWarnings true + doNative true + end end expected_request_body = { cliContext: { sv: '4.0.1', - igs: ['hl7.fhir.us.core#1.0.1'], - doNative: false, + doNative: true, extensions: ['any'], + igs: ['hl7.fhir.us.core#1.0.1'], txServer: nil, displayWarnings: true, profiles: [profile_url] From 4ce1db81ad8d9058472b7645d2d91b15cb3d50a7 Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Thu, 14 Dec 2023 11:24:39 -0500 Subject: [PATCH 3/4] review feedback --- lib/inferno/dsl/fhir_resource_validation.rb | 51 ++++++++++++++++--- .../dsl/fhir_resource_validation_spec.rb | 5 +- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index 24d796925..a23792f0a 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -50,10 +50,13 @@ def url(validator_url = nil) end # Set the IGs that the validator will need to load - # Example: ["hl7.fhir.us.core#4.0.0"] + # @example + # igs "hl7.fhir.us.core#4.0.0" + # @example + # igs("hl7.fhir.us.core#3.1.1", "hl7.fhir.us.core#6.0.0") # @param validator_igs [Array] - def igs(validator_igs = nil) - cli_context.igs(validator_igs) if validator_igs + def igs(*validator_igs) + cli_context(igs: validator_igs) if validator_igs cli_context.igs end @@ -72,11 +75,20 @@ def igs(validator_igs = nil) # end # end # + # @example + # fhir_resource_validator do + # url 'http://example.org/validator' + # cli_context({ + # noExtensibleBindingMessages: true, + # txServer: nil + # }) + # end + # # @param definition [Hash] raw fields to set, optional def cli_context(definition = nil, &) if @cli_context if definition - @cli_context.definition.merge!(definition) + merge_into_cli_context(definition) elsif block_given? @cli_context.instance_eval(&) end @@ -86,6 +98,21 @@ def cli_context(definition = nil, &) @cli_context end + # @private + def merge_into_cli_context(new_def) + @cli_context.definition.merge!(new_def) do |_key, old_val, new_val| + case old_val + when Array + # take the union of the 2 + old_val | new_val + when Hash + old_val.merge(new_val) + else + new_val + end + end + end + # @private def additional_validations @additional_validations ||= [] @@ -302,9 +329,21 @@ def method_missing(method_name, *args) # Interpret any other method as setting a field on cliContext. # Follow the same format as `Validator.url` here: # only set the value if one is provided. + # Array or Hash values will be merged if a value is already present. # args will be an empty array if no value is provided. - definition[method_name] = args[0] unless args.empty? - + unless args.empty? + new_val = args[0] + old_val = definition[method_name] + definition[method_name] = case old_val + when Array + # take the union of the 2 + old_val | new_val + when Hash + old_val.merge(new_val) + else + new_val + end + end definition[method_name] end diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index e817f3eef..a2691fd65 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -199,11 +199,12 @@ it 'uses the right cli_context when submitting the validation request' do v3 = Inferno::DSL::FHIRResourceValidation::Validator.new do url 'http://example.com' - igs ['hl7.fhir.us.core#1.0.1'] + igs 'hl7.fhir.us.core#1.0.1' cli_context do txServer nil displayWarnings true doNative true + igs ['hl7.fhir.us.core#3.1.1'] end end @@ -212,7 +213,7 @@ sv: '4.0.1', doNative: true, extensions: ['any'], - igs: ['hl7.fhir.us.core#1.0.1'], + igs: ['hl7.fhir.us.core#1.0.1', 'hl7.fhir.us.core#3.1.1'], txServer: nil, displayWarnings: true, profiles: [profile_url] From dedac17dcb4b6f3463ee2a1d5563ad3fd8c3a5fb Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Fri, 15 Dec 2023 09:09:15 -0500 Subject: [PATCH 4/4] Additional review changes, revert merging logic --- lib/inferno/dsl/fhir_resource_validation.rb | 37 +++---------------- .../dsl/fhir_resource_validation_spec.rb | 23 +++++++++--- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index a23792f0a..425866524 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -88,7 +88,7 @@ def igs(*validator_igs) def cli_context(definition = nil, &) if @cli_context if definition - merge_into_cli_context(definition) + @cli_context.definition.merge!(definition.deep_symbolize_keys) elsif block_given? @cli_context.instance_eval(&) end @@ -98,21 +98,6 @@ def cli_context(definition = nil, &) @cli_context end - # @private - def merge_into_cli_context(new_def) - @cli_context.definition.merge!(new_def) do |_key, old_val, new_val| - case old_val - when Array - # take the union of the 2 - old_val | new_val - when Hash - old_val.merge(new_val) - else - new_val - end - end - end - # @private def additional_validations @additional_validations ||= [] @@ -254,7 +239,7 @@ def wrap_resource_for_hl7_wrapper(resource, profile_url) filesToValidate: [ { fileName: "#{resource.resourceType}/#{resource.id}.json", - fileContent: resource.to_json, + fileContent: resource.source_contents, fileType: 'json' } ], @@ -320,7 +305,7 @@ class CliContext # @private def initialize(definition, &) - @definition = CLICONTEXT_DEFAULTS.merge(definition) + @definition = CLICONTEXT_DEFAULTS.merge(definition.deep_symbolize_keys) instance_eval(&) if block_given? end @@ -329,21 +314,9 @@ def method_missing(method_name, *args) # Interpret any other method as setting a field on cliContext. # Follow the same format as `Validator.url` here: # only set the value if one is provided. - # Array or Hash values will be merged if a value is already present. # args will be an empty array if no value is provided. - unless args.empty? - new_val = args[0] - old_val = definition[method_name] - definition[method_name] = case old_val - when Array - # take the union of the 2 - old_val | new_val - when Hash - old_val.merge(new_val) - else - new_val - end - end + definition[method_name] = args[0] unless args.empty? + definition[method_name] end diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index a2691fd65..98c1b3893 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -18,7 +18,7 @@ outcomes: [{ fileInfo: { fileName: 'Patient/id.json', - fileContent: resource.to_json, + fileContent: resource.source_contents, fileType: 'json' }, issues: [] @@ -111,7 +111,7 @@ filesToValidate: [ { fileName: 'Patient/0000.json', - fileContent: resource2.to_json, + fileContent: resource2.source_contents, fileType: 'json' } ], @@ -187,6 +187,14 @@ }) end + v3 = Inferno::DSL::FHIRResourceValidation::Validator.new do + url 'http://example.com' + cli_context({ + 'igs' => ['hl7.fhir.us.core#1.0.1'], + 'extensions' => [] + }) + end + expect(v1.cli_context.definition.fetch(:txServer, :missing)).to eq(nil) expect(v1.cli_context.definition.fetch(:displayWarnings, :missing)).to eq(:missing) expect(v1.cli_context.txServer).to eq(nil) @@ -194,10 +202,13 @@ expect(v2.cli_context.definition.fetch(:txServer, :missing)).to eq(:missing) expect(v2.cli_context.definition[:displayWarnings]).to eq(true) expect(v2.cli_context.displayWarnings).to eq(true) + + expect(v3.cli_context.igs).to eq(['hl7.fhir.us.core#1.0.1']) + expect(v3.cli_context.extensions).to eq([]) end it 'uses the right cli_context when submitting the validation request' do - v3 = Inferno::DSL::FHIRResourceValidation::Validator.new do + v4 = Inferno::DSL::FHIRResourceValidation::Validator.new do url 'http://example.com' igs 'hl7.fhir.us.core#1.0.1' cli_context do @@ -213,7 +224,7 @@ sv: '4.0.1', doNative: true, extensions: ['any'], - igs: ['hl7.fhir.us.core#1.0.1', 'hl7.fhir.us.core#3.1.1'], + igs: ['hl7.fhir.us.core#3.1.1'], txServer: nil, displayWarnings: true, profiles: [profile_url] @@ -221,7 +232,7 @@ filesToValidate: [ { fileName: 'Patient/.json', - fileContent: resource.to_json, + fileContent: resource.source_contents, fileType: 'json' } ], @@ -232,7 +243,7 @@ .with(body: expected_request_body) .to_return(status: 200, body: '{}') - expect(v3.validate(resource, profile_url)).to eq('{}') + expect(v4.validate(resource, profile_url)).to eq('{}') # if the request body doesn't match the stub, # validate will throw an exception end