Merge pull request Kinto#713 from Kinto/specified-timestamp-equal

Bump last_modified in storage when provided value is equal to previous

CHANGELOG.rst

@@ -8,6 +8,9 @@ This document describes changes between each past release.
 
 - Fix bug in object permissions with memory backend (#708)
 - Make sure the tombstone is deleted when the record is created with PUT. (#715)
+- Bump ``last_modified`` on record when provided value is equal to previous
+  in storage ``update()`` method (#713)
+
 
 3.2.1 (2016-07-04)
 ==================

kinto/core/storage/memory.py

@@ -208,7 +208,7 @@ def _bump_timestamp(self, collection_id, parent_id, record=None,
         # In case the timestamp was specified, the collection timestamp will
         # be different from the updated timestamp. As such, we want to return
         # the one of the record, and not the collection one.
-        if not is_specified:
+        if not is_specified or previous == current:
             current = collection_timestamp
 
         self._timestamps[collection_id][parent_id] = collection_timestamp

kinto/core/storage/postgresql/__init__.py

@@ -66,7 +66,7 @@ class Storage(StorageBase):
 
     """  # NOQA
 
-    schema_version = 12
+    schema_version = 13
 
     def __init__(self, client, max_fetch_size, *args, **kwargs):
         super(Storage, self).__init__(*args, **kwargs)

kinto/core/storage/postgresql/migrations/migration_012_013.sql

@@ -0,0 +1,71 @@
+--
+-- Triggers to set last_modified on INSERT/UPDATE
+--
+DROP TRIGGER IF EXISTS tgr_records_last_modified ON records;
+DROP TRIGGER IF EXISTS tgr_deleted_last_modified ON deleted;
+
+CREATE OR REPLACE FUNCTION bump_timestamp()
+RETURNS trigger AS $$
+DECLARE
+    previous TIMESTAMP;
+    current TIMESTAMP;
+
+BEGIN
+    previous := NULL;
+    SELECT last_modified INTO previous
+      FROM timestamps
+     WHERE parent_id = NEW.parent_id
+       AND collection_id = NEW.collection_id;
+
+    --
+    -- This bumps the current timestamp to 1 msec in the future if the previous
+    -- timestamp is equal to the current one (or higher if was bumped already).
+    --
+    -- If a bunch of requests from the same user on the same collection
+    -- arrive in the same millisecond, the unicity constraint can raise
+    -- an error (operation is cancelled).
+    -- See https://github.com/mozilla-services/cliquet/issues/25
+    --
+    current := clock_timestamp();
+    IF previous IS NOT NULL AND previous >= current THEN
+        current := previous + INTERVAL '1 milliseconds';
+    END IF;
+
+    IF NEW.last_modified IS NULL OR
+       (previous IS NOT NULL AND as_epoch(NEW.last_modified) = as_epoch(previous)) THEN
+        -- If record does not carry last-modified, or if the one specified
+        -- is equal to previous, assign it to current (i.e. bump it).
+        NEW.last_modified := current;
+    ELSE
+        -- Use record last-modified as collection timestamp.
+        IF previous IS NULL OR NEW.last_modified > previous THEN
+            current := NEW.last_modified;
+        END IF;
+    END IF;
+
+    --
+    -- Upsert current collection timestamp.
+    --
+    WITH upsert AS (
+        UPDATE timestamps SET last_modified = current
+         WHERE parent_id = NEW.parent_id AND collection_id = NEW.collection_id
+        RETURNING *
+    )
+    INSERT INTO timestamps (parent_id, collection_id, last_modified)
+    SELECT NEW.parent_id, NEW.collection_id, current
+    WHERE NOT EXISTS (SELECT * FROM upsert);
+
+    RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE TRIGGER tgr_records_last_modified
+BEFORE INSERT OR UPDATE ON records
+FOR EACH ROW EXECUTE PROCEDURE bump_timestamp();
+
+CREATE TRIGGER tgr_deleted_last_modified
+BEFORE INSERT OR UPDATE ON deleted
+FOR EACH ROW EXECUTE PROCEDURE bump_timestamp();
+
+-- Bump storage schema version.
+INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '13');

kinto/core/storage/postgresql/schema.sql

@@ -194,9 +194,10 @@ BEGIN
         current := previous + INTERVAL '1 milliseconds';
     END IF;
 
-
-    IF NEW.last_modified IS NULL THEN
-        -- If record does not carry last-modified, assign it to current.
+    IF NEW.last_modified IS NULL OR
+       (previous IS NOT NULL AND as_epoch(NEW.last_modified) = as_epoch(previous)) THEN
+        -- If record does not carry last-modified, or if the one specified
+        -- is equal to previous, assign it to current (i.e. bump it).
         NEW.last_modified := current;
     ELSE
         -- Use record last-modified as collection timestamp.
@@ -241,4 +242,4 @@ INSERT INTO metadata (name, value) VALUES ('created_at', NOW()::TEXT);
 
 -- Set storage schema version.
 -- Should match ``kinto.core.storage.postgresql.PostgreSQL.schema_version``
-INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '12');
+INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '13');

kinto/core/storage/redis.py

@@ -123,7 +123,8 @@ def _bump_timestamp(self, collection_id, parent_id, record=None,
 
                     # Return the newly generated timestamp as the current one
                     # only if nothing else was specified.
-                    if not is_specified:
+                    is_equal = previous and int(previous) == current
+                    if not is_specified or is_equal:
                         current = collection_timestamp
 
                     pipe.set(key, collection_timestamp)

kinto/tests/core/test_storage.py

@@ -590,6 +590,27 @@ def test_create_ignores_specified_last_modified_if_in_the_past(self):
         timestamp = self.storage.collection_timestamp(**self.storage_kw)
         self.assertGreater(timestamp, timestamp_before)
 
+    def test_create_ignores_specified_last_modified_if_equal(self):
+        # Create a first record, and get the timestamp.
+        first_record = self.create_record()
+        timestamp_before = first_record[self.modified_field]
+
+        # Create a new record with its timestamp in the past.
+        record = self.record.copy()
+        record[self.id_field] = RECORD_ID
+        record[self.modified_field] = timestamp_before
+        self.create_record(record=record)
+
+        # Check that record timestamp is the one specified.
+        retrieved = self.storage.get(object_id=RECORD_ID, **self.storage_kw)
+        self.assertGreater(retrieved[self.modified_field], timestamp_before)
+        self.assertGreater(retrieved[self.modified_field],
+                           record[self.modified_field])
+
+        # Check that collection timestamp was bumped (change happened).
+        timestamp = self.storage.collection_timestamp(**self.storage_kw)
+        self.assertGreater(timestamp, timestamp_before)
+
     def test_update_uses_specified_last_modified_if_in_future(self):
         stored = self.create_record()
         record_id = stored[self.id_field]
@@ -630,6 +651,25 @@ def test_update_ignores_specified_last_modified_if_in_the_past(self):
         timestamp = self.storage.collection_timestamp(**self.storage_kw)
         self.assertGreater(timestamp, timestamp_before)
 
+    def test_update_ignores_specified_last_modified_if_equal(self):
+        stored = self.create_record()
+        record_id = stored[self.id_field]
+        timestamp_before = stored[self.modified_field]
+
+        # Do not change the timestamp.
+        self.storage.update(object_id=record_id, record=stored,
+                            **self.storage_kw)
+
+        # Check that record timestamp was bumped.
+        retrieved = self.storage.get(object_id=record_id, **self.storage_kw)
+        self.assertGreater(retrieved[self.modified_field], timestamp_before)
+        self.assertGreater(retrieved[self.modified_field],
+                           stored[self.modified_field])
+
+        # Check that collection timestamp was bumped (change happened).
+        timestamp = self.storage.collection_timestamp(**self.storage_kw)
+        self.assertGreater(timestamp, timestamp_before)
+
 
 class FieldsUnicityTest(object):
     def test_does_not_fail_if_no_unique_fields_at_all(self):