Signal HTTP 404 error when trying to delete a non existing record #159

Merged
christian-monch merged 3 commits from delete-404 into master 2025-10-17 11:13:48 +00:00
10 changed files with 88 additions and 60 deletions

View file

@ -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 /<collection>/record`
- `DELETE /<collection>/curated/record`
- `DELETE /{collection}/incoming/{label}/record`
# 4.7.0 (2025-10-16)
## New features

View file

@ -1 +1 @@
__version__ = '4.7.0'
__version__ = '5.0.0'

View file

@ -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()

View file

@ -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(

View file

@ -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,

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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}).',
)