Move client logic to SdrClient

Keep it out of the controller

app/controllers/sdr_controller.rb

@@ -2,56 +2,40 @@
 
 class SdrController < ApplicationController
   def cm_inv_diff
-    unless %w(all shelve preserve publish).include?(params[:subset].to_s)
+    unless %w(all shelve preserve publish).include?(params[:subset])
       render status: :bad_request, plain: "Invalid subset value: #{params[:subset]}"
       return
     end
 
     request.body.rewind
     current_content = request.body.read
 
-    query_params = { subset: params[:subset].to_s }
-    query_params[:version] = params[:version].to_s unless params[:version].nil?
-    query_string = URI.encode_www_form(query_params)
-    path = "/objects/#{params[:druid]}/cm-inv-diff"
-    uri = sdr_uri(path)
-    sdr_response = sdr_conn(uri).post("#{uri.path}?#{query_string}", current_content, 'Content-Type' => 'application/xml')
-
+    sdr_response = sdr_client.content_diff(current_content: current_content, subset: params[:subset], version: params[:version])
     proxy_faraday_response(sdr_response)
   end
 
   def ds_manifest
-    proxy_faraday_response(sdr_get("/objects/#{params[:druid]}/manifest/#{params[:dsname]}"))
+    sdr_response = sdr_client.manifest(ds_name: params[:dsname])
+    proxy_faraday_response(sdr_response)
   end
 
   def ds_metadata
-    proxy_faraday_response(sdr_get("/objects/#{params[:druid]}/metadata/#{params[:dsname]}"))
+    sdr_response = sdr_client.metadata(ds_name: params[:dsname])
+    proxy_faraday_response(sdr_response)
   end
 
   def current_version
-    proxy_faraday_response(sdr_get("/objects/#{params[:druid]}/current_version"))
+    proxy_faraday_response(sdr_client.current_version)
   end
 
   def file_content
-    query_string = URI.encode_www_form(version: params[:version].to_s)
-    encoded_filename = URI.encode(params[:filename])
-    proxy_faraday_response(sdr_get("/objects/#{params[:druid]}/content/#{encoded_filename}?#{query_string}"))
+    sdr_response = sdr_client.file_content(version: params[:version], filename: params[:filename])
+    proxy_faraday_response(sdr_response)
   end
 
   private
 
-  def sdr_uri(path)
-    URI("#{Settings.sdr_url}#{path}")
-  end
-
-  def sdr_conn(uri)
-    Faraday.new("#{uri.scheme}://#{uri.host}").tap do |conn|
-      conn.basic_auth(uri.user, uri.password)
-    end
-  end
-
-  def sdr_get(path)
-    uri = sdr_uri(path)
-    sdr_conn(uri).get("#{uri.path}?#{uri.query}")
+  def sdr_client
+    SdrClient.new(params[:druid])
   end
 end

app/services/sdr_client.rb

@@ -5,10 +5,34 @@ class SdrClient
   # @raises [Dor::Exception] if SDR doesn't know about the object (i.e. 404 response code)
   # @raises [StandardError] if the response from SDR can't be parsed
   def self.current_version(druid)
-    uri = URI("#{Settings.sdr_url}/objects/#{druid}/current_version")
-    conn = Faraday.new "#{uri.scheme}://#{uri.host}"
-    conn.basic_auth(uri.user, uri.password)
-    response = conn.get uri.path
+    new(druid).current_version(parsed: true)
+  end
+
+  def initialize(druid)
+    @druid = druid
+  end
+
+  def content_diff(current_content:, subset:, version:)
+    query_params = { subset: subset }
+    query_params[:version] = version unless version.nil?
+    query_string = URI.encode_www_form(query_params)
+    path = "/objects/#{druid}/cm-inv-diff"
+    uri = sdr_uri(path)
+    sdr_conn(uri).post("#{uri.path}?#{query_string}", current_content, 'Content-Type' => 'application/xml')
+  end
+
+  def manifest(ds_name:)
+    sdr_get("/objects/#{druid}/manifest/#{ds_name}")
+  end
+
+  def metadata(ds_name:)
+    sdr_get("/objects/#{druid}/metadata/#{ds_name}")
+  end
+
+  def current_version(parsed: false)
+    path = "/objects/#{druid}/current_version"
+    response = sdr_get(path)
+    return response unless parsed
 
     if response.status == 404
       raise Dor::Exception, 'SDR is not yet answering queries about this object. ' \
@@ -21,7 +45,36 @@ def self.current_version(druid)
 
       return Integer(doc.text)
     rescue StandardError
-      raise "Unable to parse XML from SDR current_version API call.\n\turl: #{uri}\n\tstatus: #{response.status}\n\tbody: #{response.body}"
+      raise "Unable to parse XML from SDR current_version API call.\n\turl: #{sdr_uri(path)}\n\tstatus: #{response.status}\n\tbody: #{response.body}"
     end
   end
+
+  def file_content(version:, filename:)
+    query_string = URI.encode_www_form(version: version.to_s)
+    encoded_filename = CGI.escape(filename)
+    sdr_get("/objects/#{druid}/content/#{encoded_filename}?#{query_string}")
+  end
+
+  private
+
+  attr_reader :druid
+
+  def current_version_path
+    "/objects/#{druid}/current_version"
+  end
+
+  def sdr_uri(path)
+    URI("#{Settings.sdr_url}#{path}")
+  end
+
+  def sdr_conn(uri)
+    Faraday.new("#{uri.scheme}://#{uri.host}").tap do |conn|
+      conn.basic_auth(uri.user, uri.password)
+    end
+  end
+
+  def sdr_get(path)
+    uri = sdr_uri(path)
+    sdr_conn(uri).get("#{uri.path}?#{uri.query}")
+  end
 end

spec/controllers/sdr_controller_spec.rb

@@ -63,10 +63,10 @@
 
     context 'URI encoding' do
       let(:filename_with_spaces) { 'filename with spaces' }
-      let(:uri_encoded_filename) { URI.encode(filename_with_spaces) }
+      let(:cgi_escaped_filename) { CGI.escape(filename_with_spaces) }
 
       it 'handles file names with characters that need URI encoding' do
-        stub_request(:get, "http://sdr-services.example.com/sdr/objects/#{item.pid}/content/#{uri_encoded_filename}?version=#{item_version}")
+        stub_request(:get, "http://sdr-services.example.com/sdr/objects/#{item.pid}/content/#{cgi_escaped_filename}?version=#{item_version}")
           .with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' })
           .to_return(body: mock_response_txt, headers: { 'Content-Type' => content_type_txt })