19146: Remove unneeded special case checks, explain the needed one.
[arvados.git] / services / api / app / models / collection.rb
index e3802734a07580c70bf1921c64071c71dc43980b..b4660dbd355de72261d4584977b88533f77f829e 100644 (file)
@@ -3,7 +3,6 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 require 'arvados/keep'
-require 'sweep_trashed_objects'
 require 'trashable'
 
 class Collection < ArvadosModel
@@ -17,7 +16,7 @@ class Collection < ArvadosModel
   # Posgresql JSONB columns should NOT be declared as serialized, Rails 5
   # already know how to properly treat them.
   attribute :properties, :jsonbHash, default: {}
-  attribute :storage_classes_desired, :jsonbArray, default: ["default"]
+  attribute :storage_classes_desired, :jsonbArray, default: lambda { Rails.configuration.DefaultStorageClasses }
   attribute :storage_classes_confirmed, :jsonbArray, default: []
 
   before_validation :default_empty_manifest
@@ -37,8 +36,6 @@ class Collection < ArvadosModel
   validate :protected_managed_properties_updates, on: :update
   after_validation :set_file_count_and_total_size
   before_save :set_file_names
-  before_update :preserve_version_exclusive_updates_leave_modified_at_alone,
-    if: Proc.new { |col| col.changes.keys.sort == ['modified_at', 'updated_at', 'preserve_version'].sort }
   around_update :manage_versioning, unless: :is_past_version?
 
   api_accessible :user, extend: :common do |t|
@@ -46,8 +43,8 @@ class Collection < ArvadosModel
     t.add :description
     t.add :properties
     t.add :portable_data_hash
-    t.add :signed_manifest_text, as: :manifest_text
     t.add :manifest_text, as: :unsigned_manifest_text
+    t.add :manifest_text, as: :manifest_text
     t.add :replication_desired
     t.add :replication_confirmed
     t.add :replication_confirmed_at
@@ -73,15 +70,11 @@ class Collection < ArvadosModel
 
   def self.attributes_required_columns
     super.merge(
-                # If we don't list manifest_text explicitly, the
-                # params[:select] code gets confused by the way we
-                # expose signed_manifest_text as manifest_text in the
-                # API response, and never let clients select the
-                # manifest_text column.
-                #
-                # We need trash_at and is_trashed to determine the
-                # correct timestamp in signed_manifest_text.
-                'manifest_text' => ['manifest_text', 'trash_at', 'is_trashed'],
+                # If we don't list unsigned_manifest_text explicitly,
+                # the params[:select] code gets confused by the way we
+                # expose manifest_text as unsigned_manifest_text in
+                # the API response, and never let clients select the
+                # unsigned_manifest_text column.
                 'unsigned_manifest_text' => ['manifest_text'],
                 'name' => ['name'],
                 )
@@ -152,7 +145,9 @@ class Collection < ArvadosModel
   def strip_signatures_and_update_replication_confirmed
     if self.manifest_text_changed?
       in_old_manifest = {}
-      if not self.replication_confirmed.nil?
+      # manifest_text_was could be nil when dealing with a freshly created snapshot,
+      # so we skip this case because there was no real manifest change. (Bug #18005)
+      if (not self.replication_confirmed.nil?) and (not self.manifest_text_was.nil?)
         self.class.each_manifest_locator(manifest_text_was) do |match|
           in_old_manifest[match[1]] = true
         end
@@ -278,7 +273,7 @@ class Collection < ArvadosModel
 
       # Restore requested changes on the current version
       changes.keys.each do |attr|
-        if attr == 'preserve_version' && changes[attr].last == false
+        if attr == 'preserve_version' && changes[attr].last == false && !should_preserve_version
           next # Ignore false assignment, once true it'll be true until next version
         end
         self.attributes = {attr => changes[attr].last}
@@ -290,9 +285,6 @@ class Collection < ArvadosModel
 
       if should_preserve_version
         self.version += 1
-        if !changes.keys.include?('preserve_version')
-          self.preserve_version = false
-        end
       end
 
       yield
@@ -311,8 +303,10 @@ class Collection < ArvadosModel
     end
   end
 
-  def preserve_version_exclusive_updates_leave_modified_at_alone
-    self.modified_at = self.modified_at_was
+  def maybe_update_modified_by_fields
+    if !(self.changes.keys - ['updated_at', 'preserve_version']).empty?
+      super
+    end
   end
 
   def syncable_updates
@@ -407,7 +401,7 @@ class Collection < ArvadosModel
     end
   end
 
-  def signed_manifest_text
+  def signed_manifest_text_only_for_tests
     if !has_attribute? :manifest_text
       return nil
     elsif is_trashed
@@ -416,11 +410,11 @@ class Collection < ArvadosModel
       token = Thread.current[:token]
       exp = [db_current_time.to_i + Rails.configuration.Collections.BlobSigningTTL.to_i,
              trash_at].compact.map(&:to_i).min
-      self.class.sign_manifest manifest_text, token, exp
+      self.class.sign_manifest_only_for_tests manifest_text, token, exp
     end
   end
 
-  def self.sign_manifest manifest, token, exp=nil
+  def self.sign_manifest_only_for_tests manifest, token, exp=nil
     if exp.nil?
       exp = db_current_time.to_i + Rails.configuration.Collections.BlobSigningTTL.to_i
     end
@@ -621,11 +615,6 @@ class Collection < ArvadosModel
     super - ["manifest_text", "storage_classes_desired", "storage_classes_confirmed", "current_version_uuid"]
   end
 
-  def self.where *args
-    SweepTrashedObjects.sweep_if_stale
-    super
-  end
-
   protected
 
   # Although the defaults for these columns is already set up on the schema,
@@ -633,7 +622,7 @@ class Collection < ArvadosModel
   # validation on empty desired storage classes return an error.
   def default_storage_classes
     if self.storage_classes_desired.nil? || self.storage_classes_desired.empty?
-      self.storage_classes_desired = ["default"]
+      self.storage_classes_desired = Rails.configuration.DefaultStorageClasses
     end
     self.storage_classes_confirmed ||= []
   end