Merge pull request #72 from jsangmeister/fix-create-reserve-ids

Adjust id_sequences on create events

writer/tests/integration/reserve_ids/test_reserve_ids.py

@@ -1,3 +1,4 @@
+from collections import defaultdict
 from unittest.mock import MagicMock
 
 import pytest
@@ -18,19 +19,16 @@ class FakeConnectionHandler:
     # We do just need the following three methods from the connection handler
 
     def __init__(self):
-        self.storage = {}
+        self.storage = defaultdict(lambda: 1)
 
     def get_connection_context(self):
         return MagicMock()
 
-    def execute(self, statement, arguments):
-        collection = arguments[0]
-        id = arguments[1]
-        self.storage[collection] = id
-
     def query_single_value(self, query, arguments):
         collection = arguments[0]
-        return self.storage.get(collection)
+        amount = arguments[1]
+        self.storage[collection] += amount - 1
+        return self.storage[collection]
 
 
 @pytest.fixture(autouse=True)

writer/tests/system/test_write_reserve_ids.py

@@ -0,0 +1,49 @@
+import copy
+
+import pytest
+
+from shared.tests.util import assert_response_code
+from writer.flask_frontend.routes import RESERVE_IDS_URL, WRITE_URL
+
+
+@pytest.fixture()
+def data():
+    yield copy.deepcopy(
+        {
+            "user_id": 1,
+            "information": {},
+            "locked_fields": {},
+            "events": [{"type": "create", "fqid": "a/1", "fields": {"f": 1}}],
+        }
+    )
+
+
+def test_create_reserve(json_client, data, db_cur):
+    response = json_client.post(WRITE_URL, data)
+    assert_response_code(response, 201)
+
+    response = json_client.post(RESERVE_IDS_URL, {"amount": 1, "collection": "a"})
+    assert response.status_code == 200
+
+    assert response.json == {"ids": [2]}
+
+    db_cur.execute("select * from id_sequences")
+    result = db_cur.fetchall()
+    assert result == [("a", 3)]
+
+
+def test_reserve_create(json_client, data, db_cur):
+    response = json_client.post(RESERVE_IDS_URL, {"amount": 5, "collection": "a"})
+    assert_response_code(response, 200)
+    assert response.json == {"ids": [1, 2, 3, 4, 5]}
+
+    db_cur.execute("select * from id_sequences")
+    result = db_cur.fetchall()
+    assert result == [("a", 6)]
+
+    response = json_client.post(WRITE_URL, data)
+    assert_response_code(response, 201)
+
+    db_cur.execute("select * from id_sequences")
+    result = db_cur.fetchall()
+    assert result == [("a", 6)]

writer/tests/system/write/test_write.py

@@ -45,6 +45,26 @@ def test_create_simple(json_client, data, redis_connection, reset_redis_data):
     create_model(json_client, data, redis_connection, reset_redis_data)
 
 
+def test_assert_increased_id_sequence(
+    json_client, data, redis_connection, reset_redis_data, db_cur
+):
+    create_model(json_client, data, redis_connection, reset_redis_data)
+    db_cur.execute("select id from id_sequences where collection = %s", ["a"])
+    id = db_cur.fetchone()[0]
+    assert id == 2
+
+
+def test_create_double_assert_increased_id_sequence(
+    json_client, data, redis_connection, reset_redis_data, db_cur
+):
+    create_model(json_client, data, redis_connection, reset_redis_data)
+    data["events"][0]["fqid"] = "a/3"
+    json_client.post(WRITE_URL, data)
+    db_cur.execute("select id from id_sequences where collection = %s", ["a"])
+    id = db_cur.fetchone()[0]
+    assert id == 4
+
+
 def test_create_empty_field(json_client, data, redis_connection):
     data["events"][0]["fields"]["empty"] = None
     response = json_client.post(WRITE_URL, data)

writer/tests/unit/postgresql_backend/test_sql_database_backend_service.py

@@ -315,7 +315,7 @@ def test_insert(self, sql_backend, connection):
 
         sql_backend.insert_create_event(event, position)
 
-        ex.assert_called_once()
+        ex.assert_called()
         ide.assert_called_with(
             event, [position, event.fqid, EVENT_TYPES.CREATE, "test_data"], position
         )
@@ -470,26 +470,22 @@ def test_collection_too_long(self, sql_backend):
             sql_backend.reserve_next_ids("x" * (COLLECTION_MAX_LEN + 1), 1)
 
     def test_initial_collection_query(self, sql_backend, connection):
-        connection.query_single_value = MagicMock(return_value=None)
-        connection.execute = ex = MagicMock()
+        connection.query_single_value = qsv = MagicMock(return_value=4)
 
         result = sql_backend.reserve_next_ids("my_collection", 3)
 
         assert result == [1, 2, 3]
-        ex.assert_called_once()
-        args = ex.call_args.args[1]
+        args = qsv.call_args.args[1]
         assert args == ["my_collection", 4]
 
     def test_collection_query(self, sql_backend, connection):
-        connection.query_single_value = MagicMock(return_value=4)
-        connection.execute = ex = MagicMock()
+        connection.query_single_value = qsv = MagicMock(return_value=7)
 
         result = sql_backend.reserve_next_ids("my_collection", 3)
 
         assert result == [4, 5, 6]
-        ex.assert_called_once()
-        args = ex.call_args.args[1]
-        assert args == ["my_collection", 7]
+        args = qsv.call_args.args[1]
+        assert args == ["my_collection", 4]
 
 
 def test_truncate_db(sql_backend, connection):

writer/writer/core/event_executor.py

@@ -117,7 +117,6 @@ def add_position(self):
             model[META_POSITION] = self.position
 
     def write_back(self):
-        print(self.models)
         write_models = {
             fqid: self.models[fqid]
             for fqid, status in self.model_status.items()

writer/writer/postgresql_backend/sql_database_backend_service.py

@@ -12,8 +12,10 @@
     ModelDoesNotExist,
     ModelExists,
     ModelNotDeleted,
+    collection_from_fqid,
     collectionfield_from_fqid_and_field,
     field_from_collectionfield,
+    id_from_fqid,
 )
 from writer.core import BaseDbEvent
 from writer.core.db_events import (
@@ -86,6 +88,19 @@ def insert_create_event(self, create_event, position: int) -> None:
         if self.exists_query("models_lookup", "fqid=%s", [create_event.fqid]):
             raise ModelExists(create_event.fqid)
 
+        # update max collection id if neccessary
+        statement = dedent(
+            """\
+            insert into id_sequences (collection, id) values (%s, %s)
+            on conflict(collection) do update
+            set id=greatest(id_sequences.id, excluded.id);"""
+        )
+        arguments: List[Any] = [
+            collection_from_fqid(create_event.fqid),
+            int(id_from_fqid(create_event.fqid)) + 1,
+        ]
+        self.connection.execute(statement, arguments)
+
         arguments = [
             position,
             create_event.fqid,
@@ -250,23 +265,16 @@ def reserve_next_ids(self, collection: str, amount: int) -> List[int]:
                 f"collection length must be between 1 and {COLLECTION_MAX_LEN}"
             )
 
-        query = "select id from id_sequences where collection=%s"
-        arguments = [collection]
-        low_id = self.connection.query_single_value(query, arguments)
-        if low_id is None:
-            low_id = 1
-        high_id = low_id + amount  # high id not included in the result
-
-        statement = "update id_sequences set id=%s where collection=%s"
         statement = dedent(
             """\
             insert into id_sequences (collection, id) values (%s, %s)
-            on conflict(collection) do update set id=excluded.id;"""
+            on conflict(collection) do update
+            set id=id_sequences.id + excluded.id - 1 returning id;"""
         )
-        arguments = [collection, high_id]
-        self.connection.execute(statement, arguments)
+        arguments = [collection, amount + 1]
+        new_max_id = self.connection.query_single_value(statement, arguments)
 
-        return list(range(low_id, high_id))
+        return list(range(new_max_id - amount, new_max_id))
 
     def truncate_db(self) -> None:
         for table in ALL_TABLES: