From: Lucas Di Pentima <lucas@di-pentima.com.ar>
Date: Tue, 28 Jul 2020 19:16:29 +0000 (-0300)
Subject: 16470: Fixes false unpersisted status when retrieving a record with audit logs.
X-Git-Tag: 2.1.0~132^2~11
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/752845cfd9355b1d207f3c876de28ba07f9d1883

16470: Fixes false unpersisted status when retrieving a record with audit logs.

The cleanest solution I came up with is to flag the instance when it's
retrieved from the database, and reset any changes after stashing its
state on the log_start_state callback.
Haven't found a way to read the serialized attributes without making
them appear as changed, and I think it isn't possible because the
attributes have to be unserialized before the read operation, and thus
the dirty state machinery would assume the attribute may be modified.
This solution isn't ideal, but I think it's acceptable as it doesn't
make additional database requests.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>
---

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 80ea0c0b7f..c3e1ff42ad 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -16,6 +16,7 @@ class ArvadosModel < ApplicationRecord
   include DbCurrentTime
   extend RecordFilters
 
+  after_find :schedule_restoring_changes
   after_initialize :log_start_state
   before_save :ensure_permission_to_save
   before_save :ensure_owner_uuid_is_permitted
@@ -834,10 +835,24 @@ class ArvadosModel < ApplicationRecord
              Rails.configuration.AuditLogs.MaxDeleteBatch.to_i > 0)
   end
 
+  def schedule_restoring_changes
+    # This will be checked at log_start_state, to reset any (virtual) changes
+    # produced by the act of reading a serialized attribute.
+    @fresh_from_database = true
+  end
+
   def log_start_state
     if is_audit_logging_enabled?
       @old_attributes = Marshal.load(Marshal.dump(attributes))
       @old_logged_attributes = Marshal.load(Marshal.dump(logged_attributes))
+      if @fresh_from_database
+        # This instance was created from reading a database record. Attributes
+        # haven't been changed, but those serialized attributes will be reported
+        # as unpersisted, so we restore them to avoid issues with lock!() and
+        # with_lock().
+        restore_attributes
+        @fresh_from_database = nil
+      end
     end
   end