Merge pull request #270 from sul-dlss/no-retry

Don't retry on request errors

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-05-13 10:30:48 -0500 using RuboCop version 0.65.0.
+# on 2019-05-16 12:52:58 -0500 using RuboCop version 0.65.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -36,7 +36,7 @@ Metrics/AbcSize:
 # Offense count: 3
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 139
+  Max: 137
 
 # Offense count: 4
 Metrics/CyclomaticComplexity:
@@ -47,7 +47,7 @@ Metrics/CyclomaticComplexity:
 Metrics/MethodLength:
   Max: 63
 
-# Offense count: 3
+# Offense count: 2
 Metrics/PerceivedComplexity:
   Max: 27
 
@@ -79,7 +79,7 @@ Naming/VariableName:
     - 'app/services/public_desc_metadata_service.rb'
     - 'spec/rails_helper.rb'
 
-# Offense count: 26
+# Offense count: 25
 RSpec/AnyInstance:
   Exclude:
     - 'spec/controllers/objects_controller_spec.rb'
@@ -167,22 +167,21 @@ RSpec/InstanceVariable:
     - 'spec/services/metadata_service_spec.rb'
     - 'spec/services/registration_service_spec.rb'
 
-# Offense count: 60
+# Offense count: 52
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
   Exclude:
     - 'spec/controllers/objects_controller_spec.rb'
     - 'spec/controllers/versions_controller_spec.rb'
     - 'spec/controllers/workflows_controller_spec.rb'
-    - 'spec/dor/goobi_spec.rb'
     - 'spec/dor/update_marc_record_service_spec.rb'
     - 'spec/services/metadata_service_spec.rb'
     - 'spec/services/public_desc_metadata_service_spec.rb'
     - 'spec/services/registration_service_spec.rb'
     - 'spec/services/version_service_spec.rb'
 
-# Offense count: 22
+# Offense count: 23
 RSpec/NestedGroups:
   Max: 5
 
@@ -222,11 +221,10 @@ RSpec/SubjectStub:
   Exclude:
     - 'spec/dor/update_marc_record_service_spec.rb'
 
-# Offense count: 12
+# Offense count: 11
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Exclude:
-    - 'spec/controllers/objects_controller_spec.rb'
     - 'spec/dor/service_item_spec.rb'
     - 'spec/dor/update_marc_record_service_spec.rb'
     - 'spec/services/metadata_service_spec.rb'
@@ -311,11 +309,10 @@ Style/GuardClause:
   Exclude:
     - 'config/initializers/okcomputer.rb'
 
-# Offense count: 2
+# Offense count: 1
 # Cop supports --auto-correct.
 Style/IfUnlessModifier:
   Exclude:
-    - 'app/models/dor/goobi.rb'
     - 'config/initializers/okcomputer.rb'
 
 # Offense count: 2

app/models/dor/goobi.rb

@@ -3,12 +3,18 @@
 module Dor
   # This class passes data to the Goobi server using a custom XML message that was developed by Intranda
   class Goobi < ServiceItem
+    # Any RestClient exception that is a 500 or greater
+    RETRIABLE_EXCEPTIONS = RestClient::Exceptions::EXCEPTIONS_MAP.select { |k, _v| k >= 500 }.values +
+                           [RestClient::RequestTimeout,
+                            RestClient::ServerBrokeConnection,
+                            RestClient::SSLCertificateNotVerified]
+
     def register
       with_retries(max_tries: Dor::Config.goobi.max_tries,
                    base_sleep_seconds: Dor::Config.goobi.base_sleep_seconds,
-                   max_sleep_seconds: Dor::Config.goobi.max_sleep_seconds) do |_attempt|
-        response = RestClient.post(Dor::Config.goobi.url, xml_request, content_type: 'application/xml')
-        response
+                   max_sleep_seconds: Dor::Config.goobi.max_sleep_seconds,
+                   rescue: RETRIABLE_EXCEPTIONS) do |_attempt|
+        RestClient.post(Dor::Config.goobi.url, xml_request, content_type: 'application/xml')
       end
     end
 

spec/controllers/objects_controller_spec.rb

@@ -133,9 +133,6 @@
         stub_request(:post, Dor::Config.goobi.url)
           .to_return(body: 'conflict',
                      status: 409)
-
-        # This just speeds up the test by avoiding retries
-        allow_any_instance_of(Dor::Goobi).to receive(:with_retries).and_yield(nil)
       end
 
       it 'returns the conflict code' do

spec/dor/goobi_spec.rb

@@ -127,12 +127,35 @@
     expect(goobi.xml_request).to include('<tags></tags>')
   end
 
-  # rubocop:disable RSpec/SubjectStub
-  it 'makes a call to the goobi server with the appropriate xml params' do
-    stub_request(:post, Dor::Config.goobi.url).to_return(body: '<somexml/>', headers: { 'Content-Type' => 'text/xml' })
-    expect(goobi).to receive(:xml_request)
-    response = goobi.register
-    expect(response.code).to eq(200)
+  describe '#register' do
+    subject(:response) { goobi.register }
+
+    before do
+      allow(goobi).to receive(:xml_request)
+    end
+
+    context 'with a successful response' do
+      before do
+        stub_request(:post, Dor::Config.goobi.url)
+          .to_return(body: '<somexml/>', headers: { 'Content-Type' => 'text/xml' }, status: 201)
+      end
+
+      it 'makes a call to the goobi server with the appropriate xml params' do
+        expect(response.code).to eq 201
+      end
+    end
+
+    context 'with a 409 response' do
+      before do
+        allow(RestClient).to receive(:post).and_call_original
+        stub_request(:post, Dor::Config.goobi.url)
+          .to_return(body: '<somexml/>', headers: { 'Content-Type' => 'text/xml' }, status: 409)
+      end
+
+      it 'makes a call to the goobi server with the appropriate xml params' do
+        expect { response }.to raise_error(RestClient::Conflict)
+        expect(RestClient).to have_received(:post).once # Don't retry request errors
+      end
+    end
   end
-  # rubocop:enable RSpec/SubjectStub
 end