fix document api

src/basic_memory/api/routers/documents.py

@@ -22,7 +22,7 @@ async def create_document(
     service: DocumentServiceDep,
 ) -> DocumentCreateResponse:
     """Create a new document.
-
+    
     The document will be created with appropriate frontmatter including:
     - Generated ID
     - Creation timestamp
@@ -35,18 +35,18 @@ async def create_document(
             content=doc.content,
             metadata=doc.doc_metadata,
         )
-        return DocumentCreateResponse.from_orm(document)
+        return DocumentCreateResponse.model_validate(document.__dict__)
     except DocumentWriteError as e:
         raise HTTPException(status_code=400, detail=str(e))
 
 
-@router.get("/", response_model=List[DocumentResponse])
+@router.get("/", response_model=List[DocumentCreateResponse])
 async def list_documents(
     service: DocumentServiceDep,
-) -> List[DocumentResponse]:
-    """List all documents."""
+) -> List[DocumentCreateResponse]:
+    """List all documents (without content)."""
     documents = await service.list_documents()
-    return [DocumentResponse.from_orm(doc) for doc in documents]
+    return [DocumentCreateResponse.model_validate(doc.__dict__) for doc in documents]
 
 
 @router.get("/{path:path}", response_model=DocumentResponse)
@@ -57,57 +57,45 @@ async def get_document(
     """Get a document by path."""
     try:
         document, content = await service.read_document(path)
-        response = DocumentResponse.model_validate(document.__dict__ | {"content": content})
+        doc_dict = document.__dict__ | {"content": content}
+        response = DocumentResponse.model_validate(doc_dict)
         return response
     except DocumentNotFoundError:
-        raise HTTPException(status_code=404, detail=f"Document not found: {path}")
+        raise HTTPException(
+            status_code=404, 
+            detail=f"Document not found: {path}"
+        )
     except DocumentWriteError as e:
         raise HTTPException(status_code=400, detail=str(e))
 
 
-@router.put("/{path:path}", response_model=DocumentResponse)
+@router.put("/{id:int}", response_model=DocumentResponse)
 async def update_document(
-    path: str,
+    id: int,
     doc: DocumentUpdate,
     service: DocumentServiceDep,
 ) -> DocumentResponse:
-    """Update a document's content and/or metadata."""
+    """Update a document by ID."""
+    # Verify IDs match
+    if doc.id != id:
+        raise HTTPException(
+            status_code=400,
+            detail="Document ID in URL must match ID in request body"
+        )
+
     try:
-        document = await service.update_document(
-            path=path,
+        document = await service.update_document_by_id(
+            id=id,
             content=doc.content,
             metadata=doc.doc_metadata,
         )
-        return DocumentResponse.from_orm(document)
+        doc_dict = document.__dict__ | {"content": doc.content}
+        return DocumentResponse.model_validate(doc_dict)
     except DocumentNotFoundError:
-        raise HTTPException(status_code=404, detail=f"Document not found: {path}")
-    except DocumentWriteError as e:
-        raise HTTPException(status_code=400, detail=str(e))
-
-
-@router.patch("/{path:path}", response_model=DocumentResponse)
-async def patch_document(
-    path: str,
-    patch: DocumentPatch,
-    service: DocumentServiceDep,
-) -> DocumentResponse:
-    """Partially update a document."""
-    # Require full content updates for now
-    if patch.content is None:
         raise HTTPException(
-            status_code=400,
-            detail=("Partial content updates not yet implemented. " "Please provide full content."),
+            status_code=404, 
+            detail=f"Document not found: {id}"
         )
-
-    try:
-        document = await service.update_document(
-            path=path,
-            content=patch.content,
-            metadata=patch.doc_metadata,
-        )
-        return DocumentResponse.from_orm(document)
-    except DocumentNotFoundError:
-        raise HTTPException(status_code=404, detail=f"Document not found: {path}")
     except DocumentWriteError as e:
         raise HTTPException(status_code=400, detail=str(e))
 
@@ -121,6 +109,9 @@ async def delete_document(
     try:
         await service.delete_document(path)
     except DocumentNotFoundError:
-        raise HTTPException(status_code=404, detail=f"Document not found: {path}")
+        raise HTTPException(
+            status_code=404,
+            detail=f"Document not found: {path}"
+        )
     except DocumentWriteError as e:
         raise HTTPException(status_code=400, detail=str(e))

src/basic_memory/schemas/request.py

@@ -1,16 +1,4 @@
-"""Request schemas for interacting with the knowledge graph.
-
-This module defines all the request schemas for creating, retrieving, and managing
-knowledge graph entities. Each schema includes validation rules and clear examples
-of proper usage.
-
-Request Types:
-1. Entity Creation - Create new nodes in the graph
-2. Observation Addition - Add facts to existing entities
-3. Relation Creation - Connect entities with typed edges
-4. Node Search - Find entities across the graph
-5. Node Retrieval - Load specific entities by ID
-"""
+"""Request schemas for interacting with the knowledge graph."""
 
 from typing import List, Optional, Annotated, Dict, Any
 
@@ -220,14 +208,14 @@ class DocumentCreate(BaseModel):
 
 
 class DocumentUpdate(BaseModel):
-    id: int
-    checksum: str
+    """Update an existing document by ID."""
+
+    id: int  # Document ID is required for updates
     content: str
     doc_metadata: Optional[Dict[str, Any]] = None
 
 
 class DocumentPatch(BaseModel):
     id: int
-    checksum: str
     content: Optional[str] = None
     doc_metadata: Optional[Dict[str, Any]] = None

src/basic_memory/services/document_service.py

@@ -3,11 +3,11 @@
 import hashlib
 from datetime import datetime, UTC
 from pathlib import Path
-from typing import Optional, Dict, Any, Sequence
+from typing import Optional, Dict, Any, List
 
 import yaml
-from icecream import ic
 from loguru import logger
+from sqlalchemy import select
 
 from basic_memory.models import Document
 from basic_memory.repository.document_repository import DocumentRepository
@@ -16,19 +16,16 @@ from basic_memory.services.service import BaseService
 
 class DocumentError(Exception):
     """Base exception for document operations."""
-
     pass
 
 
 class DocumentNotFoundError(DocumentError):
     """Raised when a document doesn't exist."""
-
     pass
 
 
 class DocumentWriteError(DocumentError):
     """Raised when document file operations fail."""
-
     pass
 
 
@@ -49,41 +46,37 @@ class DocumentService(BaseService[DocumentRepository]):
 
     async def ensure_parent_directory(self, path: Path) -> None:
         """
-        Ensure parent directory exists and is writable.
+        Ensure parent directory exists.
 
         Args:
             path: Path to check
 
         Raises:
-            DocumentWriteError: If directory cannot be created or is not writable
+            DocumentWriteError: If directory cannot be created
         """
         parent = path.parent
         try:
-            if not parent.exists():
-                parent.mkdir(parents=True)
-
-            # Verify we can write to it
-            test_file = parent / ".write_test"
-            test_file.touch()
-            test_file.unlink()
+            parent.mkdir(parents=True, exist_ok=True)
         except Exception as e:
-            raise DocumentWriteError(f"Directory not writable: {parent}: {e}")
+            raise DocumentWriteError(f"Failed to create directory: {parent}: {e}")
 
-    async def add_frontmatter(
-        self, content: str, doc_id: int, metadata: Optional[Dict[str, Any]] = None
-    ) -> str:
+    async def add_frontmatter(self, content: str, doc_id: int, metadata: Optional[Dict[str, Any]] = None) -> str:
         """Add frontmatter to document content."""
         # Generate frontmatter with timestamps
         now = datetime.now(UTC).isoformat()
-        frontmatter = {"id": doc_id, "created": now, "modified": now}
+        frontmatter = {
+            "id": doc_id,
+            "created": now,
+            "modified": now
+        }
         if metadata:
             frontmatter.update(metadata)
-
+            
         yaml_fm = yaml.dump(frontmatter, sort_keys=False)
         return f"---\n{yaml_fm}---\n\n{content}"
 
-    async def list_documents(self) -> Sequence[Document]:
-        """List all documents in the database."""
+    async def list_documents(self) -> List[Document]:
+        """List all documents."""
         return await self.repository.find_all()
 
     async def create_document(
@@ -105,13 +98,16 @@ class DocumentService(BaseService[DocumentRepository]):
         """
         logger.debug(f"Creating document at {path}")
 
-        # Ensure parent directories exist and are writable
+        # Ensure parent directories exist
         file_path = Path(path)
         await self.ensure_parent_directory(file_path)
 
         try:
             # 1. Create initial DB record to get ID
-            doc = await self.repository.create({"path": str(path), "doc_metadata": metadata})
+            doc = await self.repository.create({
+                "path": str(path),
+                "doc_metadata": metadata
+            })
 
             # 2. Add frontmatter with DB-generated ID
             content_with_frontmatter = await self.add_frontmatter(content, doc.id, metadata)
@@ -122,12 +118,11 @@ class DocumentService(BaseService[DocumentRepository]):
             # 4. Update DB with checksum to mark completion
             checksum = await self.compute_checksum(content_with_frontmatter)
             doc = await self.repository.update(doc.id, {"checksum": checksum})
-            ic(doc)
             return doc
 
         except Exception as e:
             # Clean up on any failure
-            if "doc" in locals():  # DB record was created
+            if 'doc' in locals():  # DB record was created
                 await self.repository.delete(doc.id)
             file_path.unlink(missing_ok=True)
             raise DocumentWriteError(f"Failed to create document: {e}")
@@ -167,6 +162,39 @@ class DocumentService(BaseService[DocumentRepository]):
 
         return doc, content
 
+    async def update_document_by_id(
+        self, id: int, content: str, metadata: Optional[Dict[str, Any]] = None
+    ) -> Document:
+        """
+        Update a document using its ID.
+        
+        Args:
+            id: Document ID
+            content: New content
+            metadata: Optional new metadata
+            
+        Returns:
+            Updated document record
+            
+        Raises:
+            DocumentNotFoundError: If document doesn't exist
+            DocumentWriteError: If update fails
+        """
+        logger.debug(f"Updating document with ID: {id}")
+
+        # Find document first
+        query = select(Document).where(Document.id == id)
+        document = await self.repository.find_one(query)
+        if not document:
+            raise DocumentNotFoundError(f"Document not found: {id}")
+
+        # Use existing path to update file
+        return await self.update_document(
+            path=document.path,
+            content=content,
+            metadata=metadata
+        )
+
     async def update_document(
         self, path: str, content: str, metadata: Optional[Dict[str, Any]] = None
     ) -> Document:
@@ -244,4 +272,4 @@ class DocumentService(BaseService[DocumentRepository]):
         # Delete database record if it exists
         doc = await self.repository.find_by_path(str(path))
         if doc:
-            await self.repository.delete(doc.id)
+            await self.repository.delete(doc.id)

tests/api/test_documents_router.py

@@ -5,8 +5,6 @@ from pathlib import Path
 import pytest
 from httpx import AsyncClient
 
-from basic_memory.schemas.response import DocumentCreateResponse
-
 
 @pytest.mark.asyncio
 async def test_create_document(client: AsyncClient, tmp_path: Path):
@@ -36,6 +34,19 @@ async def test_create_document(client: AsyncClient, tmp_path: Path):
     assert "# Test" in content  # Has our content
 
 
+@pytest.mark.asyncio
+async def test_create_document_invalid_path(client: AsyncClient, tmp_path: Path):
+    """Test creating document in non-existent directory."""
+    test_doc = {
+        "path": str(tmp_path / "nonexistent" / "test.md"),
+        "content": "test content",
+        "doc_metadata": {"type": "test"},
+    }
+
+    response = await client.post("/documents/", json=test_doc)
+    assert response.status_code == 201  # We now create parent directories
+
+
 @pytest.mark.asyncio
 async def test_get_document(client: AsyncClient, tmp_path: Path):
     """Test document retrieval endpoint."""
@@ -48,6 +59,7 @@ async def test_get_document(client: AsyncClient, tmp_path: Path):
     # Create document
     create_response = await client.post("/documents/", json=test_doc)
     assert create_response.status_code == 201
+    created = create_response.json()
 
     # Get document
     response = await client.get(f"/documents/{test_doc['path']}")
@@ -63,3 +75,127 @@ async def test_get_document(client: AsyncClient, tmp_path: Path):
     assert "id:" in content  # Has generated ID
     assert "# Test" in content  # Has heading
     assert "This is a test document" in content  # Has body
+
+
+@pytest.mark.asyncio
+async def test_get_nonexistent_document(client: AsyncClient, tmp_path: Path):
+    """Test getting a document that doesn't exist."""
+    response = await client.get(f"/documents/{tmp_path}/nonexistent.md")
+    assert response.status_code == 404
+    assert "not found" in response.json()["detail"].lower()
+
+
+@pytest.mark.asyncio
+async def test_update_document(client: AsyncClient, tmp_path: Path):
+    """Test document update endpoint using document ID."""
+    # Create initial document
+    test_doc = {
+        "path": str(tmp_path / "test.md"),
+        "content": "# Original\nOriginal content.",
+        "doc_metadata": {"type": "test", "status": "draft"},
+    }
+    create_response = await client.post("/documents/", json=test_doc)
+    assert create_response.status_code == 201
+    created = create_response.json()
+
+    # Update the document
+    update_doc = {
+        "id": created["id"],
+        "content": "# Updated\nUpdated content.",
+        "doc_metadata": {"type": "test", "status": "final"},
+    }
+    response = await client.put(f"/documents/{created['id']}", json=update_doc)
+    assert response.status_code == 200
+
+    data = response.json()
+    assert data["doc_metadata"] == update_doc["doc_metadata"]
+    assert "# Updated" in data["content"]
+    assert "Updated content" in data["content"]
+    
+    # Verify file was updated
+    doc_path = Path(test_doc["path"])
+    content = doc_path.read_text()
+    assert "# Updated" in content
+    assert "Updated content" in content
+
+
+@pytest.mark.asyncio
+async def test_update_nonexistent_document(client: AsyncClient):
+    """Test updating a document that doesn't exist."""
+    update_doc = {
+        "id": 99999,  # Non-existent ID
+        "content": "new content",
+        "doc_metadata": {"type": "test"},
+    }
+    response = await client.put("/documents/99999", json=update_doc)
+    assert response.status_code == 404
+    assert "not found" in response.json()["detail"].lower()
+
+
+@pytest.mark.asyncio
+async def test_delete_document(client: AsyncClient, tmp_path: Path):
+    """Test document deletion endpoint."""
+    test_doc = {
+        "path": str(tmp_path / "test.md"),
+        "content": "# Test\nTest content.",
+        "doc_metadata": {"type": "test"},
+    }
+
+    # Create document
+    create_response = await client.post("/documents/", json=test_doc)
+    assert create_response.status_code == 201
+
+    # Delete document
+    response = await client.delete(f"/documents/{test_doc['path']}")
+    assert response.status_code == 204
+
+    # Verify document is gone
+    doc_path = Path(test_doc["path"])
+    assert not doc_path.exists()
+
+    # Verify 404 on subsequent get
+    get_response = await client.get(f"/documents/{test_doc['path']}")
+    assert get_response.status_code == 404
+
+
+@pytest.mark.asyncio
+async def test_list_documents(client: AsyncClient, tmp_path: Path):
+    """Test document listing endpoint."""
+    # Create a few test documents
+    docs = [
+        {
+            "path": str(tmp_path / "doc1.md"),
+            "content": "# Doc 1",
+            "doc_metadata": {"type": "test", "number": 1},
+        },
+        {
+            "path": str(tmp_path / "doc2.md"),
+            "content": "# Doc 2",
+            "doc_metadata": {"type": "test", "number": 2},
+        },
+    ]
+
+    # Create all documents
+    created_docs = []
+    for doc in docs:
+        response = await client.post("/documents/", json=doc)
+        assert response.status_code == 201
+        created_docs.append(response.json())
+
+    # List all documents
+    response = await client.get("/documents/")
+    assert response.status_code == 200
+    data = response.json()
+    
+    # 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 metadata was preserved
+    for item in data:
+        matching_doc = next(d for d in docs if d["path"] == item["path"])
+        assert item["doc_metadata"] == matching_doc["doc_metadata"]