9278: Set expires_at=now if a client sets it to a time in the past.

The definition of "now" in the default collection scope changes from
current_timestamp (time the current transaction started) to
statement_timestamp() (time the current statement started) so a test
case can expire a collection and then confirm that it is not in the
default scope, all within a single test transaction.

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index f1e7b4f1e164c8525118a5d62c5f06de3d5e54e1..459eacc77cbfc02f37d8c0f45d9b337a4e9631a0 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -15,9 +15,10 @@ class Collection < ArvadosModel
   before_validation :strip_signatures_and_update_replication_confirmed
   validate :ensure_pdh_matches_manifest_text
   before_save :set_file_names
+  before_save :expires_at_not_in_past
 
   # Query only undeleted collections by default.
-  default_scope where("expires_at IS NULL or expires_at > CURRENT_TIMESTAMP")
+  default_scope where("expires_at IS NULL or expires_at > statement_timestamp()")
 
   api_accessible :user, extend: :common do |t|
     t.add :name
@@ -381,4 +382,14 @@ class Collection < ArvadosModel
     end
     super
   end
+
+  # If expires_at is being changed to a time in the past, change it to
+  # now. This allows clients to say "expires {client-current-time}"
+  # without failing due to clock skew, while avoiding odd log entries
+  # like "expiry date changed to {1 year ago}".
+  def expires_at_not_in_past
+    if expires_at_changed? and expires_at
+      self.expires_at = [db_current_time, expires_at].max
+    end
+  end
 end

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index c81d543ebb02403866535561fd7b179aba6e8adb..536d9ca47e8a378f40d15da3058815eb2b000862 100644 (file)
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -1,6 +1,8 @@
 require 'test_helper'
 
 class CollectionTest < ActiveSupport::TestCase
+  include DbCurrentTime
+
   def create_collection name, enc=nil
     txt = ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:#{name}.txt\n"
     txt.force_encoding(enc) if enc
@@ -340,4 +342,14 @@ class CollectionTest < ActiveSupport::TestCase
     coll_uuids = coll_list.map(&:uuid)
     assert_includes(coll_uuids, collections(:docker_image).uuid)
   end
+
+  test 'expires_at cannot be set too far in the past' do
+    act_as_user users(:active) do
+      t0 = db_current_time
+      c = Collection.create!(manifest_text: '', name: 'foo')
+      c.update_attributes! expires_at: (t0 - 2.weeks)
+      c.reload
+      assert_operator c.expires_at, :>, t0
+    end
+  end
 end