[ARVADOS] updated: 2.1.0-218-gd5745d536
Git user
git at public.arvados.org
Fri Dec 11 15:32:14 UTC 2020
Summary of changes:
.../arvados/v1/collections_controller.rb | 8 +++
services/api/app/models/collection.rb | 5 +-
.../arvados/v1/collections_controller_test.rb | 2 +-
.../api/test/integration/collections_api_test.rb | 78 ++++++++++++++++++++++
services/api/test/unit/collection_test.rb | 7 +-
5 files changed, 93 insertions(+), 7 deletions(-)
via d5745d536d013a6731e0e6a872abe34d71f0995e (commit)
from 653f8d8ba8d07a2d7081a924a67ee31b1a8ceebd (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit d5745d536d013a6731e0e6a872abe34d71f0995e
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Fri Dec 11 12:28:22 2020 -0300
17152: Moves conditional preserve_version disabling to collection's controller.
There was a bug that flipped preserve_version to false after creating a new
version even though the update request included preserve_version=true.
This happened because the logic was at model level, and was incorrect because
at that level we cannot know if the client included the attribute on its
request.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 2e7e2f82b..440ac6401 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -43,6 +43,14 @@ class Arvados::V1::CollectionsController < ApplicationController
super
end
+ def update
+ # preserve_version should be disabled unless explicitly asked otherwise.
+ if !resource_attrs[:preserve_version]
+ resource_attrs[:preserve_version] = false
+ end
+ super
+ end
+
def find_objects_for_index
opts = {
include_trash: params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name),
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index e3802734a..c2c60ec70 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -278,7 +278,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 +290,6 @@ class Collection < ArvadosModel
if should_preserve_version
self.version += 1
- if !changes.keys.include?('preserve_version')
- self.preserve_version = false
- end
end
yield
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 c025394bc..1ca2dd1dc 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1145,7 +1145,7 @@ EOS
end
[:admin, :active].each do |user|
- test "get trashed collection via filters and #{user} user" do
+ test "get trashed collection via filters and #{user} user without including its past versions" do
uuid = 'zzzzz-4zz18-mto52zx1s7sn3ih' # expired_collection
authorize_with user
get :index, params: {
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index 86195fba7..73cbad643 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -495,4 +495,82 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
assert_equal Hash, json_response['properties'].class, 'Collection properties attribute should be of type hash'
assert_equal 'value_1', json_response['properties']['property_1']
end
+
+ test "update collection with versioning enabled and using preserve_version" do
+ Rails.configuration.Collections.CollectionVersioning = true
+ Rails.configuration.Collections.PreserveVersionIfIdle = -1 # Disable auto versioning
+
+ signed_manifest = Collection.sign_manifest(". bad42fa702ae3ea7d888fef11b46f450+44 0:44:my_test_file.txt\n", api_token(:active))
+ post "/arvados/v1/collections",
+ params: {
+ format: :json,
+ collection: {
+ name: 'Test collection',
+ manifest_text: signed_manifest,
+ }.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_not_nil json_response['uuid']
+ assert_equal 1, json_response['version']
+ assert_equal false, json_response['preserve_version']
+
+ # Versionable update including preserve_version=true should create a new
+ # version that will also be persisted.
+ put "/arvados/v1/collections/#{json_response['uuid']}",
+ params: {
+ format: :json,
+ collection: {
+ name: 'Test collection v2',
+ preserve_version: true,
+ }.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_equal 2, json_response['version']
+ assert_equal true, json_response['preserve_version']
+
+ # 2nd versionable update including preserve_version=true should create a new
+ # version that will also be persisted.
+ put "/arvados/v1/collections/#{json_response['uuid']}",
+ params: {
+ format: :json,
+ collection: {
+ name: 'Test collection v3',
+ preserve_version: true,
+ }.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_equal 3, json_response['version']
+ assert_equal true, json_response['preserve_version']
+
+ # 3rd versionable update without including preserve_version should create a new
+ # version that will have its preserve_version attr reset to false.
+ put "/arvados/v1/collections/#{json_response['uuid']}",
+ params: {
+ format: :json,
+ collection: {
+ name: 'Test collection v4',
+ }.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_equal 4, json_response['version']
+ assert_equal false, json_response['preserve_version']
+
+ # 4th versionable update without including preserve_version=true should NOT
+ # create a new version.
+ put "/arvados/v1/collections/#{json_response['uuid']}",
+ params: {
+ format: :json,
+ collection: {
+ name: 'Test collection v5?',
+ }.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_equal 4, json_response['version']
+ assert_equal false, json_response['preserve_version']
+ end
end
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 70645615a..916ca0958 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -190,7 +190,7 @@ class CollectionTest < ActiveSupport::TestCase
test "preserve_version updates" do
Rails.configuration.Collections.CollectionVersioning = true
- Rails.configuration.Collections.PreserveVersionIfIdle = 3600
+ Rails.configuration.Collections.PreserveVersionIfIdle = -1 # disabled
act_as_user users(:active) do
# Set up initial collection
c = create_collection 'foo', Encoding::US_ASCII
@@ -227,7 +227,10 @@ class CollectionTest < ActiveSupport::TestCase
assert_equal 2, c.replication_desired
assert_equal true, c.preserve_version
# This is a versionable update
- c.update_attributes!({'name' => 'foobar'})
+ c.update_attributes!({
+ 'preserve_version' => false,
+ 'name' => 'foobar'
+ })
c.reload
assert_equal 3, c.version
assert_equal false, c.preserve_version
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list