13561: Raise exception instead of ignoring versioning attributes updates.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 250dfef3684c9a6cd5f2df9c61ab9e157cb89f4f..718ffc0d0a51416440ff75ec98c442cfe64423b9 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -27,7 +27,7 @@ class Collection < ArvadosModel
   validate :ensure_pdh_matches_manifest_text
   validate :ensure_storage_classes_desired_is_not_empty
   validate :ensure_storage_classes_contain_non_empty_strings
-  validate :current_versions_always_point_to_self, on: :update
+  validate :versioning_metadata_updates, on: :update
   validate :past_versions_cannot_be_updated, on: :update
   before_save :set_file_names
   around_update :manage_versioning
@@ -242,9 +242,7 @@ class Collection < ArvadosModel
 
       # Restore requested changes on the current version
       changes.keys.each do |attr|
-        if attr == 'version'
-          next
-        elsif attr == 'preserve_version' && changes[attr].last == false
+        if attr == 'preserve_version' && changes[attr].last == false
           next # Ignore false assignment, once true it'll be true until next version
         end
         self.attributes = {attr => changes[attr].last}
@@ -629,11 +627,17 @@ class Collection < ArvadosModel
     end
   end
 
-  def current_versions_always_point_to_self
+  def versioning_metadata_updates
+    valid = true
     if (current_version_uuid_was == uuid_was) && current_version_uuid_changed?
       errors.add(:current_version_uuid, "cannot be updated")
-      false
+      valid = false
+    end
+    if version_changed?
+      errors.add(:version, "cannot be updated")
+      valid = false
     end
+    valid
   end
 
   def assign_uuid

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 21450d7a55c8ee325bf8cdddd7e93f4967b557b4..9797ed63dc0d098898d38a4e0741ecd9fc7e0e4c 100644 (file)
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -173,6 +173,26 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  [
+    ['version', 10],
+    ['current_version_uuid', 'zzzzz-4zz18-bv31uwvy3neko21'],
+  ].each do |name, new_value|
+    test "'#{name}' updates on current version collections are not allowed" do
+      act_as_user users(:active) do
+        # Set up initial collection
+        c = create_collection 'foo', Encoding::US_ASCII
+        assert c.valid?
+        assert_equal 1, c.version
+
+        assert_raises(ActiveRecord::RecordInvalid) do
+          c.update_attributes!({
+            name => new_value
+          })
+        end
+      end
+    end
+  end
+
   test "uuid updates on current version make older versions update their pointers" do
     Rails.configuration.collection_versioning = true
     Rails.configuration.preserve_version_if_idle = 0