[ARVADOS] created: 1.2.0-217-geab1bd711

[Git user]

        at  eab1bd711a010468edfef42d1d59ad66523713aa (commit)


commit eab1bd711a010468edfef42d1d59ad66523713aa
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Wed Oct 17 15:51:36 2018 -0300

    13561: Raise exception instead of ignoring versioning attributes updates.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 250dfef36..718ffc0d0 100644
--- 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 21450d7a5..9797ed63d 100644
--- 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

commit 917cdcde33c5a4ab7075a1d844d6dbe2540ab05a
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Wed Oct 17 15:05:50 2018 -0300

    13561: Fixes api to get a collection with its past versions.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 18b88c4d4..5d7a7ae26 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -235,17 +235,4 @@ class Arvados::V1::CollectionsController < ApplicationController
       @select ||= model_class.selectable_attributes - ["manifest_text", "unsigned_manifest_text"]
     end
   end
-
-  def load_filters_param
-    super
-    return if !params[:include_old_versions]
-    @filters = @filters.map do |col, operator, operand|
-      # Replace uuid filters when including past versions
-      if col == 'uuid'
-        ['current_version_uuid', operator, operand]
-      else
-        [col, operator, operand]
-      end
-    end
-  end
 end
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index fdc54894f..f5bed6384 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1197,7 +1197,7 @@ EOS
   test 'can get collection with past versions' do
     authorize_with :active
     get :index, {
-      filters: [['uuid','=',collections(:collection_owned_by_active).uuid]],
+      filters: [['current_version_uuid','=',collections(:collection_owned_by_active).uuid]],
       include_old_versions: true
     }
     assert_response :success

-----------------------------------------------------------------------


hooks/post-receive
--