[ARVADOS] updated: 1.3.0-614-g79316b0eb
Git user
git at public.curoverse.com
Tue Apr 2 15:51:32 UTC 2019
Summary of changes:
doc/api/methods/collections.html.textile.liquid | 4 +-
services/api/app/models/collection.rb | 16 +----
.../arvados/v1/collections_controller_test.rb | 68 +++++++++-------------
services/api/test/unit/collection_test.rb | 9 ++-
4 files changed, 36 insertions(+), 61 deletions(-)
via 79316b0ebbf5bfe934267579478b89f770c0e5ba (commit)
from dd2e6f664a3e59e02349901a04e182bda6286f6f (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 79316b0ebbf5bfe934267579478b89f770c0e5ba
Author: Eric Biagiotti <ebiagiotti at veritasgenetics.com>
Date: Tue Apr 2 11:49:29 2019 -0400
14484: Update collection model to overwrite file stats if a change is attempted
Also updates tests and documentation
Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <ebiagiotti at veritasgenetics.com>
diff --git a/doc/api/methods/collections.html.textile.liquid b/doc/api/methods/collections.html.textile.liquid
index 38253d6bc..d611c5b16 100644
--- a/doc/api/methods/collections.html.textile.liquid
+++ b/doc/api/methods/collections.html.textile.liquid
@@ -39,8 +39,8 @@ table(table table-bordered table-condensed).
|current_version_uuid|string|UUID of the collection's current version. On new collections, it'll be equal to the @uuid@ attribute.||
|version|number|Version number, starting at 1 on new collections. This attribute is read-only.||
|preserve_version|boolean|When set to true on a current version, it will be saved on the next versionable update.||
-|file_count|number|The total number of files in the collection. ||
-|file_size_total|number|The sum of the file sizes in the collection. ||
+|file_count|number|The total number of files in the collection. This attribute is read-only.||
+|file_size_total|number|The sum of the file sizes in the collection. This attribute is read-only.||
h3. Conditions of creating a Collection
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 3c7f7c745..bcf510d88 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -29,7 +29,6 @@ class Collection < ArvadosModel
validate :ensure_storage_classes_contain_non_empty_strings
validate :versioning_metadata_updates, on: :update
validate :past_versions_cannot_be_updated, on: :update
- validate :file_count_and_size_cannot_be_changed, on: :update
after_validation :set_file_count_and_total_size
before_save :set_file_names
around_update :manage_versioning
@@ -200,7 +199,7 @@ class Collection < ArvadosModel
end
def set_file_count_and_total_size
- if self.manifest_text_changed?
+ if self.manifest_text_changed? || self.file_count_changed? || self.file_size_total_changed?
m = Keep::Manifest.new(self.manifest_text)
self.file_size_total = m.files_size
self.file_count = m.files_count
@@ -650,19 +649,6 @@ class Collection < ArvadosModel
end
end
- def file_count_and_size_cannot_be_changed
- valid = true
- if file_count_changed?
- errors.add(:file_count, "cannot be changed")
- valid = false
- end
- if file_size_total_changed?
- errors.add(:file_size_total, "cannot be changed")
- valid = false
- end
- valid
- end
-
def versioning_metadata_updates
valid = true
if (current_version_uuid_was == uuid_was) && current_version_uuid_changed?
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 3bcb1ec1b..ccc875c15 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -955,21 +955,7 @@ EOS
end
end
- test "create collection with file stats and expect overwrite" do
- authorize_with :active
- post :create, {
- collection: {
- manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n",
- file_count: 10,
- file_size_total: 100
- }
- }
- assert_response 200
- assert_equal 1, json_response['file_count']
- assert_equal 34, json_response['file_size_total']
- end
-
- test "update collection manifest and expect file stats" do
+ test "update collection manifest and expect new file stats" do
authorize_with :active
post :update, {
id: 'zzzzz-4zz18-bv31uwvy3neko21',
@@ -982,34 +968,38 @@ EOS
assert_equal 34, json_response['file_size_total']
end
- test "update collection file count and expect error" do
- authorize_with :active
- post :update, {
- id: 'zzzzz-4zz18-znfnqtbbv4spc3w',
- collection: {
- file_count: 10
+ [
+ ['file_count', 1],
+ ['file_size_total', 34]
+ ].each do |attribute, val|
+ test "create collection with #{attribute} and expect overwrite" do
+ authorize_with :active
+ post :create, {
+ collection: {
+ manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n",
+ "#{attribute}": 10
+ }
}
- }
- assert_response 422
- response_errors = json_response['errors']
- assert_not_nil response_errors, 'Expected error in response'
- assert(response_errors.first.include?('File count cannot be changed'),
- "Expected file count error in #{response_errors.first}")
+ assert_response 200
+ assert_equal val, json_response[attribute]
+ end
end
- test "update collection file size and expect error" do
- authorize_with :active
- post :update, {
- id: 'zzzzz-4zz18-znfnqtbbv4spc3w',
- collection: {
- file_size_total: 10
+ [
+ ['file_count', 1],
+ ['file_size_total', 3]
+ ].each do |attribute, val|
+ test "update collection with #{attribute} and expect overwrite" do
+ authorize_with :active
+ post :update, {
+ id: 'zzzzz-4zz18-bv31uwvy3neko21',
+ collection: {
+ "#{attribute}": 10
+ }
}
- }
- assert_response 422
- response_errors = json_response['errors']
- assert_not_nil response_errors, 'Expected error in response'
- assert(response_errors.first.include?('File size total cannot be changed'),
- "Expected file size total error in #{response_errors.first}")
+ assert_response 200
+ assert_equal val, json_response[attribute]
+ end
end
[
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 8538b35b3..df487d965 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -76,25 +76,24 @@ class CollectionTest < ActiveSupport::TestCase
test "file stats cannot be changed unless through manifest change" do
act_as_system_user do
- # Changing file stats via an update should fail validation
+ # Changing file stats via an update should ignore and overwrite
c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n")
c.file_count = 6
c.file_size_total = 30
- assert !c.valid?
+ assert c.valid?
c.reload
assert_equal 1, c.file_count
assert_equal 34, c.file_size_total
- # Changing the file stats via manifest change validates
+ # Changing the file stats via manifest change
c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:34:foo2.txt\n")
c.reload
assert c.valid?
assert_equal 2, c.file_count
assert_equal 68, c.file_size_total
- # Changing file stats at create, will silently overwrite
+ # Changing file stats at create will ignore and overwrite
c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", file_count: 10, file_size_total: 10)
- c.reload
assert c.valid?
assert_equal 1, c.file_count
assert_equal 34, c.file_size_total
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list