[ARVADOS] created: 2.1.0-217-g653f8d8ba

Git user git at public.arvados.org
Thu Dec 10 22:21:47 UTC 2020


        at  653f8d8ba8d07a2d7081a924a67ee31b1a8ceebd (commit)


commit 653f8d8ba8d07a2d7081a924a67ee31b1a8ceebd
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Dec 10 17:58:58 2020 -0300

    17152: Avoids creating audit logs when only 'preserve_version' is updated.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 6b308a231..9290e01a1 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -394,7 +394,6 @@ class ApiClientAuthorization < ArvadosModel
   end
 
   def log_update
-
     super unless (saved_changes.keys - UNLOGGED_CHANGES).empty?
   end
 end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 919d2fcb4..e3802734a 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -64,6 +64,8 @@ class Collection < ArvadosModel
     t.add :file_size_total
   end
 
+  UNLOGGED_CHANGES = ['preserve_version', 'updated_at']
+
   after_initialize do
     @signatures_checked = false
     @computed_pdh_for_manifest_text = false
@@ -751,4 +753,8 @@ class Collection < ArvadosModel
     self.current_version_uuid ||= self.uuid
     true
   end
+
+  def log_update
+    super unless (saved_changes.keys - UNLOGGED_CHANGES).empty?
+  end
 end
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 61bb3f79f..1f2eab73a 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -21,7 +21,7 @@ collection_owned_by_active:
   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_client_uuid: zzzzz-ozdt8-teyxzyd8qllg11h
   modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
   modified_at: 2014-02-03T18:22:54Z
   updated_at: 2014-02-03T18:22:54Z
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 76d78f9ea..66c8c8d92 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -228,6 +228,20 @@ class LogTest < ActiveSupport::TestCase
     assert_logged(auth, :update)
   end
 
+  test "don't log changes only to Collection.preserve_version" do
+    set_user_from_auth :admin_trustedclient
+    col = collections(:collection_owned_by_active)
+    start_log_count = get_logs_about(col).size
+    assert_equal false, col.preserve_version
+    col.preserve_version = true
+    col.save!
+    assert_equal(start_log_count, get_logs_about(col).size,
+                 "log count changed after updating Collection.preserve_version")
+    col.name = 'updated by admin'
+    col.save!
+    assert_logged(col, :update)
+  end
+
   test "token isn't included in ApiClientAuthorization logs" do
     set_user_from_auth :admin_trustedclient
     auth = ApiClientAuthorization.new

commit 8239993669bd614c2aaa1c014e870990df3a8354
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Dec 10 16:53:49 2020 -0300

    17152: Changes preserve_version semantics, updates related documentation.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 3f622112e..ac697d870 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -35,10 +35,14 @@ TODO: extract this information based on git commit messages and generate changel
 <div class="releasenotes">
 </notextile>
 
-h2(#main). development main (as of 2020-10-28)
+h2(#main). development main (as of 2020-12-10)
 
 "Upgrading from 2.1.0":#v2_1_0
 
+h3. Changes on the collection's @preserve_version@ attribute semantics
+
+The @preserve_version@ attribute on collections was originally designed to allow clients to persist a preexisting collection version. This forced clients to make 2 requests if the intention is to "make this set of changes in a new version that will be kept", so we have changed the semantics to do just that: When passing @preserve_version=true@ along with other collection updates, the current version is persisted and also the newly created one will be persisted on the next update.
+
 h3. Centos7 Python 3 dependency upgraded to python3
 
 Now that Python 3 is part of the base repository in CentOS 7, the Python 3 dependency for Centos7 Arvados packages was changed from SCL rh-python36 to python3.
diff --git a/doc/api/methods/collections.html.textile.liquid b/doc/api/methods/collections.html.textile.liquid
index 4372cc2f5..fd4a36f29 100644
--- a/doc/api/methods/collections.html.textile.liquid
+++ b/doc/api/methods/collections.html.textile.liquid
@@ -38,7 +38,7 @@ table(table table-bordered table-condensed).
 |is_trashed|boolean|True if @trash_at@ is in the past, false if not.||
 |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.||
+|preserve_version|boolean|When set to true on a current version, it will be persisted. When passing @true@ as part of a bigger update call, both current and newly created versions are persisted.||
 |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.||
 
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 3637f34e1..919d2fcb4 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -37,6 +37,8 @@ class Collection < ArvadosModel
   validate :protected_managed_properties_updates, on: :update
   after_validation :set_file_count_and_total_size
   before_save :set_file_names
+  before_update :preserve_version_exclusive_updates_leave_modified_at_alone,
+    if: Proc.new { |col| col.changes.keys.sort == ['modified_at', 'updated_at', 'preserve_version'].sort }
   around_update :manage_versioning, unless: :is_past_version?
 
   api_accessible :user, extend: :common do |t|
@@ -286,7 +288,9 @@ class Collection < ArvadosModel
 
       if should_preserve_version
         self.version += 1
-        self.preserve_version = false
+        if !changes.keys.include?('preserve_version')
+          self.preserve_version = false
+        end
       end
 
       yield
@@ -305,6 +309,10 @@ class Collection < ArvadosModel
     end
   end
 
+  def preserve_version_exclusive_updates_leave_modified_at_alone
+    self.modified_at = self.modified_at_was
+  end
+
   def syncable_updates
     updates = {}
     if self.changes.any?
@@ -359,6 +367,7 @@ class Collection < ArvadosModel
 
     idle_threshold = Rails.configuration.Collections.PreserveVersionIfIdle
     if !self.preserve_version_was &&
+      !self.preserve_version &&
       (idle_threshold < 0 ||
         (idle_threshold > 0 && self.modified_at_was > db_current_time-idle_threshold.seconds))
       return false

commit d9a6e1a21c215def927b78231f9c10b388ac6b47
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Dec 10 16:04:52 2020 -0300

    17152: Updates collection tests to reflect the new behavior.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index a28893e01..70645615a 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -188,7 +188,7 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
-  test "preserve_version=false assignment is ignored while being true and not producing a new version" do
+  test "preserve_version updates" do
     Rails.configuration.Collections.CollectionVersioning = true
     Rails.configuration.Collections.PreserveVersionIfIdle = 3600
     act_as_user users(:active) do
@@ -199,28 +199,58 @@ class CollectionTest < ActiveSupport::TestCase
       assert_equal false, c.preserve_version
       # This update shouldn't produce a new version, as the idle time is not up
       c.update_attributes!({
-        'name' => 'bar',
-        'preserve_version' => true
+        'name' => 'bar'
       })
       c.reload
       assert_equal 1, c.version
       assert_equal 'bar', c.name
+      assert_equal false, c.preserve_version
+      # This update should produce a new version, even if the idle time is not up
+      # and also keep the preserve_version=true flag to persist it.
+      c.update_attributes!({
+        'name' => 'baz',
+        'preserve_version' => true
+      })
+      c.reload
+      assert_equal 2, c.version
+      assert_equal 'baz', c.name
       assert_equal true, c.preserve_version
       # Make sure preserve_version is not disabled after being enabled, unless
       # a new version is created.
+      # This is a non-versionable update
       c.update_attributes!({
         'preserve_version' => false,
         'replication_desired' => 2
       })
       c.reload
-      assert_equal 1, c.version
+      assert_equal 2, c.version
       assert_equal 2, c.replication_desired
       assert_equal true, c.preserve_version
+      # This is a versionable update
       c.update_attributes!({'name' => 'foobar'})
       c.reload
-      assert_equal 2, c.version
+      assert_equal 3, c.version
       assert_equal false, c.preserve_version
       assert_equal 'foobar', c.name
+      # Flipping only 'preserve_version' to true doesn't create a new version
+      c.update_attributes!({'preserve_version' => true})
+      c.reload
+      assert_equal 3, c.version
+      assert_equal true, c.preserve_version
+    end
+  end
+
+  test "preserve_version updates don't change modified_at timestamp" do
+    act_as_user users(:active) do
+      c = create_collection 'foo', Encoding::US_ASCII
+      assert c.valid?
+      assert_equal false, c.preserve_version
+      modified_at = c.modified_at.to_f
+      c.update_attributes!({'preserve_version' => true})
+      c.reload
+      assert_equal true, c.preserve_version
+      assert_equal modified_at, c.modified_at.to_f,
+        'preserve_version updates should not trigger modified_at changes'
     end
   end
 

commit 97c83ddd8852d5ca445527f9914a31a8976a9031
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Dec 10 16:04:05 2020 -0300

    17152: Updates API revision number.
    
    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/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index b9aba2726..9e1939799 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -36,7 +36,7 @@ class Arvados::V1::SchemaController < ApplicationController
         # format is YYYYMMDD, must be fixed width (needs to be lexically
         # sortable), updated manually, may be used by clients to
         # determine availability of API server features.
-        revision: "20200331",
+        revision: "20201210",
         source_version: AppVersion.hash,
         sourceVersion: AppVersion.hash, # source_version should be deprecated in the future
         packageVersion: AppVersion.package_version,

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list