From cb08b0f6f1b11643632071113e10921501da3808 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Fri, 2 Aug 2024 15:48:12 +0200 Subject: [PATCH] Allow to omit collection in bulk insertions (#113) * In bulk insertions allow to omit collection in items Same checks in item bulk insertions as in single insertions Added tests * Fixing whitespace * Update changelog * Linting fix * fix tests --------- Co-authored-by: vincentsarago --- CHANGES.md | 1 + stac_fastapi/pgstac/transactions.py | 18 ++++-- tests/clients/test_postgres.py | 90 +++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8356053..1995e07 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,7 @@ ## [Unreleased] - Enable filter extension for `GET /items` requests and add `Queryables` links in `/collections` and `/collections/{collection_id}` responses ([#89](https://github.com/stac-utils/stac-fastapi-pgstac/pull/89)) +- Allow to omit `collection` in bulk item insertions. Same identifier checks as with single insertions ([#113](https://github.com/stac-utils/stac-fastapi-pgstac/pull/113)) ## [3.0.0a4] - 2024-07-10 diff --git a/stac_fastapi/pgstac/transactions.py b/stac_fastapi/pgstac/transactions.py index ed624f9..b4a68c4 100644 --- a/stac_fastapi/pgstac/transactions.py +++ b/stac_fastapi/pgstac/transactions.py @@ -25,10 +25,7 @@ logger.setLevel(logging.INFO) -@attr.s -class TransactionsClient(AsyncBaseTransactionsClient): - """Transactions extension specific CRUD operations.""" - +class ClientValidateMixIn: def _validate_id(self, id: str, settings: Settings): invalid_chars = settings.invalid_id_chars id_regex = "[" + "".join(re.escape(char) for char in invalid_chars) + "]" @@ -67,6 +64,11 @@ def _validate_item( detail=f"Item ID from path parameter ({expected_item_id}) does not match Item ID from Item ({body_item_id})", ) + +@attr.s +class TransactionsClient(AsyncBaseTransactionsClient, ClientValidateMixIn): + """Transactions extension specific CRUD operations.""" + async def create_item( self, collection_id: str, @@ -203,11 +205,17 @@ async def delete_collection( @attr.s -class BulkTransactionsClient(AsyncBaseBulkTransactionsClient): +class BulkTransactionsClient(AsyncBaseBulkTransactionsClient, ClientValidateMixIn): """Postgres bulk transactions.""" async def bulk_item_insert(self, items: Items, request: Request, **kwargs) -> str: """Bulk item insertion using pgstac.""" + collection_id = request.path_params["collection_id"] + + for item_id, item in items.items.items(): + self._validate_item(request, item, collection_id, item_id) + item["collection"] = collection_id + items_to_insert = list(items.items.values()) async with request.app.state.get_connection(request, "w") as conn: diff --git a/tests/clients/test_postgres.py b/tests/clients/test_postgres.py index 3622521..8a8d5ca 100644 --- a/tests/clients/test_postgres.py +++ b/tests/clients/test_postgres.py @@ -407,6 +407,96 @@ async def test_create_bulk_items_already_exist_upsert( assert resp.text == '"Successfully upserted 2 items."' +async def test_create_bulk_items_omit_collection( + app_client, load_test_data: Callable, load_test_collection +): + coll = load_test_collection + item = load_test_data("test_item.json") + + items = {} + for _ in range(2): + _item = deepcopy(item) + _item["id"] = str(uuid.uuid4()) + # remove collection ID here + del _item["collection"] + items[_item["id"]] = _item + + payload = {"items": items, "method": "insert"} + + resp = await app_client.post( + f"/collections/{coll['id']}/bulk_items", + json=payload, + ) + assert resp.status_code == 200 + assert resp.text == '"Successfully added 2 items."' + + for item_id in items.keys(): + resp = await app_client.get(f"/collections/{coll['id']}/items/{item_id}") + assert resp.status_code == 200 + + # Try creating the same items again, but using upsert. + # This should succeed. + payload["method"] = "upsert" + resp = await app_client.post( + f"/collections/{coll['id']}/bulk_items", + json=payload, + ) + assert resp.status_code == 200 + assert resp.text == '"Successfully upserted 2 items."' + + +async def test_create_bulk_items_collection_mismatch( + app_client, load_test_data: Callable, load_test_collection +): + coll = load_test_collection + item = load_test_data("test_item.json") + + items = {} + for _ in range(2): + _item = deepcopy(item) + _item["id"] = str(uuid.uuid4()) + _item["collection"] = "wrong-collection" + items[_item["id"]] = _item + + payload = {"items": items, "method": "insert"} + + resp = await app_client.post( + f"/collections/{coll['id']}/bulk_items", + json=payload, + ) + assert resp.status_code == 400 + assert ( + resp.json()["detail"] + == "Collection ID from path parameter (test-collection) does not match Collection ID from Item (wrong-collection)" + ) + + +async def test_create_bulk_items_id_mismatch( + app_client, load_test_data: Callable, load_test_collection +): + coll = load_test_collection + item = load_test_data("test_item.json") + + items = {} + for _ in range(2): + _item = deepcopy(item) + _item["id"] = str(uuid.uuid4()) + _item["collection"] = "wrong-collection" + items[_item["id"] + "wrong"] = _item + + payload = {"items": items, "method": "insert"} + + resp = await app_client.post( + f"/collections/{coll['id']}/bulk_items", + json=payload, + ) + assert resp.status_code == 400 + assert ( + resp.json()["detail"] + == "Collection ID from path parameter (test-collection) does not match Collection ID from Item (wrong-collection)" + ) + + # TODO since right now puts implement upsert # test_create_collection_already_exists # test create_item_already_exists