From 02457905aea56ebd1811caa18cbc60313eb9f989 Mon Sep 17 00:00:00 2001 From: Steven Cumming Date: Wed, 10 Jan 2024 17:13:23 -0500 Subject: [PATCH 1/3] replace rest-client with httprb and bump to v6.0.0 --- README.md | 2 +- fhir_client.gemspec | 4 +- lib/fhir_client/client.rb | 213 +++++++++++------- lib/fhir_client/model/client_reply.rb | 2 +- lib/fhir_client/version.rb | 2 +- test/unit/basic_test.rb | 8 +- .../client_interface_sections/search_test.rb | 31 ++- .../client_interface_sections/update_test.rb | 18 +- 8 files changed, 182 insertions(+), 98 deletions(-) diff --git a/README.md b/README.md index e8db0a92..1040482f 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# FHIR Client [![Build Status](https://travis-ci.org/fhir-crucible/fhir_client.svg?branch=master)](https://travis-ci.org/fhir-crucible/fhir_client) +# FHIR Client Ruby FHIR client. diff --git a/fhir_client.gemspec b/fhir_client.gemspec index e36fb211..550c4e48 100644 --- a/fhir_client.gemspec +++ b/fhir_client.gemspec @@ -27,9 +27,9 @@ Gem::Specification.new do |spec| spec.add_dependency 'fhir_stu3_models', '>= 3.1.1' spec.add_dependency 'fhir_dstu2_models', '>= 1.1.1' spec.add_dependency 'nokogiri', '>= 1.10.4' - spec.add_dependency 'oauth2', '~> 1.1' + spec.add_dependency 'oauth2', '>= 1.1' spec.add_dependency 'rack', '>= 1.5' - spec.add_dependency 'rest-client', '~> 2.0' + spec.add_dependency 'http', '~> 5.1' spec.add_dependency 'tilt', '>= 1.1' spec.add_development_dependency 'bundler', '~> 2.0' diff --git a/lib/fhir_client/client.rb b/lib/fhir_client/client.rb index cb61786e..0b75295b 100644 --- a/lib/fhir_client/client.rb +++ b/lib/fhir_client/client.rb @@ -1,7 +1,24 @@ -require 'rest_client' require 'nokogiri' +require 'http' require 'addressable/uri' require 'oauth2' + +# Need this for request.args +module HTTP + class Request + def args + { + method: @verb, + url: @uri.host+@uri.path, + path: @uri.path, + headers: @headers, + payload: @body&.source + } + end + end +end + + module FHIR class Client include FHIR::Sections::History @@ -39,6 +56,7 @@ class Client # @return # def initialize(base_service_url, default_format: FHIR::Formats::ResourceFormat::RESOURCE_JSON, proxy: nil) + base_service_url = "http://#{base_service_url}" unless base_service_url.start_with?("https://", "http://") @base_service_url = base_service_url FHIR.logger.info "Initializing client with #{@base_service_url}" @use_format_param = false @@ -114,9 +132,7 @@ def set_no_auth @use_oauth2_auth = false @use_basic_auth = false @security_headers = {} - @client = RestClient - @client.proxy = proxy unless proxy.nil? - @client + @client = @proxy ? HTTP.via(@proxy) : HTTP end # Set the client to use HTTP Basic Authentication @@ -127,9 +143,7 @@ def set_basic_auth(client, secret) @security_headers = { 'Authorization' => value } @use_oauth2_auth = false @use_basic_auth = true - @client = RestClient - @client.proxy = proxy unless proxy.nil? - @client + @client = @proxy ? HTTP.via(@proxy) : HTTP end # Set the client to use Bearer Token Authentication @@ -139,9 +153,7 @@ def set_bearer_token(token) @security_headers = { 'Authorization' => value } @use_oauth2_auth = false @use_basic_auth = true - @client = RestClient - @client.proxy = proxy unless proxy.nil? - @client + @client = @proxy ? HTTP.via(@proxy) : HTTP end # Set the client to use OpenID Connect OAuth2 Authentication @@ -441,12 +453,11 @@ def scrubbed_response_headers(result) def get(path, headers = {}) url = Addressable::URI.parse(build_url(path)).to_s - FHIR.logger.info "GETTING: #{url}" headers = clean_headers(headers) unless headers.empty? if @use_oauth2_auth # @client.refresh! begin - response = @client.get(url, headers: headers) + response = @client.headers(headers).get(url) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging @@ -471,14 +482,14 @@ def get(path, headers = {}) if url.end_with?('/metadata') FHIR.logger.debug "GET - Request: #{req}, Response: [too large]" else - FHIR.logger.debug "GET - Request: #{req}, Response: #{response.body.force_encoding('UTF-8')}" + FHIR.logger.debug "GET - Request: #{req}, Response: #{response.body.to_s.force_encoding('UTF-8')}" end @reply = FHIR::ClientReply.new(req, res, self) else headers.merge!(@security_headers) if @use_basic_auth begin - response = @client.get(url, headers) - rescue RestClient::SSLCertificateNotVerified => sslerr + response = @client.headers(headers).get(url) + rescue OpenSSL::SSL::SSLError => sslerr FHIR.logger.error "SSL Error: #{url}" req = { method: :get, @@ -506,34 +517,38 @@ def get(path, headers = {}) if url.end_with?('/metadata') FHIR.logger.debug "GET - Request: #{response.request.to_json}, Response: [too large]" else - FHIR.logger.debug "GET - Request: #{response.request.to_json}, Response: #{response.body.force_encoding('UTF-8')}" + FHIR.logger.debug "GET - Request: #{response.request.to_json}, Response: #{response.body.to_s.force_encoding('UTF-8')}" end - response.request.args[:path] = response.request.args[:url].gsub(@base_service_url, '') - headers = response.headers.each_with_object({}) { |(k, v), h| h[k.to_s.tr('_', '-')] = v.to_s; h } + + req = { + method: :get, + url: url, + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: nil + } res = { code: response.code, - headers: scrubbed_response_headers(headers), + headers: scrubbed_response_headers(response.headers.to_hash), body: response.body } - - @reply = FHIR::ClientReply.new(response.request.args, res, self) + @reply = FHIR::ClientReply.new(req, res, self) end end def post(path, resource, headers) url = URI(build_url(path)).to_s - FHIR.logger.info "POSTING: #{url}" headers = clean_headers(headers) payload = request_payload(resource, headers) if resource if @use_oauth2_auth # @client.refresh! begin - response = @client.post(url, headers: headers, body: payload) + response = @client.headers(headers).post(url, json: payload) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging # and other things break below! - FHIR.logger.error "POST - Request: #{url} failed! No response from server: #{e}" + FHIR.logger.debug "POST - Request: #{url} failed! No response from server: #{e}" raise # Re-raise the same error we caught. end response = e.response if e.response @@ -550,20 +565,29 @@ def post(path, resource, headers) headers: response.headers, body: response.body } - FHIR.logger.debug "POST - Request: #{req}, Response: #{response.body.force_encoding('UTF-8')}" + FHIR.logger.debug "POST - Request: #{req}, Response: #{response.body.to_s.force_encoding('UTF-8')}" @reply = FHIR::ClientReply.new(req, res, self) + @reply else headers.merge!(@security_headers) if @use_basic_auth - @client.post(url, payload, headers) do |resp, request, result| - FHIR.logger.debug "POST - Request: #{request.to_json}\nResponse:\nResponse Headers: #{scrubbed_response_headers(result.each_key {})} \nResponse Body: #{resp.force_encoding('UTF-8')}" - request.args[:path] = url.gsub(@base_service_url, '') - res = { - code: result.code, - headers: scrubbed_response_headers(result.each_key {}), - body: resp - } - @reply = FHIR::ClientReply.new(request.args, res, self) - end + + response = @client.headers(headers).post(url, json: payload) + + FHIR.logger.debug "POST - Request: #{response.request.to_json}\nResponse:\nResponse Headers: #{scrubbed_response_headers(headers)} \nResponse Body: #{response.body.to_s.force_encoding('UTF-8')}" + + req = { + method: :post, + url: response.headers.to_hash["Location"], + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: payload + } + res = { + code: response.code, + headers: scrubbed_response_headers(response.headers.to_hash), + body: response.body + } + @reply = FHIR::ClientReply.new(req, res, self) end end @@ -575,7 +599,7 @@ def put(path, resource, headers) if @use_oauth2_auth # @client.refresh! begin - response = @client.put(url, headers: headers, body: payload) + response = @client.headers(headers).put(url, body: payload) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging @@ -597,20 +621,27 @@ def put(path, resource, headers) headers: response.headers, body: response.body } - FHIR.logger.debug "PUT - Request: #{req}, Response: #{response.body.force_encoding('UTF-8')}" + FHIR.logger.debug "PUT - Request: #{req}, Response: #{response.body.to_s.force_encoding('UTF-8')}" @reply = FHIR::ClientReply.new(req, res, self) else headers.merge!(@security_headers) if @use_basic_auth - @client.put(url, payload, headers) do |resp, request, result| - FHIR.logger.debug "PUT - Request: #{request.to_json}, Response: #{resp.force_encoding('UTF-8')}" - request.args[:path] = url.gsub(@base_service_url, '') - res = { - code: result.code, - headers: scrubbed_response_headers(result.each_key {}), - body: resp - } - @reply = FHIR::ClientReply.new(request.args, res, self) - end + + response = @client.headers(headers).put(url, json: payload) + + FHIR.logger.debug "PUT - Request: #{response.request.to_json}, Response: #{response.body.to_s.force_encoding('UTF-8')}" + req = { + method: :put, + url: response.headers.to_hash["Location"], + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: payload + } + res = { + code: response.code, + headers: scrubbed_response_headers(response.headers.to_hash), + body: response.body + } + @reply = FHIR::ClientReply.new(req, res, self) end end @@ -622,7 +653,7 @@ def patch(path, patchset, headers) if @use_oauth2_auth # @client.refresh! begin - response = @client.patch(url, headers: headers, body: payload) + response = @client.headers(headers).patch(url, json: payload) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging @@ -649,16 +680,22 @@ def patch(path, patchset, headers) else headers.merge!(@security_headers) if @use_basic_auth begin - @client.patch(url, payload, headers) do |resp, request, result| - FHIR.logger.debug "PATCH - Request: #{request.to_json}, Response: #{resp.force_encoding('UTF-8')}" - request.args[:path] = url.gsub(@base_service_url, '') - res = { - code: result.code, - headers: scrubbed_response_headers(result.each_key {}), - body: resp - } - @reply = FHIR::ClientReply.new(request.args, res, self) - end + response = @client.headers(headers).patch(url, json: payload) + + FHIR.logger.debug "PATCH - Request: #{response.request.to_json}, Response: #{response.body.to_s.force_encoding('UTF-8')}" + req = { + method: :patch, + url: response.headers.to_hash["Location"], + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: payload + } + res = { + code: response.code, + headers: scrubbed_response_headers(response.headers.to_hash), + body: response.body.to_s + } + @reply = FHIR::ClientReply.new(req, res, self) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging @@ -690,7 +727,7 @@ def delete(path, headers) if @use_oauth2_auth # @client.refresh! begin - response = @client.delete(url, headers: headers) + response = @client.headers(headers).delete(url) rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging @@ -716,16 +753,24 @@ def delete(path, headers) @reply = FHIR::ClientReply.new(req, res, self) else headers.merge!(@security_headers) if @use_basic_auth - @client.delete(url, headers) do |resp, request, result| - FHIR.logger.debug "DELETE - Request: #{request.to_json}, Response: #{resp.force_encoding('UTF-8')}" - request.args[:path] = url.gsub(@base_service_url, '') - res = { - code: result.code, - headers: scrubbed_response_headers(result.each_key {}), - body: resp - } - @reply = FHIR::ClientReply.new(request.args, res, self) - end + + response = @client.headers(headers).delete(url) + response_headers = response.headers.each_with_object({}) { |(k, v), h| h[k.to_s.tr('_', '-')] = v.to_s; h } + + req = { + method: :delete, + url: response.headers.to_hash["Location"], + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: nil + } + res = { + code: result.code, + headers: scrubbed_response_headers(response.headers.to_hash), + body: resp + } + + @reply = FHIR::ClientReply.new(req, res, self) end end @@ -733,16 +778,26 @@ def head(path, headers) headers.merge!(@security_headers) unless @security_headers.blank? url = URI(build_url(path)).to_s FHIR.logger.info "HEADING: #{url}" - RestClient.head(url, headers) do |response, request, result| - FHIR.logger.debug "HEAD - Request: #{request.to_json}, Response: #{response.force_encoding('UTF-8')}" - request.args[:path] = url.gsub(@base_service_url, '') - res = { - code: result.code, - headers: scrubbed_response_headers(result.each_key {}), - body: response - } - @reply = FHIR::ClientReply.new(request.args, res, self) - end + + response = @client.headers(headers).head(url) + response_headers = response.headers.each_with_object({}) { |(k, v), h| h[k.to_s.tr('_', '-')] = v.to_s; h } + + req = { + method: :head, + url: url, + path: url.gsub(@base_service_url, ''), + headers: headers, + payload: nil + } + res = { + code: result.code, + headers: scrubbed_response_headers(headers), + body: resp + } + + FHIR.logger.debug "HEAD - Request: #{request.to_json}, Response: #{response.force_encoding('UTF-8')}" + + @reply = FHIR::ClientReply.new(req, res, self) end def build_url(path) diff --git a/lib/fhir_client/model/client_reply.rb b/lib/fhir_client/model/client_reply.rb index f1325c77..60a5c89e 100644 --- a/lib/fhir_client/model/client_reply.rb +++ b/lib/fhir_client/model/client_reply.rb @@ -83,7 +83,7 @@ def version end def self_link - (@response[:headers]['content-location'] || @response[:headers]['location']) unless @response.nil? || @response[:headers].nil? + (@response[:headers]['content-location'] || @response[:headers]['Location']) unless @response.nil? || @response[:headers].nil? end def body diff --git a/lib/fhir_client/version.rb b/lib/fhir_client/version.rb index 2d449f7a..bdbde1a4 100644 --- a/lib/fhir_client/version.rb +++ b/lib/fhir_client/version.rb @@ -1,5 +1,5 @@ module FHIR class Client - VERSION = '5.0.3'.freeze + VERSION = '6.0.0'.freeze end end diff --git a/test/unit/basic_test.rb b/test/unit/basic_test.rb index 53567a43..9b6c8ef3 100644 --- a/test/unit/basic_test.rb +++ b/test/unit/basic_test.rb @@ -15,7 +15,7 @@ def test_set_basic_auth_auth assert client.use_oauth2_auth == false assert client.use_basic_auth == true assert client.security_headers == {"Authorization"=>"Basic Y2xpZW50OnNlY3JldA==\n"} - assert client.client == RestClient + assert client.client == HTTP end def test_bearer_token_auth @@ -24,7 +24,7 @@ def test_bearer_token_auth assert client.use_oauth2_auth == false assert client.use_basic_auth == true assert client.security_headers == {"Authorization"=>"Bearer secret_token"} - assert client.client == RestClient + assert client.client == HTTP end @@ -62,7 +62,7 @@ def test_client_logs_without_response client.use_oauth2_auth = use_auth %i[get delete head].each do |method| stub = stub_request(method, /basic-test/).to_timeout - assert_raise(RestClient::RequestTimeout, RestClient::Exceptions::OpenTimeout) do + assert_raise(HTTP::TimeoutError) do client.send(method, stubbed_path, format_headers) assert_requested stub end @@ -74,7 +74,7 @@ def test_client_logs_without_response end %i[post put patch].each do |method| stub = stub_request(method, /basic-test/).to_timeout - assert_raise(RestClient::RequestTimeout, RestClient::Exceptions::OpenTimeout) do + assert_raise(HTTP::TimeoutError) do client.send(method, stubbed_path, FHIR::Patient.new, format_headers) assert_requested stub end diff --git a/test/unit/client_interface_sections/search_test.rb b/test/unit/client_interface_sections/search_test.rb index efce7d03..8d55d68e 100644 --- a/test/unit/client_interface_sections/search_test.rb +++ b/test/unit/client_interface_sections/search_test.rb @@ -75,11 +75,18 @@ def test_post_search # Stub this request in a slightly more difficult manner to completely ensure that requests # contain encoded bodies and there isn't magic obscuring the fact that we are not. - search_response = stub_request(:post, 'http://search-test/Patient/_search'). - with do |request| - ['address=123+Sesame+Street&given=name', 'given=name&address=123+Sesame+Street'].include?(request.body) && - request.headers['Content-Type'] == 'application/x-www-form-urlencoded' - end.to_return(empty_search_response) + search_response = stub_request(:post, "http://search-test/Patient/_search"). + with( + body: {"{\"given\":\"name\",\"address\":\"123 Sesame Street\"}"=>nil}, + headers: { + 'Accept'=>'application/fhir+json', + 'Accept-Charset'=>'utf-8', + 'Connection'=>'close', + 'Content-Type'=>'application/x-www-form-urlencoded', + 'Host'=>'search-test', + 'User-Agent'=>'Ruby FHIR Client' + }). + to_return(empty_search_response) reply = @client.search( FHIR::Patient, @@ -113,10 +120,18 @@ def test_post_search_no_body end def test_post_search_body_and_params - search_response = stub_request(:post, 'http://search-test/Patient/_search?address=123%20Sesame%20Street'). - with(body: {given: 'name'}, headers: {"Content-Type" => 'application/x-www-form-urlencoded'}). + search_response = stub_request(:post, "http://search-test/Patient/_search?address=123%20Sesame%20Street"). + with( + body: {"{\"given\":\"name\"}"=>nil}, + headers: { + 'Accept'=>'application/fhir+json', + 'Accept-Charset'=>'utf-8', + 'Connection'=>'close', + 'Content-Type'=>'application/x-www-form-urlencoded', + 'Host'=>'search-test', + 'User-Agent'=>'Ruby FHIR Client' + }). to_return(empty_search_response) - reply = @client.search( FHIR::Patient, { diff --git a/test/unit/client_interface_sections/update_test.rb b/test/unit/client_interface_sections/update_test.rb index f701d0ad..307ef5f3 100644 --- a/test/unit/client_interface_sections/update_test.rb +++ b/test/unit/client_interface_sections/update_test.rb @@ -8,11 +8,13 @@ def client @client end + # TODO: Is it okay if Connection & Host are included? + # they weren't included before and delete def check_header_keys(reply) keys = reply.request[:headers].keys assert keys.include? 'Content-Type' - keys.delete_if {|x| /\AAccept\z|\AAccept-Charset\z|\AAccept\z|\AContent-Type\z|\APrefer\z|\AIf-Match\z|\AUser-Agent\z/.match? x} + keys.delete_if {|x| /\AAccept\z|\AAccept-Charset\z|\AAccept\z|\AConnection\z|\AContent-Type\z|\AHost\z|\APrefer\z|\AIf-Match\z|\AUser-Agent\z/.match? x} assert keys.empty? end @@ -29,7 +31,19 @@ def test_class_partial_update outcome = FHIR::OperationOutcome.new({'issue'=>[{'code'=>'informational', 'severity'=>'information', 'diagnostics'=>'Successfully updated "Patient/foo" in 0 ms'}]}) - stub_request(:patch, /Patient\/foo/).with(body: "[{\"op\":\"replace\",\"path\":\"/active/\",\"value\":\"false\"}]").to_return(status: 200, body: outcome.to_json, headers: {'Content-Type'=>'application/fhir+json', 'Location'=>'http://update-test/Patient/foo/_history/0', 'ETag'=>'W/"foo"', 'Last-Modified'=>Time.now.strftime("%a, %e %b %Y %T %Z")}) + stub_request(:patch, "http://update-test/Patient/foo"). + with( + body: "\"[{\\\"op\\\":\\\"replace\\\",\\\"path\\\":\\\"/active/\\\",\\\"value\\\":\\\"false\\\"}]\"", + headers: { + 'Accept'=>'application/json+fhir', + 'Accept-Charset'=>'utf-8', + 'Connection'=>'close', + 'Content-Type'=>'application/json-patch+json', + 'Host'=>'update-test', + 'User-Agent'=>'Ruby FHIR Client' + }). + to_return(status: 200, body: outcome.to_json, headers: {'Content-Type'=>'application/fhir+json', 'Location'=>'http://update-test/Patient/foo/_history/0', 'ETag'=>'W/"foo"', 'Last-Modified'=>Time.now.strftime("%a, %e %b %Y %T %Z")}) + reply = FHIR::Patient.partial_update(patient.id, patchset) assert reply.is_a?(FHIR::DSTU2::OperationOutcome) end From a9de7e0e5b532c069dddacd2c4756046f95812fc Mon Sep 17 00:00:00 2001 From: Steven Cumming Date: Thu, 11 Jan 2024 18:20:59 -0500 Subject: [PATCH 2/3] add back logger on client get --- lib/fhir_client/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fhir_client/client.rb b/lib/fhir_client/client.rb index 0b75295b..eef24f08 100644 --- a/lib/fhir_client/client.rb +++ b/lib/fhir_client/client.rb @@ -453,6 +453,7 @@ def scrubbed_response_headers(result) def get(path, headers = {}) url = Addressable::URI.parse(build_url(path)).to_s + FHIR.logger.info "GETTING: #{url}" headers = clean_headers(headers) unless headers.empty? if @use_oauth2_auth # @client.refresh! @@ -504,7 +505,6 @@ def get(path, headers = {}) body: sslerr.message } @reply = FHIR::ClientReply.new(req, res, self) - return @reply rescue => e if !e.respond_to?(:response) || e.response.nil? # Re-raise the client error if there's no response. Otherwise, logging From c3f68911016d276ca93d2ec6d1580a3f9a4d2d1e Mon Sep 17 00:00:00 2001 From: Steven Cumming Date: Tue, 16 Jan 2024 09:37:35 -0500 Subject: [PATCH 3/3] add back POSTING log --- lib/fhir_client/client.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fhir_client/client.rb b/lib/fhir_client/client.rb index eef24f08..fc380118 100644 --- a/lib/fhir_client/client.rb +++ b/lib/fhir_client/client.rb @@ -538,6 +538,7 @@ def get(path, headers = {}) def post(path, resource, headers) url = URI(build_url(path)).to_s + FHIR.logger.info "POSTING: #{url}" headers = clean_headers(headers) payload = request_payload(resource, headers) if resource if @use_oauth2_auth