Merge branch 'prepare-3.2.2' into 3.2.X

CHANGELOG.rst

@@ -3,10 +3,14 @@ Changelog
 
 This document describes changes between each past release.
 
-3.2.2 (unreleased)
+3.2.2 (2016-07-13)
 ==================
 
-- Nothing changed yet.
+- 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)
 ==================

docs/conf.py

@@ -74,7 +74,7 @@
 # The short X.Y version.
 version = '3.2'
 # The full version, including alpha/beta/rc tags.
-release = '3.2.1'
+release = '3.2.2'
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.

kinto/core/permission/memory.py

@@ -118,7 +118,7 @@ def get_authorized_principals(self, object_id, permission,
     def get_object_permissions(self, object_id, permissions=None):
         if permissions is None:
             aces = [k for k in self._store.keys()
-                    if k.startswith('permission:%s' % object_id)]
+                    if k.startswith('permission:%s:' % object_id)]
         else:
             aces = ['permission:%s:%s' % (object_id, permission)
                     for permission in permissions]

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
@@ -254,6 +254,7 @@ def update(self, collection_id, parent_id, object_id, record,
         self.set_record_timestamp(collection_id, parent_id, record,
                                   modified_field=modified_field)
         self._store[collection_id][parent_id][object_id] = record
+        self._cemetery[collection_id][parent_id].pop(object_id, None)
         return record
 
     def delete(self, collection_id, parent_id, object_id,

kinto/core/storage/postgresql/__init__.py

@@ -66,7 +66,7 @@ class Storage(StorageBase):
 
     """  # NOQA
 
-    schema_version = 11
+    schema_version = 13
 
     def __init__(self, client, max_fetch_size, *args, **kwargs):
         super(Storage, self).__init__(*args, **kwargs)
@@ -270,6 +270,12 @@ def update(self, collection_id, parent_id, object_id, record,
                modified_field=DEFAULT_MODIFIED_FIELD,
                auth=None):
         query_create = """
+        WITH delete_potential_tombstone AS (
+            DELETE FROM deleted
+             WHERE id = :object_id
+               AND parent_id = :parent_id
+               AND collection_id = :collection_id
+        )
         INSERT INTO records (id, parent_id, collection_id, data, last_modified)
         VALUES (:object_id, :parent_id,
                 :collection_id, (:data)::JSONB,

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

@@ -0,0 +1,9 @@
+-- Select all existing records and delete their tombstone if any.
+DELETE FROM deleted d
+USING records r
+WHERE d.id = r.id
+AND   d.parent_id = r.parent_id
+AND   d.collection_id = r.collection_id;
+
+-- Bump storage schema version.
+INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '12');

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', '11');
+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)
@@ -209,6 +210,10 @@ def update(self, collection_id, parent_id, object_id, record,
                 '{0}.{1}.records'.format(collection_id, parent_id),
                 object_id
             )
+            multi.srem(
+                '{0}.{1}.deleted'.format(collection_id, parent_id),
+                object_id
+            )
             multi.execute()
 
         return record

kinto/tests/core/test_permission.py

@@ -342,6 +342,7 @@ def test_object_permissions_return_all_object_acls(self):
         self.permission.add_principal_to_ace('/url/a/id/1', 'write', 'user2')
         self.permission.add_principal_to_ace('/url/a/id/1', 'read', 'user3')
         self.permission.add_principal_to_ace('/url/a/id/1', 'obj:del', 'user1')
+        self.permission.add_principal_to_ace('/url/a/id/1/sub', 'create', 'me')
         permissions = self.permission.get_object_permissions('/url/a/id/1')
         self.assertDictEqual(permissions, {
             "write": {"user1", "user2"},

kinto/tests/core/test_storage.py

@@ -311,6 +311,16 @@ def test_delete_works_properly(self):
             **self.storage_kw
         )
 
+    def test_delete_works_even_on_second_time(self):
+        # Create a record
+        self.storage.create('test', '1234', {"id": "demo"})
+        # Delete the record
+        self.storage.delete('test', '1234', "demo", with_deleted=True)
+        # Update a record (it recreates it.)
+        self.storage.update('test', '1234', "demo", {"id": "demo"})
+        # Delete the record without errors
+        self.storage.delete('test', '1234', "demo", with_deleted=True)
+
     def test_delete_can_specify_the_last_modified(self):
         stored = self.create_record()
         last_modified = stored[self.modified_field] + 10
@@ -580,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]
@@ -620,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):

kinto/tests/core/test_storage_migrations.py

@@ -151,6 +151,52 @@ def test_every_available_migration_succeeds_if_tables_were_flushed(self):
         version = self.storage._get_installed_version()
         self.assertEqual(version, self.version)
 
+    def test_migration_12_clean_tombstones(self):
+        self._delete_everything()
+        postgresql.Storage.schema_version = 11
+        self.storage.initialize_schema()
+        # Set the schema version back to 11 in the base as well
+        with self.storage.client.connect() as conn:
+            query = """
+            UPDATE metadata SET value = '11'
+            WHERE name = 'storage_schema_version';
+            """
+            conn.execute(query)
+        r = self.storage.create('test', 'jean-louis', {'drink': 'mate'})
+        self.storage.delete('test', 'jean-louis', r['id'])
+
+        # Insert back the record without removing the tombstone.
+        with self.storage.client.connect() as conn:
+            query = """
+            INSERT INTO records (id, parent_id, collection_id,
+                                 data, last_modified)
+            VALUES (:id, :parent_id, :collection_id,
+                    (:data)::JSONB, from_epoch(:last_modified));
+            """
+            placeholders = dict(id=r['id'],
+                                collection_id='test',
+                                parent_id='jean-louis',
+                                data=json.dumps({'drink': 'mate'}),
+                                last_modified=1468400666777)
+            conn.execute(query, placeholders)
+
+        records, count = self.storage.get_all('test', 'jean-louis',
+                                              include_deleted=True)
+        # Check that we have the tombstone
+        assert len(records) == 2
+        assert count == 1
+
+        # Execute the 011 to 012 migration
+        postgresql.Storage.schema_version = 12
+        self.storage.initialize_schema()
+
+        # Check that the rotted tombstone have been removed.
+        records, count = self.storage.get_all('test', 'jean-louis',
+                                              include_deleted=True)
+        # Only the record remains.
+        assert len(records) == 1
+        assert count == 1
+
 
 class PostgresqlExceptionRaisedTest(unittest.TestCase):
     def setUp(self):

requirements.txt

@@ -9,7 +9,7 @@ iso8601==0.1.11
 jsonschema==2.5.1
 newrelic==2.66.0.49
 PasteDeploy==1.5.2
-psycopg2==2.6.1
+psycopg2==2.6.2
 pyramid==1.7
 pyramid-multiauth==0.8.0
 pyramid-tm==0.12.1
@@ -20,10 +20,10 @@ repoze.lru==0.6
 requests==2.10.0
 simplejson==3.8.2
 six==1.10.0
-SQLAlchemy==1.0.13
+SQLAlchemy==1.0.14
 statsd==3.2.1
 structlog==16.1.0
-transaction==1.6.0
+transaction==1.6.1
 translationstring==1.3
 ujson==1.35
 venusian==1.0
@@ -32,4 +32,4 @@ WebOb==1.6.1
 Werkzeug==0.11.10
 zope.deprecation==4.1.2
 zope.interface==4.1.3
-zope.sqlalchemy==0.7.6
+zope.sqlalchemy==0.7.7

setup.py

@@ -71,7 +71,7 @@ def read_file(filename):
 
 
 setup(name='kinto',
-      version='3.2.1',
+      version='3.2.2',
       description='Kinto Web Service - Store, Sync, Share, and Self-Host.',
       long_description=README + "\n\n" + CHANGELOG + "\n\n" + CONTRIBUTORS,
       license='Apache License (2.0)',