change transactions clients method signatures to match use, allow result of None to return 204 (#387)

Co-authored-by: Nathan Zimmerman <[email protected]>

Author: philvarner

stac_fastapi/api/stac_fastapi/api/routes.py

@@ -7,15 +7,18 @@
 from starlette.requests import Request
 from starlette.responses import JSONResponse, Response
 from starlette.routing import BaseRoute, Match
+from starlette.status import HTTP_204_NO_CONTENT
 
 from stac_fastapi.api.models import APIRequest
 
 
 def _wrap_response(resp: Any, response_class: Type[Response]) -> Response:
     if isinstance(resp, Response):
         return resp
-    else:
+    elif resp is not None:
         return response_class(resp)
+    else:  # None is returned as 204 No Content
+        return Response(status_code=HTTP_204_NO_CONTENT)
 
 
 def create_async_endpoint(

stac_fastapi/pgstac/stac_fastapi/pgstac/core.py

@@ -77,7 +77,7 @@ async def get_collection(self, collection_id: str, **kwargs) -> Collection:
         Called with `GET /collections/{collection_id}`.
 
         Args:
-            id: Id of the collection.
+            collection_id: ID of the collection.
 
         Returns:
             Collection.
@@ -212,7 +212,8 @@ async def get_item(self, item_id: str, collection_id: str, **kwargs) -> Item:
         Called with `GET /collections/{collection_id}/items/{item_id}`.
 
         Args:
-            id: Id of the item.
+            item_id: ID of the item.
+            collection_id: ID of the collection the item is in.
 
         Returns:
             Item.

stac_fastapi/pgstac/stac_fastapi/pgstac/transactions.py

@@ -1,9 +1,10 @@
 """transactions extension client."""
 
 import logging
-from typing import Dict
+from typing import Optional, Union
 
 import attr
+from starlette.responses import JSONResponse, Response
 
 from stac_fastapi.pgstac.db import dbfunc
 from stac_fastapi.types import stac as stac_types
@@ -17,14 +18,18 @@
 class TransactionsClient(AsyncBaseTransactionsClient):
     """Transactions extension specific CRUD operations."""
 
-    async def create_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
+    async def create_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Create item."""
         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) -> stac_types.Item:
+    async def update_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Update item."""
         request = kwargs["request"]
         pool = request.app.state.writepool
@@ -33,7 +38,7 @@ async def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
 
     async def create_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Create collection."""
         request = kwargs["request"]
         pool = request.app.state.writepool
@@ -42,23 +47,27 @@ async def create_collection(
 
     async def update_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Update collection."""
         request = kwargs["request"]
         pool = request.app.state.writepool
         await dbfunc(pool, "update_collection", collection)
         return collection
 
-    async def delete_item(self, item_id: str, collection_id: str, **kwargs) -> Dict:
-        """Delete collection."""
+    async def delete_item(
+        self, item_id: str, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
+        """Delete item."""
         request = kwargs["request"]
         pool = request.app.state.writepool
         await dbfunc(pool, "delete_item", item_id)
-        return {"deleted item": item_id}
+        return JSONResponse({"deleted item": item_id})
 
-    async def delete_collection(self, collection_id: str, **kwargs) -> Dict:
+    async def delete_collection(
+        self, collection_id: str, **kwargs
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Delete collection."""
         request = kwargs["request"]
         pool = request.app.state.writepool
         await dbfunc(pool, "delete_collection", collection_id)
-        return {"deleted collection": collection_id}
+        return JSONResponse({"deleted collection": collection_id})

stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/transactions.py

@@ -1,9 +1,10 @@
 """transactions extension client."""
 
 import logging
-from typing import Optional, Type
+from typing import Optional, Type, Union
 
 import attr
+from starlette.responses import Response
 
 from stac_fastapi.extensions.third_party.bulk_transactions import (
     BaseBulkTransactionsClient,
@@ -33,64 +34,68 @@ class TransactionsClient(BaseTransactionsClient):
         default=serializers.CollectionSerializer
     )
 
-    def create_item(self, model: stac_types.Item, **kwargs) -> stac_types.Item:
+    def create_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Create item."""
         base_url = str(kwargs["request"].base_url)
-        data = self.item_serializer.stac_to_db(model)
+        data = self.item_serializer.stac_to_db(item)
         with self.session.writer.context_session() as session:
             session.add(data)
             return self.item_serializer.db_to_stac(data, base_url)
 
     def create_collection(
-        self, model: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+        self, collection: stac_types.Collection, **kwargs
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Create collection."""
         base_url = str(kwargs["request"].base_url)
-        data = self.collection_serializer.stac_to_db(model)
+        data = self.collection_serializer.stac_to_db(collection)
         with self.session.writer.context_session() as session:
             session.add(data)
             return self.collection_serializer.db_to_stac(data, base_url=base_url)
 
-    def update_item(self, model: stac_types.Item, **kwargs) -> stac_types.Item:
+    def update_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Update item."""
         base_url = str(kwargs["request"].base_url)
         with self.session.reader.context_session() as session:
             query = session.query(self.item_table).filter(
-                self.item_table.id == model["id"]
+                self.item_table.id == item["id"]
             )
-            query = query.filter(self.item_table.collection_id == model["collection"])
+            query = query.filter(self.item_table.collection_id == item["collection"])
             if not query.scalar():
                 raise NotFoundError(
-                    f"Item {model['id']} in collection {model['collection']}"
+                    f"Item {item['id']} in collection {item['collection']}"
                 )
             # SQLAlchemy orm updates don't seem to like geoalchemy types
-            db_model = self.item_serializer.stac_to_db(model)
+            db_model = self.item_serializer.stac_to_db(item)
             query.update(self.item_serializer.row_to_dict(db_model))
             stac_item = self.item_serializer.db_to_stac(db_model, base_url)
 
             return stac_item
 
     def update_collection(
-        self, model: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+        self, collection: stac_types.Collection, **kwargs
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Update collection."""
         base_url = str(kwargs["request"].base_url)
         with self.session.reader.context_session() as session:
             query = session.query(self.collection_table).filter(
-                self.collection_table.id == model["id"]
+                self.collection_table.id == collection["id"]
             )
             if not query.scalar():
-                raise NotFoundError(f"Item {model['id']} not found")
+                raise NotFoundError(f"Item {collection['id']} not found")
 
             # SQLAlchemy orm updates don't seem to like geoalchemy types
-            db_model = self.collection_serializer.stac_to_db(model)
+            db_model = self.collection_serializer.stac_to_db(collection)
             query.update(self.collection_serializer.row_to_dict(db_model))
 
             return self.collection_serializer.db_to_stac(db_model, base_url)
 
     def delete_item(
         self, item_id: str, collection_id: str, **kwargs
-    ) -> stac_types.Item:
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Delete item."""
         base_url = str(kwargs["request"].base_url)
         with self.session.writer.context_session() as session:
@@ -106,7 +111,9 @@ def delete_item(
             query.delete()
             return self.item_serializer.db_to_stac(data, base_url=base_url)
 
-    def delete_collection(self, collection_id: str, **kwargs) -> stac_types.Collection:
+    def delete_collection(
+        self, collection_id: str, **kwargs
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Delete collection."""
         base_url = str(kwargs["request"].base_url)
         with self.session.writer.context_session() as session:

stac_fastapi/types/stac_fastapi/types/core.py

@@ -6,10 +6,10 @@
 
 import attr
 from fastapi import Request
-from stac_pydantic.api import Search
 from stac_pydantic.links import Relations
 from stac_pydantic.shared import MimeTypes
 from stac_pydantic.version import STAC_VERSION
+from starlette.responses import Response
 
 from stac_fastapi.types import stac as stac_types
 from stac_fastapi.types.conformance import BASE_CONFORMANCE_CLASSES
@@ -23,16 +23,19 @@
 
 @attr.s  # type:ignore
 class BaseTransactionsClient(abc.ABC):
-    """Defines a pattern for implementing the STAC transaction extension."""
+    """Defines a pattern for implementing the STAC API Transaction Extension."""
 
     @abc.abstractmethod
-    def create_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
+    def create_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Create a new item.
 
         Called with `POST /collections/{collection_id}/items`.
 
         Args:
             item: the item
+            collection_id: the id of the collection from the resource path
 
         Returns:
             The item that was created.
@@ -41,7 +44,9 @@ def create_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
         ...
 
     @abc.abstractmethod
-    def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
+    def update_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Perform a complete update on an existing item.
 
         Called with `PUT /collections/{collection_id}/items`. It is expected that this item already exists.  The update
@@ -50,6 +55,7 @@ def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
 
         Args:
             item: the item (must be complete)
+            collection_id: the id of the collection from the resource path
 
         Returns:
             The updated item.
@@ -59,7 +65,7 @@ def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
     @abc.abstractmethod
     def delete_item(
         self, item_id: str, collection_id: str, **kwargs
-    ) -> stac_types.Item:
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Delete an item from a collection.
 
         Called with `DELETE /collections/{collection_id}/items/{item_id}`
@@ -76,7 +82,7 @@ def delete_item(
     @abc.abstractmethod
     def create_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Create a new collection.
 
         Called with `POST /collections`.
@@ -92,7 +98,7 @@ def create_collection(
     @abc.abstractmethod
     def update_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Perform a complete update on an existing collection.
 
         Called with `PUT /collections`. It is expected that this item already exists.  The update should do a diff
@@ -101,14 +107,17 @@ def update_collection(
 
         Args:
             collection: the collection (must be complete)
+            collection_id: the id of the collection from the resource path
 
         Returns:
             The updated collection.
         """
         ...
 
     @abc.abstractmethod
-    def delete_collection(self, collection_id: str, **kwargs) -> stac_types.Collection:
+    def delete_collection(
+        self, collection_id: str, **kwargs
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Delete a collection.
 
         Called with `DELETE /collections/{collection_id}`
@@ -127,7 +136,9 @@ class AsyncBaseTransactionsClient(abc.ABC):
     """Defines a pattern for implementing the STAC transaction extension."""
 
     @abc.abstractmethod
-    async def create_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
+    async def create_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Create a new item.
 
         Called with `POST /collections/{collection_id}/items`.
@@ -142,7 +153,9 @@ async def create_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
         ...
 
     @abc.abstractmethod
-    async def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
+    async def update_item(
+        self, item: stac_types.Item, **kwargs
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Perform a complete update on an existing item.
 
         Called with `PUT /collections/{collection_id}/items`. It is expected that this item already exists.  The update
@@ -160,7 +173,7 @@ async def update_item(self, item: stac_types.Item, **kwargs) -> stac_types.Item:
     @abc.abstractmethod
     async def delete_item(
         self, item_id: str, collection_id: str, **kwargs
-    ) -> stac_types.Item:
+    ) -> Optional[Union[stac_types.Item, Response]]:
         """Delete an item from a collection.
 
         Called with `DELETE /collections/{collection_id}/items/{item_id}`
@@ -177,7 +190,7 @@ async def delete_item(
     @abc.abstractmethod
     async def create_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Create a new collection.
 
         Called with `POST /collections`.
@@ -193,7 +206,7 @@ async def create_collection(
     @abc.abstractmethod
     async def update_collection(
         self, collection: stac_types.Collection, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Perform a complete update on an existing collection.
 
         Called with `PUT /collections`. It is expected that this item already exists.  The update should do a diff
@@ -211,7 +224,7 @@ async def update_collection(
     @abc.abstractmethod
     async def delete_collection(
         self, collection_id: str, **kwargs
-    ) -> stac_types.Collection:
+    ) -> Optional[Union[stac_types.Collection, Response]]:
         """Delete a collection.
 
         Called with `DELETE /collections/{collection_id}`
@@ -394,7 +407,7 @@ def conformance(self, **kwargs) -> stac_types.Conformance:
 
     @abc.abstractmethod
     def post_search(
-        self, search_request: Search, **kwargs
+        self, search_request: BaseSearchPostRequest, **kwargs
     ) -> stac_types.ItemCollection:
         """Cross catalog search (POST).
 
@@ -581,7 +594,7 @@ async def conformance(self, **kwargs) -> stac_types.Conformance:
 
     @abc.abstractmethod
     async def post_search(
-        self, search_request: Search, **kwargs
+        self, search_request: BaseSearchPostRequest, **kwargs
     ) -> stac_types.ItemCollection:
         """Cross catalog search (POST).