From d62b73382398808a440f15fdda2eea2e15e44282 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 6 Feb 2015 19:07:43 -0500 Subject: [PATCH] 3410: Add tests for replication attributes. --- services/api/app/models/collection.rb | 8 +- services/api/test/fixtures/collections.yml | 42 +++++++++ .../arvados/v1/collections_controller_test.rb | 14 ++- services/api/test/unit/collection_test.rb | 87 ++++++++++++++++++- 4 files changed, 134 insertions(+), 17 deletions(-) diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb index 3ab97bf095..cd05b4ba62 100644 --- a/services/api/app/models/collection.rb +++ b/services/api/app/models/collection.rb @@ -216,8 +216,8 @@ class Collection < ArvadosModel def self.each_manifest_locator manifest # Given a manifest text and a block, yield each locator. - manifest.andand.scan(/ ([[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+))/) do |word| - if loc = Keep::Locator.parse(word[1]) + manifest.andand.scan(/ ([[:xdigit:]]{32}(\+\S+)?)/) do |word, _| + if loc = Keep::Locator.parse(word) yield loc end end @@ -331,8 +331,8 @@ class Collection < ArvadosModel end self.class.each_manifest_locator(manifest_text) do |loc| if not in_old_manifest[loc.hash] - replication_confirmed_at = nil - replication_confirmed = nil + self.replication_confirmed_at = nil + self.replication_confirmed = nil break end end diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml index 9ddc45272c..3f848f6811 100644 --- a/services/api/test/fixtures/collections.yml +++ b/services/api/test/fixtures/collections.yml @@ -391,6 +391,48 @@ collection_with_unique_words_to_test_full_text_search: name: collection_with_some_unique_words description: The quick_brown_fox jumps over the lazy_dog +replication_undesired_unconfirmed: + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-07 00:19:28.596506247 Z + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_at: 2015-02-07 00:19:28.596338465 Z + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + replication_desired: ~ + replication_confirmed_at: ~ + replication_confirmed: ~ + updated_at: 2015-02-07 00:19:28.596236608 Z + uuid: zzzzz-4zz18-wjxq7uzx2m9jj4a + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: replication wantnull havenull + +replication_desired_2_unconfirmed: + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-07 00:21:35.050333515 Z + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_at: 2015-02-07 00:21:35.050189104 Z + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + replication_desired: 2 + replication_confirmed_at: ~ + replication_confirmed: ~ + updated_at: 2015-02-07 00:21:35.050126576 Z + uuid: zzzzz-4zz18-3t236wrz4769h7x + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: replication want2 havenull + +replication_desired_2_confirmed_2: + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-07 00:19:28.596506247 Z + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_at: 2015-02-07 00:19:28.596338465 Z + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + replication_desired: 2 + replication_confirmed_at: 2015-02-07 00:24:52.983381227 Z + replication_confirmed: 2 + updated_at: 2015-02-07 00:24:52.983381227 Z + uuid: zzzzz-4zz18-434zv1tnnf2rygp + manifest_text: ". acbd18db4cc2f85cedef654fccc4a4d8+3 37b51d194a7513e45b56f6524f2d51f2+3 0:3:foo 3:6:bar\n" + name: replication want2 have2 + # Test Helper trims the rest of the file # Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper 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 61b0557085..42059e7bd7 100644 --- a/services/api/test/functional/arvados/v1/collections_controller_test.rb +++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb @@ -698,21 +698,17 @@ EOS end [1, 5, nil].each do |ask| - test "Set replication_desired=#{ask} using redundancy attr" do - # The Python SDK checks the Collection schema in the discovery - # doc, then asks for 'redundancy' or 'replication_desired' - # accordingly, so it isn't necessary to maintain backward - # compatibility here when the attribute changes to - # replication_desired. + test "Set replication_desired=#{ask.inspect}" do + Rails.configuration.default_collection_replication = 2 authorize_with :active put :update, { - id: collections(:collection_owned_by_active).uuid, + id: collections(:replication_undesired_unconfirmed).uuid, collection: { - redundancy: ask, + replication_desired: ask, }, } assert_response :success - assert_equal (ask or 2), json_response['replication_desired'] + assert_equal ask, json_response['replication_desired'] end end end diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb index 59f9d3d41a..9dda9d5f0a 100644 --- a/services/api/test/unit/collection_test.rb +++ b/services/api/test/unit/collection_test.rb @@ -120,12 +120,91 @@ class CollectionTest < ActiveSupport::TestCase end [0, 2, 4, nil].each do |ask| - test "replication_desired reports #{ask or 2} if redundancy is #{ask}" do + test "set replication_desired to #{ask.inspect}" do + Rails.configuration.default_collection_replication = 2 act_as_user users(:active) do - c = collections(:collection_owned_by_active) - c.update_attributes redundancy: ask - assert_equal (ask or 2), c.replication_desired + c = collections(:replication_undesired_unconfirmed) + c.update_attributes replication_desired: ask + assert_equal ask, c.replication_desired end end end + + test "replication_confirmed* can be set by admin user" do + c = collections(:replication_desired_2_unconfirmed) + act_as_user users(:admin) do + assert c.update_attributes(replication_confirmed: 2, + replication_confirmed_at: Time.now) + end + end + + test "replication_confirmed* cannot be set by non-admin user" do + act_as_user users(:active) do + c = collections(:replication_desired_2_unconfirmed) + # Cannot set just one at a time. + assert_raise ArvadosModel::PermissionDeniedError do + c.update_attributes replication_confirmed: 1 + end + assert_raise ArvadosModel::PermissionDeniedError do + c.update_attributes replication_confirmed_at: Time.now + end + # Cannot set both at once. + assert_raise ArvadosModel::PermissionDeniedError do + c.update_attributes(replication_confirmed: 1, + replication_confirmed_at: Time.now) + end + end + end + + test "replication_confirmed* can be cleared (but only together) by non-admin user" do + act_as_user users(:active) do + c = collections(:replication_desired_2_confirmed_2) + # Cannot clear just one at a time. + assert_raise ArvadosModel::PermissionDeniedError do + c.update_attributes replication_confirmed: nil + end + c.reload + assert_raise ArvadosModel::PermissionDeniedError do + c.update_attributes replication_confirmed_at: nil + end + # Can clear both at once. + c.reload + assert c.update_attributes(replication_confirmed: nil, + replication_confirmed_at: nil) + end + end + + test "clear replication_confirmed* when introducing a new block in manifest" do + c = collections(:replication_desired_2_confirmed_2) + act_as_user users(:active) do + assert c.update_attributes(manifest_text: collections(:user_agreement).signed_manifest_text) + assert_nil c.replication_confirmed + assert_nil c.replication_confirmed_at + end + end + + test "don't clear replication_confirmed* when just renaming a file" do + c = collections(:replication_desired_2_confirmed_2) + act_as_user users(:active) do + new_manifest = c.signed_manifest_text.sub(':bar', ':foo') + assert c.update_attributes(manifest_text: new_manifest) + assert_equal 2, c.replication_confirmed + assert_not_nil c.replication_confirmed_at + end + end + + test "don't clear replication_confirmed* when just deleting a data block" do + c = collections(:replication_desired_2_confirmed_2) + act_as_user users(:active) do + new_manifest = c.signed_manifest_text + new_manifest.sub!(/ \S+:bar/, '') + new_manifest.sub!(/ acbd\S+/, '') + # We really deleted a block there, right? + assert_operator new_manifest.length+40, :<, c.signed_manifest_text.length + + assert c.update_attributes(manifest_text: new_manifest) + assert_equal 2, c.replication_confirmed + assert_not_nil c.replication_confirmed_at + end + end end -- 2.30.2