16470: Adds an explicit reload before every pending with_lock call.

[arvados.git] / services / api / app / models / collection.rb

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 649e0f5d090680921d58ea2af53df27fe1b170d2..8f724e1f5f94bee57982a260e6caa660f81f8ac9 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -28,6 +28,7 @@ class Collection < ArvadosModel
   before_validation :check_signatures
   before_validation :strip_signatures_and_update_replication_confirmed
   before_validation :name_null_if_empty
+  validate :ensure_filesystem_compatible_name
   validate :ensure_pdh_matches_manifest_text
   validate :ensure_storage_classes_desired_is_not_empty
   validate :ensure_storage_classes_contain_non_empty_strings
@@ -258,10 +259,10 @@ class Collection < ArvadosModel
     should_preserve_version = should_preserve_version? # Time sensitive, cache value
     return(yield) unless (should_preserve_version || syncable_updates.any?)
 
-    # Put aside the changes because with_lock forces a record reload
+    # Put aside the changes because with_lock requires an explicit record reload
     changes = self.changes
     snapshot = nil
-    with_lock do
+    reload.with_lock do
       # Copy the original state to save it as old version
       if should_preserve_version
         snapshot = self.dup
@@ -489,17 +490,13 @@ class Collection < ArvadosModel
         [c.portable_data_hash, c]
       }]
 
-    puts "mg #{migrated_collections} #{collections}"
-
     collections.map { |c|
       # Check if the listed image is compatible first, if not, then try the
       # migration link.
       manifest = Keep::Manifest.new(c.manifest_text)
-      puts "m1 #{manifest.exact_file_count?(1)} #{manifest.files[0][1]} #{pattern}"
       if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern
         c
       elsif m = migrated_collections[migrations[c.portable_data_hash]]
-        puts "m2 #{manifest.exact_file_count?(1)} #{manifest.files[0][1]} #{pattern}"
         manifest = Keep::Manifest.new(m.manifest_text)
         if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern
           m
@@ -627,10 +624,10 @@ class Collection < ArvadosModel
       return
     end
     (managed_props.keys - self.properties.keys).each do |key|
-      if managed_props[key].has_key?('Value')
-        self.properties[key] = managed_props[key]['Value']
-      elsif managed_props[key]['Function'].andand == 'original_owner'
+      if managed_props[key]['Function'] == 'original_owner'
         self.properties[key] = self.user_owner_uuid
+      elsif managed_props[key]['Value']
+        self.properties[key] = managed_props[key]['Value']
       else
         logger.warn "Unidentified default property definition '#{key}': #{managed_props[key].inspect}"
       end