Signal HTTP 404 error when trying to delete a non existing record #159
10 changed files with 88 additions and 60 deletions
15
CHANGELOG.md
15
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 /<collection>/record`
|
||||
- `DELETE /<collection>/curated/record`
|
||||
- `DELETE /{collection}/incoming/{label}/record`
|
||||
|
||||
|
||||
# 4.7.0 (2025-10-16)
|
||||
|
||||
## New features
|
||||
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
__version__ = '4.7.0'
|
||||
__version__ = '5.0.0'
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}).',
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue