Use collection_id path parameter Items Transactions endpoints (#425)

* Add collection_id path parameter and check against Item collection property

* Fix unformatted f-strings

* Fix Item PUT endpoint per #385

* Update API tests to use new PUT paths

* Make equivalent changes to sqlalchemy backend

* Add CHANGELOG entry for #425

* Fix failing tests from previous merge

* Return 400 for Item id or collection conflicts

CHANGES.md

@@ -22,6 +22,9 @@
 * docker-compose now runs uvicorn with hot-reloading enabled
 * Bump version of PGStac to 0.6.2 that includes support for hydrating results in the API backed ([#397](https://github.com/stac-utils/stac-fastapi/pull/397))
 * Make item geometry and bbox nullable in sqlalchemy backend. ([#398](https://github.com/stac-utils/stac-fastapi/pull/398))
+* Transactions Extension update Item endpoint Item is now `/collections/{collection_id}/items/{item_id}` instead of 
+  `/collections/{collection_id}/items` to align with [STAC API
+  spec](https://github.com/radiantearth/stac-api-spec/tree/main/ogcapi-features/extensions/transaction#methods) ([#425](https://github.com/stac-utils/stac-fastapi/pull/425))
 
 ### Removed
 * Remove the unused `router_middleware` function ([#439](https://github.com/stac-utils/stac-fastapi/pull/439))
@@ -36,6 +39,9 @@
 * SQLAlchemy backend bulk item insert now works ([#356](https://github.com/stac-utils/stac-fastapi/issues/356))
 * PGStac Backend has stricter implementation of Fields Extension syntax ([#397](https://github.com/stac-utils/stac-fastapi/pull/397))
 * `/queryables` endpoint now has type `application/schema+json` instead of `application/json` ([#421](https://github.com/stac-utils/stac-fastapi/pull/421))
+* Transactions Extension update Item endpoint validates that the `{collection_id}` path parameter matches the Item `"collection"` property 
+  from the request body, if present, and falls back to using the path parameter if no `"collection"` property is found in the body
+  ([#425](https://github.com/stac-utils/stac-fastapi/pull/425))
 
 ## [2.3.0]
 

scripts/ingest_joplin.py

@@ -19,8 +19,9 @@ def post_or_put(url: str, data: dict):
     """Post or put data to url."""
     r = requests.post(url, json=data)
     if r.status_code == 409:
+        new_url = url if data["type"] == "Collection" else url + f"/{data['id']}"
         # Exists, so update
-        r = requests.put(url, json=data)
+        r = requests.put(new_url, json=data)
         # Unchanged may throw a 404
         if not r.status_code == 404:
             r.raise_for_status()

stac_fastapi/api/tests/test_api.py

@@ -55,7 +55,7 @@ def test_build_api_with_route_dependencies(self):
             {"path": "/collections", "method": "PUT"},
             {"path": "/collections/{collectionId}", "method": "DELETE"},
             {"path": "/collections/{collectionId}/items", "method": "POST"},
-            {"path": "/collections/{collectionId}/items", "method": "PUT"},
+            {"path": "/collections/{collectionId}/items/{itemId}", "method": "PUT"},
             {"path": "/collections/{collectionId}/items/{itemId}", "method": "DELETE"},
         ]
         dependencies = [Depends(must_be_bob)]
@@ -68,7 +68,7 @@ def test_add_route_dependencies_after_building_api(self):
             {"path": "/collections", "method": "PUT"},
             {"path": "/collections/{collectionId}", "method": "DELETE"},
             {"path": "/collections/{collectionId}/items", "method": "POST"},
-            {"path": "/collections/{collectionId}/items", "method": "PUT"},
+            {"path": "/collections/{collectionId}/items/{itemId}", "method": "PUT"},
             {"path": "/collections/{collectionId}/items/{itemId}", "method": "DELETE"},
         ]
         api = self._build_api()

stac_fastapi/extensions/stac_fastapi/extensions/core/transaction.py

@@ -2,7 +2,7 @@
 from typing import Callable, List, Optional, Type, Union
 
 import attr
-from fastapi import APIRouter, FastAPI
+from fastapi import APIRouter, Body, FastAPI
 from pydantic import BaseModel
 from stac_pydantic import Collection, Item
 from starlette.responses import JSONResponse, Response
@@ -15,6 +15,20 @@
 from stac_fastapi.types.extension import ApiExtension
 
 
+@attr.s
+class PostItem(CollectionUri):
+    """Create Item."""
+
+    item: stac_types.Item = attr.ib(default=Body())
+
+
+@attr.s
+class PutItem(ItemUri):
+    """Update Item."""
+
+    item: stac_types.Item = attr.ib(default=Body())
+
+
 @attr.s
 class TransactionExtension(ApiExtension):
     """Transaction Extension.
@@ -77,20 +91,20 @@ def register_create_item(self):
             response_model_exclude_unset=True,
             response_model_exclude_none=True,
             methods=["POST"],
-            endpoint=self._create_endpoint(self.client.create_item, stac_types.Item),
+            endpoint=self._create_endpoint(self.client.create_item, PostItem),
         )
 
     def register_update_item(self):
         """Register update item endpoint (PUT /collections/{collection_id}/items)."""
         self.router.add_api_route(
             name="Update Item",
-            path="/collections/{collection_id}/items",
+            path="/collections/{collection_id}/items/{item_id}",
             response_model=Item if self.settings.enable_response_models else None,
             response_class=self.response_class,
             response_model_exclude_unset=True,
             response_model_exclude_none=True,
             methods=["PUT"],
-            endpoint=self._create_endpoint(self.client.update_item, stac_types.Item),
+            endpoint=self._create_endpoint(self.client.update_item, PutItem),
         )
 
     def register_delete_item(self):

stac_fastapi/pgstac/stac_fastapi/pgstac/transactions.py

@@ -4,6 +4,7 @@
 from typing import Optional, Union
 
 import attr
+from fastapi import HTTPException
 from starlette.responses import JSONResponse, Response
 
 from stac_fastapi.extensions.third_party.bulk_transactions import (
@@ -23,18 +24,38 @@ class TransactionsClient(AsyncBaseTransactionsClient):
     """Transactions extension specific CRUD operations."""
 
     async def create_item(
-        self, item: stac_types.Item, **kwargs
+        self, collection_id: str, item: stac_types.Item, **kwargs
     ) -> Optional[Union[stac_types.Item, Response]]:
         """Create item."""
+        body_collection_id = item.get("collection")
+        if body_collection_id is not None and collection_id != body_collection_id:
+            raise HTTPException(
+                status_code=400,
+                detail=f"Collection ID from path parameter ({collection_id}) does not match Collection ID from Item ({body_collection_id})",
+            )
+        item["collection"] = collection_id
         request = kwargs["request"]
         pool = request.app.state.writepool
         await dbfunc(pool, "create_item", item)
         return item
 
     async def update_item(
-        self, item: stac_types.Item, **kwargs
+        self, collection_id: str, item_id: str, item: stac_types.Item, **kwargs
     ) -> Optional[Union[stac_types.Item, Response]]:
         """Update item."""
+        body_collection_id = item.get("collection")
+        if body_collection_id is not None and collection_id != body_collection_id:
+            raise HTTPException(
+                status_code=400,
+                detail=f"Collection ID from path parameter ({collection_id}) does not match Collection ID from Item ({body_collection_id})",
+            )
+        item["collection"] = collection_id
+        body_item_id = item["id"]
+        if body_item_id != item_id:
+            raise HTTPException(
+                status_code=400,
+                detail=f"Item ID from path parameter ({item_id}) does not match Item ID from Item ({body_item_id})",
+            )
         request = kwargs["request"]
         pool = request.app.state.writepool
         await dbfunc(pool, "update_item", item)

stac_fastapi/pgstac/tests/api/test_api.py

@@ -19,7 +19,7 @@
     "POST /collections",
     "POST /collections/{collection_id}/items",
     "PUT /collections",
-    "PUT /collections/{collection_id}/items",
+    "PUT /collections/{collection_id}/items/{item_id}",
 ]
 
 

stac_fastapi/pgstac/tests/clients/test_postgres.py

@@ -76,7 +76,9 @@ async def test_update_item(app_client, load_test_collection, load_test_item):
 
     item.properties.description = "Update Test"
 
-    resp = await app_client.put(f"/collections/{coll.id}/items", content=item.json())
+    resp = await app_client.put(
+        f"/collections/{coll.id}/items/{item.id}", content=item.json()
+    )
     assert resp.status_code == 200
 
     resp = await app_client.get(f"/collections/{coll.id}/items/{item.id}")

stac_fastapi/pgstac/tests/conftest.py

@@ -201,9 +201,10 @@ async def load_test_collection(app_client, load_test_data):
 
 @pytest.fixture
 async def load_test_item(app_client, load_test_data, load_test_collection):
+    coll = load_test_collection
     data = load_test_data("test_item.json")
     resp = await app_client.post(
-        "/collections/{coll.id}/items",
+        f"/collections/{coll.id}/items",
         json=data,
     )
     assert resp.status_code == 200
@@ -223,9 +224,10 @@ async def load_test2_collection(app_client, load_test_data):
 
 @pytest.fixture
 async def load_test2_item(app_client, load_test_data, load_test2_collection):
+    coll = load_test2_collection
     data = load_test_data("test2_item.json")
     resp = await app_client.post(
-        "/collections/{coll.id}/items",
+        f"/collections/{coll.id}/items",
         json=data,
     )
     assert resp.status_code == 200

stac_fastapi/pgstac/tests/resources/test_item.py

@@ -1,7 +1,9 @@
 import json
+import random
 import uuid
 from datetime import timedelta
 from http.client import HTTP_PORT
+from string import ascii_letters
 from typing import Callable
 from urllib.parse import parse_qs, urljoin, urlparse
 
@@ -81,6 +83,24 @@ async def test_create_item(app_client, load_test_data: Callable, load_test_colle
     assert in_item.dict(exclude={"links"}) == get_item.dict(exclude={"links"})
 
 
+async def test_create_item_mismatched_collection_id(
+    app_client, load_test_data: Callable, load_test_collection
+):
+    # If the collection_id path parameter and the Item's "collection" property do not match, a 400 response should
+    # be returned.
+    coll = load_test_collection
+
+    in_json = load_test_data("test_item.json")
+    in_json["collection"] = random.choice(ascii_letters)
+    assert in_json["collection"] != coll.id
+
+    resp = await app_client.post(
+        f"/collections/{coll.id}/items",
+        json=in_json,
+    )
+    assert resp.status_code == 400
+
+
 async def test_fetches_valid_item(
     app_client, load_test_data: Callable, load_test_collection
 ):
@@ -89,7 +109,7 @@ async def test_fetches_valid_item(
     in_json = load_test_data("test_item.json")
     in_item = Item.parse_obj(in_json)
     resp = await app_client.post(
-        "/collections/{coll.id}/items",
+        f"/collections/{coll.id}/items",
         json=in_json,
     )
     assert resp.status_code == 200
@@ -117,7 +137,9 @@ async def test_update_item(
 
     item.properties.description = "Update Test"
 
-    resp = await app_client.put(f"/collections/{coll.id}/items", content=item.json())
+    resp = await app_client.put(
+        f"/collections/{coll.id}/items/{item.id}", content=item.json()
+    )
     assert resp.status_code == 200
 
     resp = await app_client.get(f"/collections/{coll.id}/items/{item.id}")
@@ -128,6 +150,25 @@ async def test_update_item(
     assert get_item.properties.description == "Update Test"
 
 
+async def test_update_item_mismatched_collection_id(
+    app_client, load_test_data: Callable, load_test_collection, load_test_item
+) -> None:
+    coll = load_test_collection
+
+    in_json = load_test_data("test_item.json")
+
+    in_json["collection"] = random.choice(ascii_letters)
+    assert in_json["collection"] != coll.id
+
+    item_id = in_json["id"]
+
+    resp = await app_client.put(
+        f"/collections/{coll.id}/items/{item_id}",
+        json=in_json,
+    )
+    assert resp.status_code == 400
+
+
 async def test_delete_item(
     app_client, load_test_data: Callable, load_test_collection, load_test_item
 ):
@@ -165,18 +206,17 @@ async def test_get_collection_items(app_client, load_test_collection, load_test_
 async def test_create_item_conflict(
     app_client, load_test_data: Callable, load_test_collection
 ):
-    pass
-
+    coll = load_test_collection
     in_json = load_test_data("test_item.json")
     Item.parse_obj(in_json)
     resp = await app_client.post(
-        "/collections/{coll.id}/items",
+        f"/collections/{coll.id}/items",
         json=in_json,
     )
     assert resp.status_code == 200
 
     resp = await app_client.post(
-        "/collections/{coll.id}/items",
+        f"/collections/{coll.id}/items",
         json=in_json,
     )
     assert resp.status_code == 409
@@ -203,7 +243,10 @@ async def test_create_item_missing_collection(
     item["collection"] = None
 
     resp = await app_client.post(f"/collections/{coll.id}/items", json=item)
-    assert resp.status_code == 424
+    assert resp.status_code == 200
+
+    post_item = resp.json()
+    assert post_item["collection"] == coll.id
 
 
 async def test_update_new_item(
@@ -213,7 +256,9 @@ async def test_update_new_item(
     item = load_test_item
     item.id = "test-updatenewitem"
 
-    resp = await app_client.put(f"/collections/{coll.id}/items", content=item.json())
+    resp = await app_client.put(
+        f"/collections/{coll.id}/items/{item.id}", content=item.json()
+    )
     assert resp.status_code == 404
 
 
@@ -224,8 +269,13 @@ async def test_update_item_missing_collection(
     item = load_test_item
     item.collection = None
 
-    resp = await app_client.put(f"/collections/{coll.id}/items", content=item.json())
-    assert resp.status_code == 424
+    resp = await app_client.put(
+        f"/collections/{coll.id}/items/{item.id}", content=item.json()
+    )
+    assert resp.status_code == 200
+
+    put_item = resp.json()
+    assert put_item["collection"] == coll.id
 
 
 async def test_pagination(app_client, load_test_data, load_test_collection):