From bf3dc1f2cc7cf3710d37990962de2250485ab3f2 Mon Sep 17 00:00:00 2001 From: 360dgries <139473729+360dgries@users.noreply.github.com> Date: Wed, 11 Oct 2023 14:27:55 -0400 Subject: [PATCH] FI-2015 GET requests added to fhir_operation (#380) * GET requests added to fhir_operation * added tests for helper function * deferred affectsState detection to test author * adjusted body_to_path to take one argument * Repeated parameter support, contract updating, let changes in spec testing * reorganization of spec tests and changes to checking parameter for primitive * added rspec test for unhandled REST methods --- Gemfile | 2 +- lib/inferno/dsl/fhir_client.rb | 48 +++++++++- package-lock.json | 2 +- package.json | 2 +- spec/inferno/dsl/fhir_client_spec.rb | 136 ++++++++++++++++++++++++++- 5 files changed, 178 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 824a968f7..4593ed536 100644 --- a/Gemfile +++ b/Gemfile @@ -36,4 +36,4 @@ group :test do gem 'simplecov-cobertura' gem 'webmock' gem 'factory_bot' -end +end \ No newline at end of file diff --git a/lib/inferno/dsl/fhir_client.rb b/lib/inferno/dsl/fhir_client.rb index 8eb728111..2f7cf93e4 100644 --- a/lib/inferno/dsl/fhir_client.rb +++ b/lib/inferno/dsl/fhir_client.rb @@ -60,26 +60,64 @@ def fhir_clients @fhir_clients ||= {} end + # Wrapper for checking if parameter contents are primitive + # + # @param param [FHIR::Parameters::Parameter] Parameter to be checked + # @private + def primitive_parameter?(param) + param_val = param.to_hash.except('name') + param_val.any? { |datatype, param_value| FHIR.primitive?(datatype: datatype[5..], value: param_value) } + end + + # Converts a list of FHIR Parameters into a query string for GET requests + # + # @param body [FHIR::Parameters] Must all be primitive if making GET request + # @private + def body_to_path(body) + query_hashes = body.parameter.map do |param| + if primitive_parameter?(param) + { param.name => param.to_hash.except('name').values[0] } + else + Inferno::Application[:logger].error "Cannot use GET request with non-primitive datatype #{param.name}" + raise ArgumentError, "Cannot use GET request with non-primitive datatype #{param.name}" + end + end + query_hashes.map(&:to_query).join('&') + end + # Perform a FHIR operation # # @note This is a placeholder method until the FHIR::Client supports - # general operations + # general operations. Note that while both POST and GET methods are allowed, + # GET is only allowed when the operation does not affect the server's state. + # See https://build.fhir.org/operationdefinition-definitions.html#OperationDefinition.affectsState + # + # @note Currently does not allow for repeated parameters if using GET # # @param path [String] - # @param body [FHIR::Parameters] + # @param body [FHIR::Parameters] Must all be primitive if making GET request # @param client [Symbol] # @param name [Symbol] Name for this request to allow it to be used by # other tests # @param headers [Hash] custom headers for this operation + # @param operation_method [Symbol] indicates which request type to use for the operation # @return [Inferno::Entities::Request] - def fhir_operation(path, body: nil, client: :default, name: nil, headers: {}) + def fhir_operation(path, body: nil, client: :default, name: nil, headers: {}, operation_method: :post) store_request_and_refresh_token(fhir_client(client), name) do tcp_exception_handler do operation_headers = fhir_client(client).fhir_headers operation_headers.merge!('Content-Type' => 'application/fhir+json') if body.present? operation_headers.merge!(headers) if headers.present? - - fhir_client(client).send(:post, path, body, operation_headers) + case operation_method + when :post + fhir_client(client).send(:post, path, body, operation_headers) + when :get + path = "#{path}?#{body_to_path(body)}" if body.present? + fhir_client(client).send(:get, path, operation_headers) + else + Inferno::Application[:logger].error "Cannot perform #{operation_method} requests, use GET or POST" + raise ArgumentError, "Cannot perform #{operation_method} requests, use GET or POST" + end end end end diff --git a/package-lock.json b/package-lock.json index 1add0b165..b2883ad61 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24701,4 +24701,4 @@ } } } -} +} \ No newline at end of file diff --git a/package.json b/package.json index 3beac0ced..e1aa144e1 100644 --- a/package.json +++ b/package.json @@ -101,4 +101,4 @@ "keywords": [], "author": "", "license": "ISC" -} +} \ No newline at end of file diff --git a/spec/inferno/dsl/fhir_client_spec.rb b/spec/inferno/dsl/fhir_client_spec.rb index 586a52d1f..65a5c1834 100644 --- a/spec/inferno/dsl/fhir_client_spec.rb +++ b/spec/inferno/dsl/fhir_client_spec.rb @@ -39,6 +39,65 @@ def test_session_id let(:default_client) { group.fhir_clients[:default] } let(:bundle) { FHIR::Bundle.new(type: 'history', entry: [{ resource: }]) } let(:session_data_repo) { Inferno::Repositories::SessionData.new } + let(:boolean_parameter) do + FHIR::Parameters::Parameter.new.tap do |param| + param.name = 'PARAM_BOOL' + param.valueBoolean = true + end + end + let(:ratio_parameter) do + FHIR::Parameters::Parameter.new.tap do |param| + param.name = 'PARAM_RATIO' + param.valueRatio = FHIR::Ratio.new.tap do |ratio| + ratio.numerator = FHIR::Quantity.new + ratio.denominator = FHIR::Quantity.new + end + end + end + let(:resource_parameter) do + FHIR::Parameters::Parameter.new.tap do |param| + param.name = 'PARAM_RESOURCE' + param.resource = FHIR::Patient.new + end + end + let(:body_with_two_primitives) do + FHIR::Parameters.new.tap do |body| + body.parameter = [ + boolean_parameter, + FHIR::Parameters::Parameter.new.tap do |param| + param.name = 'PARAM_STRING' + param.valueString = 'STRING' + end + ] + end + end + let(:body_with_repeated_parameters) do + FHIR::Parameters.new.tap do |body| + body.parameter = [ + boolean_parameter, + FHIR::Parameters::Parameter.new.tap do |param| + param.name = 'PARAM_BOOL' + param.valueBoolean = false + end + ] + end + end + let(:body_with_nonprimitive) do + FHIR::Parameters.new.tap do |body| + body.parameter = [ + boolean_parameter, + ratio_parameter + ] + end + end + let(:body_with_resource) do + FHIR::Parameters.new.tap do |body| + body.parameter = [ + boolean_parameter, + resource_parameter + ] + end + end describe '#fhir_client' do context 'without an argument' do @@ -76,23 +135,41 @@ def test_session_id end end + describe '#body_to_path' do + it 'handles repeated parameters' do + expected_body = [{ PARAM_BOOL: true }, { PARAM_BOOL: false }].map(&:to_query).join('&') + expect(group.body_to_path(body_with_repeated_parameters)).to eq(expected_body) + end + end + describe '#fhir_operation' do let(:path) { 'abc' } let(:stub_operation_request) do stub_request(:post, "#{base_url}/#{path}") .to_return(status: 200, body: resource.to_json) end + let(:stub_operation_get_request) do + stub_request(:get, "#{base_url}/#{path}") + .to_return(status: 200, body: resource.to_json) + end before do stub_operation_request + stub_operation_get_request end - it 'performs a get' do + it 'performs a post' do group.fhir_operation(path) expect(stub_operation_request).to have_been_made.once end + it 'performs a get' do + group.fhir_operation(path, operation_method: :get) + + expect(stub_operation_get_request).to have_been_made.once + end + it 'returns an Inferno::Entities::Request' do result = group.fhir_operation(path) @@ -106,6 +183,57 @@ def test_session_id expect(group.request).to eq(result) end + context 'with a body of parameters' do + it 'uses get when all parameters are primitive and GET specified' do + body = body_with_two_primitives + get_with_body_request_stub = + stub_request(:get, "#{base_url}/#{path}") + .with(query: { PARAM_BOOL: true, PARAM_STRING: 'STRING' }) + .to_return(status: 200, body: resource.to_json) + + group.fhir_operation(path, body:, operation_method: :get) + + expect(get_with_body_request_stub).to have_been_made.once + end + + # This test left for testing DoD of FI-2223 + # https://oncprojectracking.healthit.gov/support/browse/FI-2223 + # + # it 'correctly handles repeated parameters' do + # body = body_with_repeated_parameters + # get_with_body_request_stub = + # stub_request(:get, "#{base_url}/#{path}") + # .with(body: URI.encode_www_form({PARAM_BOOL: [true, false]})) + # .to_return(status: 200, body: resource.to_json) + # puts get_with_body_request_stub.to_s + + # group.fhir_operation(path, body:, operation_method: :get) + + # expect(get_with_body_request_stub).to have_been_made.once + # end + + it 'prevents get when parameters are non-primitive' do + body = body_with_nonprimitive + expect do + group.fhir_operation(path, body:, operation_method: :get) + end.to raise_error(ArgumentError, 'Cannot use GET request with non-primitive datatype PARAM_RATIO') + end + + it 'prevents get when parameters contain resources' do + body = body_with_resource + expect do + group.fhir_operation(path, body:, operation_method: :get) + end.to raise_error(ArgumentError, 'Cannot use GET request with non-primitive datatype PARAM_RESOURCE') + end + + it 'prevents REST methods other than GET and POST' do + body = body_with_two_primitives + expect do + group.fhir_operation(path, body:, operation_method: :put) + end.to raise_error(ArgumentError, 'Cannot perform put requests, use GET or POST') + end + end + context 'with the client parameter' do it 'uses that client' do other_url = 'http://www.example.com/fhir/r4' @@ -150,7 +278,7 @@ def test_session_id .to_return(status: 200, body: resource.to_json) end - it 'as custom only, performs a get' do + it 'as custom only, performs a post' do operation_request = stub_request(:post, "#{base_url}/#{path}") .with(headers: { 'CustomHeader' => 'CustomTest' }) @@ -161,7 +289,7 @@ def test_session_id expect(operation_request).to have_been_made.once end - it 'as default only, performs a get' do + it 'as default only, performs a post' do operation_request = stub_request(:post, "#{base_url}/#{path}") .with(headers: { 'DefaultHeader' => 'ClientHeader' }) @@ -173,7 +301,7 @@ def test_session_id expect(operation_request).to have_been_made.once end - it 'as both default and custom, performs a get' do + it 'as both default and custom, performs a post' do operation_request = stub_request(:post, "#{base_url}/#{path}") .with(headers: { 'DefaultHeader' => 'ClientHeader', 'CustomHeader' => 'CustomTest' })