diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d9cd8c..45ecc84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,17 @@ -# 4.7.0 (2025-10-15) +# 5.0.0 (2025-10-17) + +## Breaking changes + +- Report an HTTP 404 error if the client tries to delete a record that does not + exist. This changes the behavior of the following endpoints if the record + identified by the provided PID is not found: + + - `DELETE //record` + - `DELETE //curated/record` + - `DELETE /{collection}/incoming/{label}/record` + + +# 4.7.0 (2025-10-16) ## New features diff --git a/dump_things_service/__about__.py b/dump_things_service/__about__.py index 372a507..a0f6658 100644 --- a/dump_things_service/__about__.py +++ b/dump_things_service/__about__.py @@ -1 +1 @@ -__version__ = '4.7.0' +__version__ = '5.0.0' diff --git a/dump_things_service/backends/record_dir.py b/dump_things_service/backends/record_dir.py index 54039ec..f0e9d07 100644 --- a/dump_things_service/backends/record_dir.py +++ b/dump_things_service/backends/record_dir.py @@ -213,7 +213,8 @@ class _RecordDirStore(StorageBackend): return False if self.index.remove_iri_info(iri) is False: - return False + msg = f'failed to remove IRI {iri} from index' + raise RuntimeError(msg) _, path, _ = index_entry Path(path).unlink() diff --git a/dump_things_service/curated.py b/dump_things_service/curated.py index e580851..e315ed5 100644 --- a/dump_things_service/curated.py +++ b/dump_things_service/curated.py @@ -19,6 +19,7 @@ from pydantic import BaseModel from dump_things_service import ( HTTP_401_UNAUTHORIZED, + HTTP_404_NOT_FOUND, HTTP_413_CONTENT_TOO_LARGE, HTTP_422_UNPROCESSABLE_CONTENT, ) @@ -31,6 +32,7 @@ from dump_things_service.lazy_list import ModifierList from dump_things_service.store.model_store import ModelStore from dump_things_service.utils import ( authenticate_token, + check_bounds, check_collection, cleaned_json, wrap_http_exception, @@ -63,20 +65,6 @@ router = APIRouter() add_pagination(router) -def check_bounds( - length: int | None, - max_length: int, - collection: str, - alternative_url: str -): - if length > max_length: - raise HTTPException( - status_code=HTTP_413_CONTENT_TOO_LARGE, - detail=f'Too many records found in collection "{collection}". ' - f'Please use pagination (/{collection}{alternative_url}).', - ) - - @router.get( '/{collection}/curated/records/{class_name}', tags=['Curator read records'], @@ -211,7 +199,9 @@ async def _read_curated_records( len(result_list), upper_bound, collection, - f'/curated/records/p/{class_name}', + f'/curated/records/p/{class_name}' + if class_name + else f'/curated/records/p/', ) return ModifierList( @@ -226,7 +216,13 @@ async def _delete_curated_record( api_key: str | None = None, ) -> bool: model_store, backend = await _get_store_and_backend(collection, api_key) - return backend.remove_record(model_store.pid_to_iri(pid)) + if not backend.remove_record(model_store.pid_to_iri(pid)): + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Could not remove record with PID '{pid}' from curated area " + f"of collection '{collection}'.", + ) + return True async def _get_store_and_backend( diff --git a/dump_things_service/incoming.py b/dump_things_service/incoming.py index 31ee63c..560a400 100644 --- a/dump_things_service/incoming.py +++ b/dump_things_service/incoming.py @@ -19,6 +19,7 @@ from pydantic import BaseModel from dump_things_service import ( HTTP_401_UNAUTHORIZED, + HTTP_404_NOT_FOUND, HTTP_413_CONTENT_TOO_LARGE, HTTP_422_UNPROCESSABLE_CONTENT, ) @@ -31,6 +32,7 @@ from dump_things_service.lazy_list import ModifierList from dump_things_service.store.model_store import ModelStore from dump_things_service.utils import ( authenticate_token, + check_bounds, check_collection, check_label, cleaned_json, @@ -70,20 +72,6 @@ router = APIRouter() add_pagination(router) -def check_bounds( - length: int | None, - max_length: int, - collection: str, - alternative_url: str -): - if length > max_length: - raise HTTPException( - status_code=HTTP_413_CONTENT_TOO_LARGE, - detail=f'Too many records found in collection "{collection}". ' - f'Please use pagination (/{collection}{alternative_url}).', - ) - - @router.get( '/{collection}/incoming/', tags=['Incoming read labels'], @@ -246,7 +234,9 @@ async def _incoming_read_records( len(result_list), upper_bound, collection, - f'/incoming/records/p/{class_name}', + f'/incoming/{label}/records/p/{class_name}' + if class_name + else f'/incoming/{label}/records/p/' ) return ModifierList( @@ -262,8 +252,13 @@ async def _incoming_delete_record( api_key: str | None = None, ) -> bool: model_store, backend = await _get_store_and_backend(collection, label, api_key) - return backend.remove_record(model_store.pid_to_iri(pid)) - + if not backend.remove_record(model_store.pid_to_iri(pid)): + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Could not remove record with PID '{pid}' from incoming " + f"area '{label}' of collection '{collection}'.", + ) + return True async def _get_store_and_backend( collection: str, diff --git a/dump_things_service/main.py b/dump_things_service/main.py index 6290d16..b80c58a 100644 --- a/dump_things_service/main.py +++ b/dump_things_service/main.py @@ -42,7 +42,6 @@ from dump_things_service import ( HTTP_400_BAD_REQUEST, HTTP_403_FORBIDDEN, HTTP_404_NOT_FOUND, - HTTP_413_CONTENT_TOO_LARGE, HTTP_422_UNPROCESSABLE_CONTENT, Format, config_file_name, @@ -81,6 +80,7 @@ from dump_things_service.model import ( get_subclasses, ) from dump_things_service.utils import ( + check_bounds, check_collection, combine_ttl, get_default_token_name, @@ -384,7 +384,7 @@ async def read_all_records( @app.get('/{collection}/records/p/', tags=['Read records']) -async def read_all_records( +async def read_all_records_paginated( collection: str, matching: str | None = None, format: Format = Format.json, # noqa A002 @@ -447,13 +447,6 @@ async def _read_all_records( api_key: str = Depends(api_key_header_scheme), bound: int | None = None, ) -> LazyList: - def check_bounds(length: int, max_length: int): - if length > max_length: - raise HTTPException( - status_code=HTTP_413_CONTENT_TOO_LARGE, - detail=f'Too many records found in collection "{collection}". ' - f'Please use pagination (/{collection}/records/p/).', - ) def convert_to_http_exception(e: BaseException): raise HTTPException( @@ -470,7 +463,7 @@ async def _read_all_records( if final_permissions.incoming_read: token_store_list = token_store.get_all_objects(matching=matching) if bound: - check_bounds(len(token_store_list), bound) + check_bounds(len(token_store_list), bound, collection, 'records/p/') result_list.add_list(token_store_list) if final_permissions.curated_read: @@ -480,7 +473,7 @@ async def _read_all_records( matching=matching, ) if bound: - check_bounds(len(curated_store_list), bound) + check_bounds(len(curated_store_list), bound, collection, 'records/p/') result_list.add_list(curated_store_list) # Sort the result list. @@ -510,15 +503,6 @@ async def _read_records_of_type( api_key: str = Depends(api_key_header_scheme), bound: int | None = None, ) -> LazyList: - def check_bounds(length: int, max_length: int): - if length > max_length: - raise HTTPException( - status_code=HTTP_413_CONTENT_TOO_LARGE, - detail=f'Too many records found for class "{class_name}" in ' - f'collection "{collection}". Please use pagination ' - f'(/{collection}/records/p/{class_name}).', - ) - def convert_to_http_exception(e: BaseException): raise HTTPException( status_code=HTTP_400_BAD_REQUEST, @@ -545,7 +529,7 @@ async def _read_records_of_type( matching=matching, ) if bound: - check_bounds(len(token_store_list), bound) + check_bounds(len(token_store_list), bound, collection, f'/records/p/{class_name}') result_list.add_list(token_store_list) if final_permissions.curated_read: @@ -557,7 +541,7 @@ async def _read_records_of_type( matching=matching, ) if bound: - check_bounds(len(curated_store_list), bound) + check_bounds(len(curated_store_list), bound, collection, f'/records/p/{class_name}') result_list.add_list(curated_store_list) # Sort the result list. @@ -593,10 +577,16 @@ async def delete_record( if not final_permissions.incoming_write: raise HTTPException( status_code=HTTP_403_FORBIDDEN, - detail=f'No write access to incoming data in collection "{collection}".', + detail=f"No write access to incoming data in collection '{collection}'.", ) - return token_store.delete_object(pid) + if not token_store.delete_object(pid): + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Could not remove record with PID '{pid}' from collection '{collection}'.", + ) + return True + # If we have a valid configuration, create dynamic endpoints and rebuild the diff --git a/dump_things_service/tests/test_basic.py b/dump_things_service/tests/test_basic.py index 481ec6d..b00bc67 100644 --- a/dump_things_service/tests/test_basic.py +++ b/dump_things_service/tests/test_basic.py @@ -4,6 +4,7 @@ from .. import ( HTTP_200_OK, HTTP_401_UNAUTHORIZED, HTTP_403_FORBIDDEN, + HTTP_404_NOT_FOUND, ) from ..__about__ import __version__ from ..utils import cleaned_json @@ -97,6 +98,12 @@ def test_delete(fastapi_client_simple): assert response.status_code == HTTP_200_OK assert response.json() is None + response = test_client.delete( + '/collection_1/record?pid=abc:delete-me', + headers={'x-dumpthings-token': 'token-1'}, + ) + assert response.status_code == HTTP_404_NOT_FOUND + def test_hashed_token(fastapi_client_simple): test_client, _ = fastapi_client_simple diff --git a/dump_things_service/tests/test_curated.py b/dump_things_service/tests/test_curated.py index 60e12cf..b43896c 100644 --- a/dump_things_service/tests/test_curated.py +++ b/dump_things_service/tests/test_curated.py @@ -103,3 +103,9 @@ def test_curated_delete(fastapi_client_simple): ) assert response.status_code == HTTP_200_OK assert response.json() is None + + response = test_client.delete( + '/collection_8/curated/record?pid=abc:delete-me', + headers={'x-dumpthings-token': 'token_1_xxxxx'}, + ) + assert response.status_code == HTTP_404_NOT_FOUND diff --git a/dump_things_service/tests/test_incoming.py b/dump_things_service/tests/test_incoming.py index e62beec..44eaaba 100644 --- a/dump_things_service/tests/test_incoming.py +++ b/dump_things_service/tests/test_incoming.py @@ -180,6 +180,12 @@ def test_incoming_delete(fastapi_client_simple): assert response.status_code == HTTP_200_OK assert response.json() is None + response = test_client.delete( + '/collection_7/incoming/admin_common/record?pid=abc:delete-me', + headers={'x-dumpthings-token': 'token_admin'}, + ) + assert response.status_code == HTTP_404_NOT_FOUND + def test_incoming_on_disk_only(fastapi_client_simple): test_client, data_root = fastapi_client_simple diff --git a/dump_things_service/utils.py b/dump_things_service/utils.py index 7b1472a..2dcf39f 100644 --- a/dump_things_service/utils.py +++ b/dump_things_service/utils.py @@ -445,3 +445,17 @@ def create_sqlite_token_store( db_path=store_dir / sqlite_record_file_name, order_by=order_by, ) + + +def check_bounds( + length: int | None, + max_length: int, + collection: str, + alternative_url: str +): + if length > max_length: + raise HTTPException( + status_code=HTTP_413_CONTENT_TOO_LARGE, + detail=f"Too many records found in collection '{collection}'. " + f'Please use pagination (/{collection}{alternative_url}).', + )