Use .save! instead of custom method to save db objects

.rubocop.yml

@@ -132,10 +132,6 @@ Rails/Output:
     - 'lib/audit/catalog_to_moab.rb' # use puts statements for following progress of long job
     - 'lib/audit/moab_to_catalog.rb' # use puts statements for following progress of long job
 
-Rails/SkipsModelValidations:
-  Exclude:
-    - 'app/services/preserved_object_handler.rb' # we're using touch on purpose
-
 # --- RSpec ---
 
 RSpec/BeforeAfterAll:

app/services/preserved_object_handler.rb

@@ -82,7 +82,7 @@ def check_existence
             if moab_validation_errors.empty?
               pres_copy.upd_audstamps_version_size(ran_moab_validation?, incoming_version, incoming_size)
               pres_object.current_version = incoming_version
-              update_db_object(pres_object)
+              pres_object.save!
             else
               update_status(pres_copy, PreservedCopy::INVALID_MOAB_STATUS)
             end
@@ -92,7 +92,7 @@ def check_existence
             handler_results.add_result(PreservedObjectHandlerResults::ARG_VERSION_LESS_THAN_DB_OBJECT, pres_object.class.name)
           end
           pres_copy.update_audit_timestamps(ran_moab_validation?, true)
-          update_db_object(pres_copy)
+          pres_copy.save!
         end
         handler_results.remove_db_updated_results unless transaction_ok
       elsif endpoint.endpoint_type.endpoint_class == 'archive'
@@ -231,9 +231,9 @@ def update_online_version(status=nil, set_status_to_unexp_version=false)
 
         pres_copy.upd_audstamps_version_size(ran_moab_validation?, incoming_version, incoming_size)
         update_status(pres_copy, status) if status && ran_moab_validation?
-        update_db_object(pres_copy)
+        pres_copy.save!
         pres_object.current_version = incoming_version
-        update_db_object(pres_object)
+        pres_object.save!
       else
         if set_status_to_unexp_version
           status = PreservedCopy::EXPECTED_VERS_NOT_FOUND_ON_STORAGE_STATUS
@@ -262,7 +262,7 @@ def update_pc_invalid_moab
       # FIXME: what if there is more than one associated pres_copy?
       update_status(pres_copy, PreservedCopy::INVALID_MOAB_STATUS)
       pres_copy.update_audit_timestamps(ran_moab_validation?, false)
-      update_db_object(pres_copy)
+      pres_copy.save!
     end
     handler_results.remove_db_updated_results unless transaction_ok
   end
@@ -297,7 +297,7 @@ def update_pc_unexpected_version(pres_copy, pres_object, new_status)
 
     update_status(pres_copy, new_status) if new_status
     pres_copy.update_audit_timestamps(ran_moab_validation?, true)
-    update_db_object(pres_copy)
+    pres_copy.save!
   end
 
   # shameless green implementation
@@ -318,7 +318,7 @@ def confirm_online_version
         handler_results.add_result(PreservedObjectHandlerResults::UNEXPECTED_VERSION, pres_copy.class.name)
       end
       pres_copy.update_audit_timestamps(ran_moab_validation?, true)
-      update_db_object(pres_copy)
+      pres_copy.save!
     end
     handler_results.remove_db_updated_results unless transaction_ok
   end
@@ -360,19 +360,6 @@ def update_status(preserved_copy, new_status)
     end
   end
 
-  # TODO: this may need reworking if we need to distinguish db timestamp updates when
-  #   version matched vs. incoming version less than db object
-  def update_db_object(db_object)
-    if db_object.changed?
-      db_object.save!
-      handler_results.add_result(PreservedObjectHandlerResults::UPDATED_DB_OBJECT, db_object.class.name)
-    else
-      # FIXME: we may not want to do this, but instead to update specific timestamp for check
-      db_object.touch
-      handler_results.add_result(PreservedObjectHandlerResults::UPDATED_DB_OBJECT_TIMESTAMP_ONLY, db_object.class.name)
-    end
-  end
-
   def version_string_to_int(val)
     result = string_to_int(val)
     return result if result.instance_of?(Integer)

app/services/preserved_object_handler_results.rb

@@ -10,25 +10,21 @@ class PreservedObjectHandlerResults
   VERSION_MATCHES = 2
   ARG_VERSION_GREATER_THAN_DB_OBJECT = 3
   ARG_VERSION_LESS_THAN_DB_OBJECT = 4
-  UPDATED_DB_OBJECT = 5
-  UPDATED_DB_OBJECT_TIMESTAMP_ONLY = 6
-  CREATED_NEW_OBJECT = 7
-  DB_UPDATE_FAILED = 8
-  OBJECT_ALREADY_EXISTS = 9
-  OBJECT_DOES_NOT_EXIST = 10
-  PC_STATUS_CHANGED = 11
-  UNEXPECTED_VERSION = 12
-  INVALID_MOAB = 13
-  PC_PO_VERSION_MISMATCH = 14
-  ONLINE_MOAB_DOES_NOT_EXIST = 15
+  CREATED_NEW_OBJECT = 5
+  DB_UPDATE_FAILED = 6
+  OBJECT_ALREADY_EXISTS = 7
+  OBJECT_DOES_NOT_EXIST = 8
+  PC_STATUS_CHANGED = 9
+  UNEXPECTED_VERSION = 10
+  INVALID_MOAB = 11
+  PC_PO_VERSION_MISMATCH = 12
+  ONLINE_MOAB_DOES_NOT_EXIST = 13
 
   RESPONSE_CODE_TO_MESSAGES = {
     INVALID_ARGUMENTS => "encountered validation error(s): %{addl}",
     VERSION_MATCHES => "incoming version (%{incoming_version}) matches %{addl} db version",
     ARG_VERSION_GREATER_THAN_DB_OBJECT => "incoming version (%{incoming_version}) greater than %{addl} db version",
     ARG_VERSION_LESS_THAN_DB_OBJECT => "incoming version (%{incoming_version}) less than %{addl} db version; ERROR!",
-    UPDATED_DB_OBJECT => "%{addl} db object updated",
-    UPDATED_DB_OBJECT_TIMESTAMP_ONLY => "%{addl} updated db timestamp only",
     CREATED_NEW_OBJECT => "added object to db as it did not exist",
     DB_UPDATE_FAILED => "db update failed: %{addl}",
     OBJECT_ALREADY_EXISTS => "%{addl} db object already exists",
@@ -51,8 +47,6 @@ class PreservedObjectHandlerResults
   ].freeze
 
   DB_UPDATED_CODES = [
-    UPDATED_DB_OBJECT,
-    UPDATED_DB_OBJECT_TIMESTAMP_ONLY,
     CREATED_NEW_OBJECT,
     PC_STATUS_CHANGED
   ].freeze
@@ -63,8 +57,6 @@ def self.logger_severity_level(result_code)
     when VERSION_MATCHES then Logger::INFO
     when ARG_VERSION_GREATER_THAN_DB_OBJECT then Logger::INFO
     when ARG_VERSION_LESS_THAN_DB_OBJECT then Logger::ERROR
-    when UPDATED_DB_OBJECT then Logger::INFO
-    when UPDATED_DB_OBJECT_TIMESTAMP_ONLY then Logger::INFO
     when CREATED_NEW_OBJECT then Logger::INFO
     when DB_UPDATE_FAILED then Logger::ERROR
     when OBJECT_ALREADY_EXISTS then Logger::ERROR

lib/audit/catalog_to_moab.rb

@@ -92,10 +92,7 @@ def check_catalog_version
     end
 
     preserved_copy.update_audit_timestamps(ran_moab_validation?, true)
-
-    # TODO: We need to save preserved copy.  Do we want to use something like
-    #  PreservedObjectHandler.update_db_object? - see #478
-    # update_db_object(preserved_copy) - see #478
+    preserved_copy.save!
   end
 
   private

spec/services/preserved_object_handler_check_exist_spec.rb

@@ -33,7 +33,6 @@
         let(:exp_msg_prefix) { "PreservedObjectHandler(#{druid}, 2, 1, #{ep.endpoint_name})" }
         let(:version_matches_po_msg) { "#{exp_msg_prefix} incoming version (2) matches PreservedObject db version" }
         let(:version_matches_pc_msg) { "#{exp_msg_prefix} incoming version (2) matches PreservedCopy db version" }
-        let(:updated_pc_db_msg) { "#{exp_msg_prefix} PreservedCopy db object updated" }
 
         context 'PreservedCopy' do
           context 'changed' do
@@ -81,7 +80,6 @@
         it "logs at info level" do
           expect(Rails.logger).to receive(:log).with(Logger::INFO, version_matches_po_msg)
           expect(Rails.logger).to receive(:log).with(Logger::INFO, version_matches_pc_msg)
-          expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_pc_db_msg)
           po_handler.check_existence
         end
         it 'does not validate moab' do
@@ -94,19 +92,15 @@
           # results = [result1, result2]
           # result1 = {response_code: msg}
           # result2 = {response_code: msg}
-          it '3 results' do
+          it '2 results' do
             expect(results).to be_an_instance_of Array
-            expect(results.size).to eq 3
+            expect(results.size).to eq 2
           end
           it 'VERSION_MATCHES results' do
             code = PreservedObjectHandlerResults::VERSION_MATCHES
             expect(results).to include(a_hash_including(code => version_matches_pc_msg))
             expect(results).to include(a_hash_including(code => version_matches_po_msg))
           end
-          it 'UPDATED_DB_OBJECT PreservedCopy result' do
-            code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-            expect(results).to include(a_hash_including(code => updated_pc_db_msg))
-          end
         end
       end
 
@@ -116,7 +110,6 @@
         let(:version_gt_pc_msg) { "#{exp_msg_prefix} incoming version (#{incoming_version}) greater than PreservedCopy db version" }
         let(:version_gt_po_msg) { "#{exp_msg_prefix} incoming version (#{incoming_version}) greater than PreservedObject db version" }
         let(:updated_pc_db_msg) { "#{exp_msg_prefix} PreservedCopy db object updated" }
-        let(:updated_po_db_msg) { "#{exp_msg_prefix} PreservedObject db object updated" }
         let(:updated_status_msg_regex) { Regexp.new(Regexp.escape("#{exp_msg_prefix} PreservedCopy status changed from")) }
 
         it 'calls Stanford::StorageObjectValidator.validation_errors for moab' do
@@ -212,8 +205,6 @@
             allow(po_handler).to receive(:moab_validation_errors).and_return([])
             expect(Rails.logger).to receive(:log).with(Logger::INFO, version_gt_po_msg)
             expect(Rails.logger).to receive(:log).with(Logger::INFO, version_gt_pc_msg)
-            expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_po_db_msg)
-            expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_pc_db_msg)
             expect(Rails.logger).not_to receive(:log).with(Logger::INFO, updated_status_msg_regex)
             po_handler.check_existence
           end
@@ -227,20 +218,15 @@
             # results = [result1, result2]
             # result1 = {response_code: msg}
             # result2 = {response_code: msg}
-            it '4 results' do
+            it '2 results' do
               expect(results).to be_an_instance_of Array
-              expect(results.size).to eq 4
+              expect(results.size).to eq 2
             end
             it 'ARG_VERSION_GREATER_THAN_DB_OBJECT results' do
               code = PreservedObjectHandlerResults::ARG_VERSION_GREATER_THAN_DB_OBJECT
               expect(results).to include(a_hash_including(code => version_gt_pc_msg))
               expect(results).to include(a_hash_including(code => version_gt_po_msg))
             end
-            it "UPDATED_DB_OBJECT results" do
-              code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-              expect(results).to include(a_hash_including(code => updated_pc_db_msg))
-              expect(results).to include(a_hash_including(code => updated_po_db_msg))
-            end
             it 'what results/logging desired for incoming version > catalog' do
               skip 'we need clarification of requirements on results/logging in this case'
             end
@@ -346,20 +332,15 @@
             # results = [result1, result2]
             # result1 = {response_code: msg}
             # result2 = {response_code: msg}
-            it '5 results' do
+            it '4 results' do
               expect(results).to be_an_instance_of Array
-              expect(results.size).to eq 5
+              expect(results.size).to eq 4
             end
             it 'ARG_VERSION_GREATER_THAN_DB_OBJECT results' do
               code = PreservedObjectHandlerResults::ARG_VERSION_GREATER_THAN_DB_OBJECT
               expect(results).to include(a_hash_including(code => version_gt_pc_msg))
               expect(results).to include(a_hash_including(code => version_gt_po_msg))
             end
-            it "UPDATED_DB_OBJECT results" do
-              code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-              expect(results).to include(a_hash_including(code => updated_pc_db_msg))
-              expect(results).to include(hash_not_including(code => updated_po_db_msg))
-            end
             it 'INVALID_MOAB result' do
               expect(results).to include(a_hash_including(PreservedObjectHandlerResults::INVALID_MOAB))
             end
@@ -470,10 +451,10 @@
         allow(pc).to receive(:version).and_return(1)
         allow(pc).to receive(:update_audit_timestamps)
         allow(pc).to receive(:changed?).and_return(false)
-        allow(pc).to receive(:touch)
+        allow(pc).to receive(:save!)
         po_handler.check_existence
         expect(po).not_to have_received(:touch)
-        expect(pc).to have_received(:touch)
+        expect(pc).to have_received(:save!)
       end
       it 'logs a debug message' do
         msg = "check_existence #{druid} called"

spec/services/preserved_object_handler_confirm_version_spec.rb

@@ -51,7 +51,6 @@
         let(:exp_msg_prefix) { "PreservedObjectHandler(#{druid}, 2, 1, #{ep.endpoint_name})" }
         let(:version_matches_po_msg) { "#{exp_msg_prefix} incoming version (2) matches PreservedObject db version" }
         let(:version_matches_pc_msg) { "#{exp_msg_prefix} incoming version (2) matches PreservedCopy db version" }
-        let(:updated_pc_db_msg) { "#{exp_msg_prefix} PreservedCopy db object updated" }
 
         context 'PreservedCopy' do
           context 'changed' do
@@ -99,7 +98,6 @@
         it "logs at info level" do
           expect(Rails.logger).to receive(:log).with(Logger::INFO, version_matches_po_msg)
           expect(Rails.logger).to receive(:log).with(Logger::INFO, version_matches_pc_msg)
-          expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_pc_db_msg)
           po_handler.confirm_version
         end
         context 'returns' do
@@ -108,19 +106,15 @@
           # results = [result1, result2]
           # result1 = {response_code: msg}
           # result2 = {response_code: msg}
-          it '3 results' do
+          it '2 results' do
             expect(results).to be_an_instance_of Array
-            expect(results.size).to eq 3
+            expect(results.size).to eq 2
           end
           it 'VERSION_MATCHES results' do
             code = PreservedObjectHandlerResults::VERSION_MATCHES
             expect(results).to include(a_hash_including(code => version_matches_pc_msg))
             expect(results).to include(a_hash_including(code => version_matches_po_msg))
           end
-          it 'UPDATED_DB_OBJECT PreservedCopy result' do
-            code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-            expect(results).to include(a_hash_including(code => updated_pc_db_msg))
-          end
         end
       end
 
@@ -158,7 +152,6 @@
         let(:updated_pc_db_status_msg) {
           "#{exp_msg_prefix} PreservedCopy status changed from ok to expected_vers_not_found_on_storage"
         }
-        let(:updated_pc_db_obj_msg) { "#{exp_msg_prefix} PreservedCopy db object updated" }
 
         before do
           allow(po_handler).to receive(:moab_validation_errors).and_return([])
@@ -211,7 +204,6 @@
         it "logs at error level" do
           expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_pc_db_status_msg)
           expect(Rails.logger).to receive(:log).with(Logger::ERROR, unexpected_version_pc_msg)
-          expect(Rails.logger).to receive(:log).with(Logger::INFO, updated_pc_db_obj_msg)
           po_handler.confirm_version
         end
         context 'returns' do
@@ -220,18 +212,14 @@
           # results = [result1, result2]
           # result1 = {response_code: msg}
           # result2 = {response_code: msg}
-          it '3 results' do
+          it '2 results' do
             expect(results).to be_an_instance_of Array
-            expect(results.size).to eq 3
+            expect(results.size).to eq 2
           end
           it 'UNEXPECTED_VERSION PreservedCopy result' do
             code = PreservedObjectHandlerResults::UNEXPECTED_VERSION
             expect(results).to include(a_hash_including(code => unexpected_version_pc_msg))
           end
-          it 'UPDATED_DB_OBJECT PreservedCopy result' do
-            code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-            expect(results).to include(a_hash_including(code => updated_pc_db_obj_msg))
-          end
           it "PC_STATUS_CHANGED PreservedCopy result" do
             code = PreservedObjectHandlerResults::PC_STATUS_CHANGED
             expect(results).to include(a_hash_including(code => updated_pc_db_status_msg))
@@ -307,11 +295,11 @@
         allow(pc).to receive(:version).and_return(1)
         allow(pc).to receive(:update_audit_timestamps)
         allow(pc).to receive(:changed?).and_return(false)
-        allow(pc).to receive(:touch)
+        allow(pc).to receive(:save!)
         allow(pc).to receive(:matches_po_current_version?).and_return(true)
         po_handler.confirm_version
         expect(po).not_to have_received(:touch)
-        expect(pc).to have_received(:touch)
+        expect(pc).to have_received(:save!)
       end
       it 'logs a debug message' do
         msg = "confirm_version #{druid} called"

spec/services/preserved_object_handler_results_spec.rb

@@ -39,13 +39,9 @@
         code = PreservedObjectHandlerResults::PC_STATUS_CHANGED
         status_details = { old_status: PreservedCopy::INVALID_MOAB_STATUS, new_status: PreservedCopy::OK_STATUS }
         pohr.add_result(code, status_details)
-        code = PreservedObjectHandlerResults::UPDATED_DB_OBJECT
-        db_obj_details = 'PreservedCopy'
-        pohr.add_result(code, db_obj_details)
         not_matched_str = 'does not match PreservedObject current_version'
         expect(Rails.logger).to receive(:log).with(Logger::ERROR, a_string_matching(not_matched_str))
         expect(Rails.logger).to receive(:log).with(Logger::INFO, a_string_matching(PreservedCopy::INVALID_MOAB_STATUS))
-        expect(Rails.logger).to receive(:log).with(Logger::INFO, a_string_matching(db_obj_details))
         pohr.report_results
       end
     end