Skip to content

Commit bb838c9

Browse files
refactor: use FileService.ensure_directory() for project directory creation
Replace direct Path.mkdir() calls with FileService.ensure_directory() to enable future cloud (S3) compatibility. ConfigManager.add_project() is now pure config management — directory creation is delegated to ProjectService via FileService. - Make FileService.markdown_processor Optional (not needed for ensure_directory) - Add file_service: Optional[FileService] to ProjectService.__init__ - ProjectService.add_project() and move_project() use file_service.ensure_directory() - deps/services.py injects a system-level FileService into ProjectService - Update tests/conftest.py project_service fixture to include file_service - Update test_config.py: test that ConfigManager.add_project() never creates dirs Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
1 parent eb25fb4 commit bb838c9

6 files changed

Lines changed: 47 additions & 32 deletions

File tree

src/basic_memory/config.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -685,12 +685,8 @@ def add_project(self, name: str, path: str) -> ProjectConfig:
685685
if project_name: # pragma: no cover
686686
raise ValueError(f"Project '{name}' already exists")
687687

688-
# Ensure the path exists on disk (skip in cloud mode where storage is S3)
689-
project_path = Path(path)
690-
if not self.config.project_root:
691-
project_path.mkdir(parents=True, exist_ok=True) # pragma: no cover
692-
693688
# Load config, modify it, and save it
689+
project_path = Path(path)
694690
config = self.load_config()
695691
config.projects[name] = ProjectEntry(path=str(project_path))
696692
self.save_config(config)

src/basic_memory/deps/services.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import asyncio
1111
import os
12+
from pathlib import Path
1213
from typing import Annotated, Any, Callable, Coroutine, Mapping, Protocol
1314

1415
from fastapi import Depends
@@ -549,9 +550,15 @@ async def _reindex_project(**_: Any) -> None:
549550

550551
async def get_project_service(
551552
project_repository: ProjectRepositoryDep,
553+
app_config: AppConfigDep,
552554
) -> ProjectService:
553-
"""Create ProjectService with repository."""
554-
return ProjectService(repository=project_repository)
555+
"""Create ProjectService with repository and a system-level FileService for directory operations."""
556+
# A system-level FileService for project directory creation (no project-specific base_path needed).
557+
# ensure_directory() accepts absolute paths and ignores base_path for those, so Path.home() is safe.
558+
entity_parser = EntityParser(Path.home())
559+
markdown_processor = MarkdownProcessor(entity_parser, app_config=app_config)
560+
file_service = FileService(Path.home(), markdown_processor, app_config=app_config)
561+
return ProjectService(repository=project_repository, file_service=file_service)
555562

556563

557564
ProjectServiceDep = Annotated[ProjectService, Depends(get_project_service)]

src/basic_memory/services/file_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class FileService:
4343
def __init__(
4444
self,
4545
base_path: Path,
46-
markdown_processor: MarkdownProcessor,
46+
markdown_processor: Optional[MarkdownProcessor] = None,
4747
max_concurrent_files: int = 10,
4848
app_config: Optional["BasicMemoryConfig"] = None,
4949
):

src/basic_memory/services/project_service.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import shutil
77
from datetime import datetime
88
from pathlib import Path
9-
from typing import Dict, Optional, Sequence
9+
from typing import TYPE_CHECKING, Dict, Optional, Sequence
1010

1111

1212
from loguru import logger
@@ -30,16 +30,22 @@
3030
)
3131
from basic_memory.utils import generate_permalink
3232

33+
if TYPE_CHECKING: # pragma: no cover
34+
from basic_memory.services.file_service import FileService
35+
3336

3437
class ProjectService:
3538
"""Service for managing Basic Memory projects."""
3639

3740
repository: ProjectRepository
3841

39-
def __init__(self, repository: ProjectRepository):
42+
def __init__(
43+
self, repository: ProjectRepository, file_service: Optional["FileService"] = None
44+
):
4045
"""Initialize the project service."""
4146
super().__init__()
4247
self.repository = repository
48+
self.file_service = file_service
4349

4450
@property
4551
def config_manager(self) -> ConfigManager:
@@ -205,6 +211,16 @@ async def add_project(self, name: str, path: str, set_default: bool = False) ->
205211
f"Projects cannot share directory trees."
206212
)
207213

214+
# Ensure the project directory exists on disk.
215+
# Trigger: project_root not set means local filesystem mode (not S3/cloud)
216+
# Why: FileService (or future S3FileService) provides cloud-compatible directory creation;
217+
# direct Path.mkdir() bypasses this abstraction
218+
# Outcome: directory exists before config/DB entries are written
219+
if not self.config_manager.config.project_root:
220+
if self.file_service is None:
221+
raise ValueError("file_service is required for local project directory creation")
222+
await self.file_service.ensure_directory(Path(resolved_path))
223+
208224
# First add to config file (this validates project uniqueness and keeps
209225
# config + database aligned for all backends).
210226
self.config_manager.add_project(name, resolved_path)
@@ -460,8 +476,13 @@ async def move_project(self, name: str, new_path: str) -> None:
460476
raise ValueError(f"Project '{name}' not found in configuration")
461477

462478
# Create the new directory if it doesn't exist (skip in cloud mode where storage is S3)
479+
# Trigger: project_root not set means local filesystem mode
480+
# Why: FileService (or future S3FileService) provides cloud-compatible directory creation
481+
# Outcome: destination directory exists before config/DB are updated
463482
if not self.config_manager.config.project_root:
464-
Path(resolved_path).mkdir(parents=True, exist_ok=True)
483+
if self.file_service is None:
484+
raise ValueError("file_service is required for local project directory creation")
485+
await self.file_service.ensure_directory(Path(resolved_path))
465486

466487
# Update in configuration
467488
config = self.config_manager.load_config()

tests/conftest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,10 @@ async def sample_entity(entity_repository: EntityRepository) -> Entity:
466466
@pytest_asyncio.fixture
467467
async def project_service(
468468
project_repository: ProjectRepository,
469+
file_service: FileService,
469470
) -> ProjectService:
470-
"""Create ProjectService with repository."""
471-
return ProjectService(repository=project_repository)
471+
"""Create ProjectService with repository and file service for directory operations."""
472+
return ProjectService(repository=project_repository, file_service=file_service)
472473

473474

474475
@pytest_asyncio.fixture

tests/test_config.py

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -580,23 +580,15 @@ def test_add_project_uses_platform_native_separators(self, monkeypatch):
580580
assert "/" in saved_path
581581
assert saved_path == str(project_path)
582582

583-
def test_add_project_skips_mkdir_when_project_root_set(self, monkeypatch):
584-
"""Test that add_project skips mkdir when project_root is set (cloud mode).
583+
def test_add_project_never_creates_directory(self):
584+
"""Test that ConfigManager.add_project() is pure config management — no mkdir.
585585
586-
In cloud mode, file storage is S3 — no local directories needed.
587-
Without this, add_project crashes on read-only filesystems (e.g. macOS root /).
586+
Directory creation is delegated to ProjectService via FileService, which
587+
supports both local and cloud (S3) backends.
588588
"""
589-
import basic_memory.config
590-
591589
with tempfile.TemporaryDirectory() as temp_dir:
592590
temp_path = Path(temp_dir)
593591

594-
# Simulate cloud mode: project_root set, postgres backend
595-
monkeypatch.setenv("BASIC_MEMORY_PROJECT_ROOT", "/cloud-root")
596-
monkeypatch.setenv("BASIC_MEMORY_DATABASE_BACKEND", "postgres")
597-
# Clear module-level cache so env vars are picked up
598-
basic_memory.config._CONFIG_CACHE = None
599-
600592
config_manager = ConfigManager()
601593
config_manager.config_dir = temp_path / "basic-memory"
602594
config_manager.config_file = config_manager.config_dir / "config.json"
@@ -605,17 +597,15 @@ def test_add_project_skips_mkdir_when_project_root_set(self, monkeypatch):
605597
initial_config = BasicMemoryConfig(projects={})
606598
config_manager.save_config(initial_config)
607599

608-
# Use a path that would fail mkdir on most systems (read-only root)
609-
config_manager.add_project("test-project", "/nonexistent/cloud/path")
600+
# Use a path that does not exist — ConfigManager should not create it
601+
nonexistent_path = str(temp_path / "nonexistent" / "project")
602+
config_manager.add_project("test-project", nonexistent_path)
610603

611604
# Verify project was added to config without creating the directory
612605
config = config_manager.load_config()
613606
assert "test-project" in config.projects
614-
assert config.projects["test-project"].path == "/nonexistent/cloud/path"
615-
assert not Path("/nonexistent/cloud/path").exists()
616-
617-
# Clean up cache for subsequent tests
618-
basic_memory.config._CONFIG_CACHE = None
607+
assert config.projects["test-project"].path == nonexistent_path
608+
assert not Path(nonexistent_path).exists()
619609

620610
def test_model_post_init_uses_platform_native_separators(self, config_home, monkeypatch):
621611
"""Test that model_post_init uses platform-native separators."""

0 commit comments

Comments
 (0)