mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix delete to use get by id
This commit is contained in:
@@ -100,18 +100,18 @@ async def update_document(
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
|
||||
|
||||
@router.delete("/{path:path}", status_code=204)
|
||||
@router.delete("/{id:int}", status_code=204)
|
||||
async def delete_document(
|
||||
path: str,
|
||||
id: int,
|
||||
service: DocumentServiceDep,
|
||||
) -> None:
|
||||
"""Delete a document."""
|
||||
"""Delete a document by ID."""
|
||||
try:
|
||||
await service.delete_document(path)
|
||||
await service.delete_document_by_id(id)
|
||||
except DocumentNotFoundError:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Document not found: {path}"
|
||||
detail=f"Document not found: {id}"
|
||||
)
|
||||
except DocumentWriteError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
|
||||
@@ -266,6 +266,35 @@ class DocumentService(BaseService[DocumentRepository]):
|
||||
assert updated_document is not None, f"Could not update document {doc.id}"
|
||||
return updated_document
|
||||
|
||||
async def delete_document_by_id(self, id: int) -> None:
|
||||
"""
|
||||
Delete a document by ID.
|
||||
|
||||
Args:
|
||||
id: Document ID
|
||||
|
||||
Raises:
|
||||
DocumentNotFoundError: If document doesn't exist
|
||||
DocumentWriteError: If deletion fails
|
||||
"""
|
||||
logger.debug(f"Deleting document with ID {id}")
|
||||
|
||||
# Get document record first
|
||||
query = select(Document).where(Document.id == id)
|
||||
doc = await self.repository.find_one(query)
|
||||
if not doc:
|
||||
raise DocumentNotFoundError(f"Document not found: {id}")
|
||||
|
||||
# Delete file first since it's source of truth
|
||||
try:
|
||||
file_path = Path(doc.path)
|
||||
file_path.unlink(missing_ok=True)
|
||||
except Exception as e:
|
||||
raise DocumentWriteError(f"Failed to delete document {id}: {e}")
|
||||
|
||||
# Delete database record
|
||||
await self.repository.delete(doc.id)
|
||||
|
||||
async def delete_document(self, path: str) -> None:
|
||||
"""
|
||||
Delete a document.
|
||||
|
||||
@@ -148,19 +148,27 @@ async def test_delete_document(client: AsyncClient, tmp_path: Path):
|
||||
assert create_response.status_code == 201
|
||||
created = create_response.json()
|
||||
|
||||
# Delete document
|
||||
response = await client.delete(f"/documents/{test_doc['path']}")
|
||||
# Delete document by ID
|
||||
response = await client.delete(f"/documents/{created['id']}")
|
||||
assert response.status_code == 204
|
||||
|
||||
# Verify document is gone
|
||||
# Verify document is gone from filesystem
|
||||
doc_path = Path(test_doc["path"])
|
||||
assert not doc_path.exists()
|
||||
|
||||
# Verify 404 on subsequent get
|
||||
# Verify document is gone from DB (404 on get)
|
||||
get_response = await client.get(f"/documents/{created['id']}")
|
||||
assert get_response.status_code == 404
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_nonexistent_document(client: AsyncClient):
|
||||
"""Test deleting a document that doesn't exist."""
|
||||
response = await client.delete("/documents/99999")
|
||||
assert response.status_code == 404
|
||||
assert "not found" in response.json()["detail"].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_documents(client: AsyncClient, tmp_path: Path):
|
||||
"""Test document listing endpoint."""
|
||||
@@ -193,12 +201,12 @@ async def test_list_documents(client: AsyncClient, tmp_path: Path):
|
||||
# We should have both documents
|
||||
assert len(data) == 2
|
||||
|
||||
# Verify all paths are present
|
||||
paths = {item["path"] for item in data}
|
||||
expected_paths = {doc["path"] for doc in docs}
|
||||
assert paths == expected_paths
|
||||
# Verify all documents are present by ID
|
||||
ids = {item["id"] for item in data}
|
||||
expected_ids = {doc["id"] for doc in created_docs}
|
||||
assert ids == expected_ids
|
||||
|
||||
# Verify metadata was preserved
|
||||
for item in data:
|
||||
matching_doc = next(d for d in docs if d["path"] == item["path"])
|
||||
matching_doc = next(d for d in created_docs if d["id"] == item["id"])
|
||||
assert item["doc_metadata"] == matching_doc["doc_metadata"]
|
||||
Reference in New Issue
Block a user