Update item transactions in SQLAlchemy (#303)

* query by item and collection id

* add get item test

* add parameters

* run pre-commit

* Alter items table to have primary key on both id and collection_id

* update update_item route

* add delete_item changes

* run pre-commit

* run pre-commit

* clean up

* fix query in update/delete items

* run pre commit

* create duplicate item ids, different collections test

* create duplicate item ids, delete item test

* create duplicate item ids, update item test

* clarify tests

* create duplicate item, different collections test in postgres

* remove app client 2 in conftest

Co-authored-by: Robin Wilson <[email protected]>
Co-authored-by: Jeff Albrecht <[email protected]>

Author: 3 people

stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/transactions.py

@@ -58,8 +58,11 @@ def update_item(self, model: stac_types.Item, **kwargs) -> stac_types.Item:
             query = session.query(self.item_table).filter(
                 self.item_table.id == model["id"]
             )
+            query = query.filter(self.item_table.collection_id == model["collection"])
             if not query.scalar():
-                raise NotFoundError(f"Item {model['id']} not found")
+                raise NotFoundError(
+                    f"Item {model['id']} in collection {model['collection']}"
+                )
             # SQLAlchemy orm updates don't seem to like geoalchemy types
             db_model = self.item_serializer.stac_to_db(model)
             query.update(self.item_serializer.row_to_dict(db_model))
@@ -91,10 +94,15 @@ def delete_item(
         """Delete item."""
         base_url = str(kwargs["request"].base_url)
         with self.session.writer.context_session() as session:
-            query = session.query(self.item_table).filter(self.item_table.id == item_id)
+            query = session.query(self.item_table).filter(
+                self.item_table.collection_id == collection_id
+            )
+            query = query.filter(self.item_table.id == item_id)
             data = query.first()
             if not data:
-                raise NotFoundError(f"Item {item_id} not found")
+                raise NotFoundError(
+                    f"Item {item_id} not found in collection {collection_id}"
+                )
             query.delete()
             return self.item_serializer.db_to_stac(data, base_url=base_url)
 

stac_fastapi/sqlalchemy/tests/clients/test_postgres.py

@@ -157,6 +157,44 @@ def test_create_item_already_exists(
         postgres_transactions.create_item(item, request=MockStarletteRequest)
 
 
+def test_create_duplicate_item_different_collections(
+    postgres_core: CoreCrudClient,
+    postgres_transactions: TransactionsClient,
+    load_test_data: Callable,
+):
+    # create test-collection
+    coll = load_test_data("test_collection.json")
+    postgres_transactions.create_collection(coll, request=MockStarletteRequest)
+
+    # create test-collection-2
+    coll["id"] = "test-collection-2"
+    postgres_transactions.create_collection(coll, request=MockStarletteRequest)
+
+    # add item to test-collection
+    item = load_test_data("test_item.json")
+    postgres_transactions.create_item(item, request=MockStarletteRequest)
+
+    # get item from test-collection
+    resp = postgres_core.get_item(
+        item["id"], item["collection"], request=MockStarletteRequest
+    )
+    assert Item(**item).dict(
+        exclude={"links": ..., "properties": {"created", "updated"}}
+    ) == Item(**resp).dict(exclude={"links": ..., "properties": {"created", "updated"}})
+
+    # add item to test-collection-2
+    item["collection"] = "test-collection-2"
+    postgres_transactions.create_item(item, request=MockStarletteRequest)
+
+    # get item with same id from test-collection-2
+    resp = postgres_core.get_item(
+        item["id"], item["collection"], request=MockStarletteRequest
+    )
+    assert Item(**item).dict(
+        exclude={"links": ..., "properties": {"created", "updated"}}
+    ) == Item(**resp).dict(exclude={"links": ..., "properties": {"created", "updated"}})
+
+
 def test_update_item(
     postgres_core: CoreCrudClient,
     postgres_transactions: TransactionsClient,

stac_fastapi/sqlalchemy/tests/resources/test_item.py

@@ -44,6 +44,141 @@ def test_create_item_conflict(app_client, load_test_data):
     assert resp.status_code == 409
 
 
+def test_create_item_duplicate(app_client, load_test_data):
+    """Test creation of an item id which already exists but in a different collection(transactions extension)"""
+
+    # add test_item to test-collection
+    test_item = load_test_data("test_item.json")
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+    # add test_item to test-collection again, resource already exists
+    test_item = load_test_data("test_item.json")
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 409
+
+    # create "test-collection-2"
+    collection_2 = load_test_data("test_collection.json")
+    collection_2["id"] = "test-collection-2"
+    resp = app_client.post("/collections", json=collection_2)
+    assert resp.status_code == 200
+
+    # add test_item to test-collection-2, posts successfully
+    test_item["collection"] = "test-collection-2"
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+
+def test_delete_item_duplicate(app_client, load_test_data):
+    """Test creation of an item id which already exists but in a different collection(transactions extension)"""
+
+    # add test_item to test-collection
+    test_item = load_test_data("test_item.json")
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+    # create "test-collection-2"
+    collection_2 = load_test_data("test_collection.json")
+    collection_2["id"] = "test-collection-2"
+    resp = app_client.post("/collections", json=collection_2)
+    assert resp.status_code == 200
+
+    # add test_item to test-collection-2
+    test_item["collection"] = "test-collection-2"
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+    # delete test_item from test-collection
+    test_item["collection"] = "test-collection"
+    resp = app_client.delete(
+        f"/collections/{test_item['collection']}/items/{test_item['id']}"
+    )
+    assert resp.status_code == 200
+
+    # test-item in test-collection has already been deleted
+    resp = app_client.delete(
+        f"/collections/{test_item['collection']}/items/{test_item['id']}"
+    )
+    assert resp.status_code == 404
+
+    # test-item in test-collection-2 still exists, was not deleted
+    test_item["collection"] = "test-collection-2"
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 409
+
+
+def test_update_item_duplicate(app_client, load_test_data):
+    """Test creation of an item id which already exists but in a different collection(transactions extension)"""
+
+    # add test_item to test-collection
+    test_item = load_test_data("test_item.json")
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+    # create "test-collection-2"
+    collection_2 = load_test_data("test_collection.json")
+    collection_2["id"] = "test-collection-2"
+    resp = app_client.post("/collections", json=collection_2)
+    assert resp.status_code == 200
+
+    # add test_item to test-collection-2
+    test_item["collection"] = "test-collection-2"
+    resp = app_client.post(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+
+    # update gsd in test_item, test-collection-2
+    test_item["properties"]["gsd"] = 16
+    resp = app_client.put(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+    updated_item = resp.json()
+    assert updated_item["properties"]["gsd"] == 16
+
+    # update gsd in test_item, test-collection
+    test_item["collection"] = "test-collection"
+    test_item["properties"]["gsd"] = 17
+    resp = app_client.put(
+        f"/collections/{test_item['collection']}/items", json=test_item
+    )
+    assert resp.status_code == 200
+    updated_item = resp.json()
+    assert updated_item["properties"]["gsd"] == 17
+
+    # test_item in test-collection, updated gsd = 17
+    resp = app_client.get(
+        f"/collections/{test_item['collection']}/items/{test_item['id']}"
+    )
+    assert resp.status_code == 200
+    item = resp.json()
+    assert item["properties"]["gsd"] == 17
+
+    # test_item in test-collection-2, updated gsd = 16
+    test_item["collection"] = "test-collection-2"
+    resp = app_client.get(
+        f"/collections/{test_item['collection']}/items/{test_item['id']}"
+    )
+    assert resp.status_code == 200
+    item = resp.json()
+    assert item["properties"]["gsd"] == 16
+
+
 def test_delete_missing_item(app_client, load_test_data):
     """Test deletion of an item which does not exist (transactions extension)"""
     test_item = load_test_data("test_item.json")
@@ -102,7 +237,7 @@ def test_update_item_missing_collection(app_client, load_test_data):
     resp = app_client.put(
         f"/collections/{test_item['collection']}/items", json=test_item
     )
-    assert resp.status_code == 422
+    assert resp.status_code == 404
 
 
 def test_update_item_geometry(app_client, load_test_data):