[ARVADOS] updated: 1.3.0-615-g425836b28

[Git user]

Summary of changes:
 services/api/app/models/collection.rb              |  7 ++++++-
 services/api/test/fixtures/collections.yml         | 16 +++++++++++++++
 .../arvados/v1/collections_controller_test.rb      |  6 +++---
 services/api/test/unit/collection_test.rb          | 24 +++++++++++++++-------
 4 files changed, 42 insertions(+), 11 deletions(-)

       via  425836b285a32c31ef643f8c5d4b48b8b42b7ac4 (commit)
      from  79316b0ebbf5bfe934267579478b89f770c0e5ba (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 425836b285a32c31ef643f8c5d4b48b8b42b7ac4
Author: Eric Biagiotti <ebiagiotti at veritasgenetics.com>
Date:   Wed Apr 3 16:01:26 2019 -0400

    14484: Improves test coverage and attribute setting performance
    
    Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <ebiagiotti at veritasgenetics.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index bcf510d88..578f25a62 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -199,10 +199,15 @@ class Collection < ArvadosModel
   end
 
   def set_file_count_and_total_size
-    if self.manifest_text_changed? || self.file_count_changed? || self.file_size_total_changed?
+    # Only update the file stats if the manifest changed
+    if self.manifest_text_changed?
       m = Keep::Manifest.new(self.manifest_text)
       self.file_size_total = m.files_size
       self.file_count = m.files_count
+    # If the manifest didn't change but the attributes did, ignore the changes
+    elsif self.file_count_changed? || self.file_size_total_changed?
+      self.file_count = self.file_count_was
+      self.file_size_total = self.file_size_total_was
     end
     true
   end
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 8763f3944..c84e479e4 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -29,6 +29,22 @@ collection_owned_by_active:
   name: owned_by_active
   version: 2
 
+collection_owned_by_active_with_file_stats:
+  uuid: zzzzz-4zz18-fjeod4od92kfj5f
+  current_version_uuid: zzzzz-4zz18-fjeod4od92kfj5f
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-02-03T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2014-02-03T17:22:54Z
+  updated_at: 2014-02-03T17:22:54Z
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  file_count: 1
+  file_size_total: 3
+  name: owned_by_active_with_file_stats
+  version: 2
+
 collection_owned_by_active_past_version_1:
   uuid: zzzzz-4zz18-znfnqtbbv4spast
   current_version_uuid: zzzzz-4zz18-bv31uwvy3neko21
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 ccc875c15..27d4fee79 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -958,7 +958,7 @@ EOS
   test "update collection manifest and expect new file stats" do
     authorize_with :active
     post :update, {
-      id: 'zzzzz-4zz18-bv31uwvy3neko21',
+      id: collections(:collection_owned_by_active_with_file_stats).uuid,
       collection: {
         manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n"
       }
@@ -989,10 +989,10 @@ EOS
     ['file_count', 1],
     ['file_size_total', 3]
   ].each do |attribute, val|
-    test "update collection with #{attribute} and expect overwrite" do
+    test "update collection with #{attribute} and expect ignore" do
       authorize_with :active
       post :update, {
-        id: 'zzzzz-4zz18-bv31uwvy3neko21',
+        id: collections(:collection_owned_by_active_with_file_stats).uuid,
         collection: {
           "#{attribute}": 10
         }
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index df487d965..8deedee01 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -76,24 +76,34 @@ 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 ignore and overwrite
+      # Direct changes to file stats should be ignored
       c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n")
       c.file_count = 6
       c.file_size_total = 30
       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 
-      c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:34:foo2.txt\n")
-      c.reload
+      # File stats specified on create should be ignored and overwritten
+      c = Collection.create(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", file_count: 10, file_size_total: 10)
+      assert c.valid?
+      assert_equal 1, c.file_count
+      assert_equal 34, c.file_size_total
+
+      # Updating the manifest should change file stats
+      c.update_attributes(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:34:foo2.txt\n")
       assert c.valid?
       assert_equal 2, c.file_count
       assert_equal 68, c.file_size_total
 
-      # 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)
+      # Updating file stats and the manifest should use manifest values
+      c.update_attributes(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", file_count:10, file_size_total: 10)
+      assert c.valid?
+      assert_equal 1, c.file_count
+      assert_equal 34, c.file_size_total
+
+      # Updating just the file stats should be ignored
+      c.update_attributes(file_count: 10, file_size_total: 10)
       assert c.valid?
       assert_equal 1, c.file_count
       assert_equal 34, c.file_size_total

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


hooks/post-receive
--