[ARVADOS] updated: 65121f8db54a1ed15207d050e1f48c5fc26d646b

Git user git at public.curoverse.com
Thu Mar 30 21:37:49 EDT 2017


Summary of changes:
 services/api/app/models/collection.rb            | 18 ++++++++++--------
 services/api/app/models/container_request.rb     |  8 +-------
 services/api/test/unit/collection_test.rb        | 23 ++++++++++++++---------
 services/api/test/unit/container_request_test.rb |  6 +++---
 4 files changed, 28 insertions(+), 27 deletions(-)

       via  65121f8db54a1ed15207d050e1f48c5fc26d646b (commit)
      from  4437774e863465c0daa41dfd9716174e18d93122 (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 65121f8db54a1ed15207d050e1f48c5fc26d646b
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Mar 30 18:47:12 2017 -0400

    11100: If caller sets collection.delete_at too early, set it to the earliest allowed time instead of failing.

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index b45c178..92300d6 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -527,14 +527,16 @@ class Collection < ArvadosModel
       errors.add :delete_at, "must be set if trash_at is set, and must be nil otherwise"
     end
 
-    earliest_delete = ([@validation_timestamp, trash_at_was].compact.min +
-                       Rails.configuration.blob_signature_ttl.seconds)
-    if delete_at && delete_at < earliest_delete
-      errors.add :delete_at, "#{delete_at} is too soon: earliest allowed is #{earliest_delete}"
-    end
-
-    if delete_at && delete_at < trash_at
-      errors.add :delete_at, "must not be earlier than trash_at"
+    if delete_at
+      if delete_at < trash_at
+        errors.add :delete_at, "must not be earlier than trash_at"
+      else
+        earliest_delete = ([@validation_timestamp, trash_at_was].compact.min +
+                           Rails.configuration.blob_signature_ttl.seconds)
+        if delete_at < earliest_delete
+          self.delete_at = earliest_delete
+        end
+      end
     end
 
     true
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index edd69d0..628ef88 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -102,18 +102,12 @@ class ContainerRequest < ArvadosModel
       next if pdh.nil?
       coll_name = "Container #{out_type} for request #{uuid}"
       trash_at = nil
-      delete_at = nil
       if out_type == 'output'
         if self.output_name
           coll_name = self.output_name
         end
         if self.output_ttl > 0
           trash_at = db_current_time + self.output_ttl
-          # delete_at cannot be sooner than blob_signature_ttl, even
-          # after the delay between now and the collection validation.
-          delete_at = db_current_time +
-                      [self.output_ttl,
-                       Rails.configuration.blob_signature_ttl + 60].max
         end
       end
       manifest = Collection.unscoped do
@@ -125,7 +119,7 @@ class ContainerRequest < ArvadosModel
                             portable_data_hash: pdh,
                             name: coll_name,
                             trash_at: trash_at,
-                            delete_at: delete_at,
+                            delete_at: trash_at,
                             properties: {
                               'type' => out_type,
                               'container_request' => uuid,
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 87bec21..882e260 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -370,21 +370,26 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  now = Time.now
   [['trash-to-delete interval negative',
     :collection_owned_by_active,
-    {trash_at: Time.now+2.weeks, delete_at: Time.now},
+    {trash_at: now+2.weeks, delete_at: now},
     {state: :invalid}],
-   ['trash-to-delete interval too short',
+   ['now-to-delete interval short',
     :collection_owned_by_active,
-    {trash_at: Time.now+3.days, delete_at: Time.now+7.days},
-    {state: :invalid}],
+    {trash_at: now+3.days, delete_at: now+7.days},
+    {state: :trash_future}],
+   ['now-to-delete interval short, trash=delete',
+    :collection_owned_by_active,
+    {trash_at: now+3.days, delete_at: now+3.days},
+    {state: :trash_future}],
    ['trash-to-delete interval ok',
     :collection_owned_by_active,
-    {trash_at: Time.now, delete_at: Time.now+15.days},
+    {trash_at: now, delete_at: now+15.days},
     {state: :trash_now}],
    ['trash-to-delete interval short, but far enough in future',
     :collection_owned_by_active,
-    {trash_at: Time.now+13.days, delete_at: Time.now+15.days},
+    {trash_at: now+13.days, delete_at: now+15.days},
     {state: :trash_future}],
    ['trash by setting is_trashed bool',
     :collection_owned_by_active,
@@ -392,11 +397,11 @@ class CollectionTest < ActiveSupport::TestCase
     {state: :trash_now}],
    ['trash in future by setting just trash_at',
     :collection_owned_by_active,
-    {trash_at: Time.now+1.week},
+    {trash_at: now+1.week},
     {state: :trash_future}],
    ['trash in future by setting trash_at and delete_at',
     :collection_owned_by_active,
-    {trash_at: Time.now+1.week, delete_at: Time.now+4.weeks},
+    {trash_at: now+1.week, delete_at: now+4.weeks},
     {state: :trash_future}],
    ['untrash by clearing is_trashed bool',
     :expired_collection,
@@ -416,7 +421,7 @@ class CollectionTest < ActiveSupport::TestCase
         end
         updates_ok = c.update_attributes(updates)
         expect_valid = expect[:state] != :invalid
-        assert_equal updates_ok, expect_valid, c.errors.full_messages.to_s
+        assert_equal expect_valid, updates_ok, c.errors.full_messages.to_s
         case expect[:state]
         when :invalid
           refute c.valid?
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index f9ce41c..a3dd1f9 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -627,15 +627,15 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_not_nil(trash)
     assert_not_nil(delete)
     assert_in_delta(trash, now + 1.second, 10)
-    assert_in_delta(delete, now + Rails.configuration.blob_signature_ttl.second, 120)
+    assert_in_delta(delete, now + Rails.configuration.blob_signature_ttl.second, 10)
   end
 
   def check_output_ttl_1y(now, trash, delete)
     year = (86400*365).second
     assert_not_nil(trash)
     assert_not_nil(delete)
-    assert_in_delta(trash, now + year, 20)
-    assert_in_delta(delete, now + year, 20)
+    assert_in_delta(trash, now + year, 10)
+    assert_in_delta(delete, now + year, 10)
   end
 
   def run_container(cr)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list