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

@@ -36,4 +36,4 @@ group :test do
   gem 'simplecov-cobertura'
   gem 'webmock'
   gem 'factory_bot'
-end
+end

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

package.json

@@ -101,4 +101,4 @@
   "keywords": [],
   "author": "",
   "license": "ISC"
-}
+}

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' })