diff --git a/src/basic_memory/alembic/versions/a1b2c3d4e5f6_fix_project_foreign_keys.py b/src/basic_memory/alembic/versions/a1b2c3d4e5f6_fix_project_foreign_keys.py new file mode 100644 index 00000000..b282a713 --- /dev/null +++ b/src/basic_memory/alembic/versions/a1b2c3d4e5f6_fix_project_foreign_keys.py @@ -0,0 +1,54 @@ +"""fix project foreign keys + +Revision ID: a1b2c3d4e5f6 +Revises: 647e7a75e2cd +Create Date: 2025-08-19 22:06:00.000000 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = "a1b2c3d4e5f6" +down_revision: Union[str, None] = "647e7a75e2cd" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Re-establish foreign key constraints that were lost during project table recreation. + + The migration 647e7a75e2cd recreated the project table but did not re-establish + the foreign key constraint from entity.project_id to project.id, causing + foreign key constraint failures when trying to delete projects with related entities. + """ + # SQLite doesn't allow adding foreign key constraints to existing tables easily + # We need to be careful and handle the case where the constraint might already exist + + with op.batch_alter_table("entity", schema=None) as batch_op: + # Try to drop existing foreign key constraint (may not exist) + try: + batch_op.drop_constraint("fk_entity_project_id", type_="foreignkey") + except Exception: + # Constraint may not exist, which is fine - we'll create it next + pass + + # Add the foreign key constraint with CASCADE DELETE + # This ensures that when a project is deleted, all related entities are also deleted + batch_op.create_foreign_key( + "fk_entity_project_id", + "project", + ["project_id"], + ["id"], + ondelete="CASCADE" + ) + + +def downgrade() -> None: + """Remove the foreign key constraint.""" + with op.batch_alter_table("entity", schema=None) as batch_op: + batch_op.drop_constraint("fk_entity_project_id", type_="foreignkey") \ No newline at end of file diff --git a/tests/db/test_issue_254_foreign_key_constraints.py b/tests/db/test_issue_254_foreign_key_constraints.py new file mode 100644 index 00000000..9e7175d8 --- /dev/null +++ b/tests/db/test_issue_254_foreign_key_constraints.py @@ -0,0 +1,154 @@ +"""Test to verify that issue #254 is fixed. + +Issue #254: Foreign key constraint failures when deleting projects with related entities. + +The issue was that when migration 647e7a75e2cd recreated the project table, +it did not re-establish the foreign key constraint from entity.project_id to project.id +with CASCADE DELETE, causing foreign key constraint failures when trying to delete +projects that have related entities. + +Migration a1b2c3d4e5f6 was created to fix this by adding the missing foreign key +constraint with CASCADE DELETE behavior. + +This test file verifies that the fix works correctly in production databases +that have had the migration applied. +""" +from datetime import datetime, timezone + +import pytest + +from basic_memory.services.project_service import ProjectService + + +#@pytest.mark.skip(reason="Issue #254 not fully resolved yet - foreign key constraint errors still occur") +@pytest.mark.asyncio +async def test_issue_254_foreign_key_constraint_fix(project_service: ProjectService, tmp_path): + """Test to verify issue #254 is fixed: project removal with foreign key constraints. + + This test reproduces the exact scenario from issue #254: + 1. Create a project + 2. Create entities, observations, and relations linked to that project + 3. Attempt to remove the project + 4. Verify it succeeds without "FOREIGN KEY constraint failed" errors + 5. Verify all related data is properly cleaned up via CASCADE DELETE + + Once issue #254 is fully fixed, remove the @pytest.mark.skip decorator. + """ + test_project_name = "issue-254-verification" + test_project_path = str(tmp_path / "issue-254-verification") + + # Step 1: Create test project + await project_service.add_project(test_project_name, test_project_path) + project = await project_service.get_project(test_project_name) + assert project is not None, "Project should be created successfully" + + # Step 2: Create related entities that would cause foreign key constraint issues + from basic_memory.repository.entity_repository import EntityRepository + from basic_memory.repository.observation_repository import ObservationRepository + from basic_memory.repository.relation_repository import RelationRepository + + entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id) + obs_repo = ObservationRepository(project_service.repository.session_maker, project_id=project.id) + rel_repo = RelationRepository(project_service.repository.session_maker, project_id=project.id) + + # Create entity + entity_data = { + "title": "Issue 254 Test Entity", + "entity_type": "note", + "content_type": "text/markdown", + "project_id": project.id, + "permalink": "issue-254-entity", + "file_path": "issue-254-entity.md", + "checksum": "issue254test", + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + } + entity = await entity_repo.create(entity_data) + + # Create observation linked to entity + observation_data = { + "entity_id": entity.id, + "content": "This observation should be cascade deleted", + "category": "test" + } + observation = await obs_repo.create(observation_data) + + # Create relation involving the entity + relation_data = { + "from_id": entity.id, + "to_name": "some-other-entity", + "relation_type": "relates-to" + } + relation = await rel_repo.create(relation_data) + + # Step 3: Attempt to remove the project + # This is where issue #254 manifested - should NOT raise "FOREIGN KEY constraint failed" + try: + await project_service.remove_project(test_project_name) + except Exception as e: + if "FOREIGN KEY constraint failed" in str(e): + pytest.fail( + f"Issue #254 not fixed - foreign key constraint error still occurs: {e}. " + f"The migration a1b2c3d4e5f6 may not have been applied correctly or " + f"the CASCADE DELETE constraint is not working as expected." + ) + else: + # Re-raise unexpected errors + raise + + # Step 4: Verify project was successfully removed + removed_project = await project_service.get_project(test_project_name) + assert removed_project is None, "Project should have been removed" + + # Step 5: Verify related data was cascade deleted + remaining_entity = await entity_repo.find_by_id(entity.id) + assert remaining_entity is None, "Entity should have been cascade deleted" + + remaining_observation = await obs_repo.find_by_id(observation.id) + assert remaining_observation is None, "Observation should have been cascade deleted" + + remaining_relation = await rel_repo.find_by_id(relation.id) + assert remaining_relation is None, "Relation should have been cascade deleted" + + +@pytest.mark.asyncio +async def test_issue_254_reproduction(project_service: ProjectService, tmp_path): + """Test that reproduces issue #254 to document the current state. + + This test demonstrates the current behavior and will fail until the issue is fixed. + It serves as documentation of what the problem was. + """ + test_project_name = "issue-254-reproduction" + test_project_path = str(tmp_path / "issue-254-reproduction") + + # Create project and entity + await project_service.add_project(test_project_name, test_project_path) + project = await project_service.get_project(test_project_name) + + from basic_memory.repository.entity_repository import EntityRepository + entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id) + + entity_data = { + "title": "Reproduction Entity", + "entity_type": "note", + "content_type": "text/markdown", + "project_id": project.id, + "permalink": "reproduction-entity", + "file_path": "reproduction-entity.md", + "checksum": "repro123", + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + } + entity = await entity_repo.create(entity_data) + + # This should eventually work without errors once issue #254 is fixed + #with pytest.raises(Exception) as exc_info: + await project_service.remove_project(test_project_name) + + # Document the current error for tracking + # error_message = str(exc_info.value) + # assert any(keyword in error_message for keyword in [ + # "FOREIGN KEY constraint failed", + # "constraint", + # "integrity" + # ]), f"Expected foreign key or integrity constraint error, got: {error_message}" \ No newline at end of file diff --git a/tests/services/test_project_removal_bug.py b/tests/services/test_project_removal_bug.py new file mode 100644 index 00000000..06e97265 --- /dev/null +++ b/tests/services/test_project_removal_bug.py @@ -0,0 +1,125 @@ +"""Test for project removal bug #254.""" + +import os +from datetime import timezone, datetime + +import pytest + +from basic_memory.services.project_service import ProjectService + + +@pytest.mark.asyncio +async def test_remove_project_with_related_entities(project_service: ProjectService, tmp_path): + """Test removing a project that has related entities (reproduces issue #254). + + This test verifies that projects with related entities (entities, observations, relations) + can be properly deleted without foreign key constraint violations. + + The bug was caused by missing foreign key constraints with CASCADE DELETE after + the project table was recreated in migration 647e7a75e2cd. + """ + test_project_name = f"test-remove-with-entities-{os.urandom(4).hex()}" + test_project_path = str(tmp_path / "test-remove-with-entities") + + # Make sure the test directory exists + os.makedirs(test_project_path, exist_ok=True) + + try: + # Step 1: Add the test project + await project_service.add_project(test_project_name, test_project_path) + + # Verify project exists + project = await project_service.get_project(test_project_name) + assert project is not None + + # Step 2: Create related entities for this project + from basic_memory.repository.entity_repository import EntityRepository + entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id) + + entity_data = { + "title": "Test Entity for Deletion", + "entity_type": "note", + "content_type": "text/markdown", + "project_id": project.id, + "permalink": "test-deletion-entity", + "file_path": "test-deletion-entity.md", + "checksum": "test123", + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + } + entity = await entity_repo.create(entity_data) + assert entity is not None + + # Step 3: Create observations for the entity + from basic_memory.repository.observation_repository import ObservationRepository + obs_repo = ObservationRepository(project_service.repository.session_maker, project_id=project.id) + + observation_data = { + "entity_id": entity.id, + "content": "This is a test observation", + "category": "note" + } + observation = await obs_repo.create(observation_data) + assert observation is not None + + # Step 4: Create relations involving the entity + from basic_memory.repository.relation_repository import RelationRepository + rel_repo = RelationRepository(project_service.repository.session_maker, project_id=project.id) + + relation_data = { + "from_id": entity.id, + "to_name": "some-target-entity", + "relation_type": "relates-to" + } + relation = await rel_repo.create(relation_data) + assert relation is not None + + # Step 5: Attempt to remove the project + # This should work with proper cascade delete, or fail with foreign key constraint + await project_service.remove_project(test_project_name) + + # Step 6: Verify everything was properly deleted + + # Project should be gone + removed_project = await project_service.get_project(test_project_name) + assert removed_project is None, "Project should have been removed" + + # Related entities should be cascade deleted + remaining_entity = await entity_repo.find_by_id(entity.id) + assert remaining_entity is None, "Entity should have been cascade deleted" + + # Observations should be cascade deleted + remaining_obs = await obs_repo.find_by_id(observation.id) + assert remaining_obs is None, "Observation should have been cascade deleted" + + # Relations should be cascade deleted + remaining_rel = await rel_repo.find_by_id(relation.id) + assert remaining_rel is None, "Relation should have been cascade deleted" + + except Exception as e: + # Check if this is the specific foreign key constraint error from the bug report + if "FOREIGN KEY constraint failed" in str(e): + pytest.fail( + f"Bug #254 reproduced: {e}. " + "This indicates missing foreign key constraints with CASCADE DELETE. " + "Run migration a1b2c3d4e5f6_fix_project_foreign_keys.py to fix this." + ) + else: + # Re-raise other unexpected errors + raise e + + finally: + # Clean up - remove project if it still exists + if test_project_name in project_service.projects: + try: + await project_service.remove_project(test_project_name) + except Exception: + # Manual cleanup if remove_project fails + try: + project_service.config_manager.remove_project(test_project_name) + except Exception: + pass + + project = await project_service.get_project(test_project_name) + if project: + await project_service.repository.delete(project.id) \ No newline at end of file diff --git a/tests/test_production_cascade_delete.py b/tests/test_production_cascade_delete.py new file mode 100644 index 00000000..b299f036 --- /dev/null +++ b/tests/test_production_cascade_delete.py @@ -0,0 +1,328 @@ +#!/usr/bin/env python3 +""" +Test script to verify cascade delete behavior on production SQLite database. + +This script tests whether foreign key constraints with CASCADE DELETE are properly +configured in the production database at ~/.basic-memory/memory.db. + +Usage: python test_production_cascade_delete.py +""" + +import asyncio +import os +import sqlite3 +import sys +from datetime import datetime, timezone +from pathlib import Path +from typing import Optional + +import asyncio +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker, AsyncSession + + +class ProductionCascadeTest: + """Test cascade delete behavior on production database.""" + + def __init__(self, db_path: Optional[Path] = None): + """Initialize test with database path.""" + if db_path is None: + # Default to standard Basic Memory location + home_dir = Path.home() + self.db_path = home_dir / ".basic-memory" / "memory.db" + else: + self.db_path = db_path + + # Create backup path + self.backup_path = self.db_path.with_suffix('.db.backup') + + self.engine = None + self.session_maker = None + + async def setup(self): + """Setup database connection.""" + if not self.db_path.exists(): + print(f"โŒ Production database not found at: {self.db_path}") + print("Please ensure Basic Memory has been initialized and the database exists.") + sys.exit(1) + + print(f"๐Ÿ“ Using database: {self.db_path}") + + # Create backup + print(f"๐Ÿ’พ Creating backup: {self.backup_path}") + import shutil + shutil.copy2(self.db_path, self.backup_path) + + # Connect to database + db_url = f"sqlite+aiosqlite:///{self.db_path}" + self.engine = create_async_engine(db_url, connect_args={"check_same_thread": False}) + self.session_maker = async_sessionmaker(self.engine, expire_on_commit=False) + + async def cleanup(self): + """Cleanup database connection.""" + if self.engine: + await self.engine.dispose() + + async def check_foreign_keys_enabled(self) -> bool: + """Check if foreign keys are enabled in this session.""" + async with self.session_maker() as session: + # Enable foreign keys like production does + await session.execute(text("PRAGMA foreign_keys=ON")) + + result = await session.execute(text("PRAGMA foreign_keys")) + fk_enabled = result.fetchone()[0] + return bool(fk_enabled) + + async def check_schema(self): + """Check current database schema for foreign key constraints.""" + async with self.session_maker() as session: + await session.execute(text("PRAGMA foreign_keys=ON")) + + # Check entity table foreign keys + result = await session.execute(text("PRAGMA foreign_key_list(entity)")) + entity_fks = result.fetchall() + + print("๐Ÿ” Current entity table foreign key constraints:") + for fk in entity_fks: + print(f" - Column: {fk[3]} -> {fk[2]}.{fk[4]} (ON DELETE: {fk[6]})") + + # Check if CASCADE DELETE is configured + has_cascade = any(fk[6] == 'CASCADE' for fk in entity_fks) + + if has_cascade: + print("โœ… CASCADE DELETE is configured") + else: + print("โŒ CASCADE DELETE is NOT configured (uses NO ACTION)") + + return has_cascade + + async def create_test_data(self) -> tuple[int, int]: + """Create test project and entity. Returns (project_id, entity_id).""" + async with self.session_maker() as session: + await session.execute(text("PRAGMA foreign_keys=ON")) + + # Create test project + project_sql = """ + INSERT INTO project (name, description, permalink, path, is_active, is_default, created_at, updated_at) + VALUES (:name, :description, :permalink, :path, :is_active, :is_default, :created_at, :updated_at) + """ + now = datetime.now(timezone.utc) + + result = await session.execute(text(project_sql), { + "name": "cascade-test-project", + "description": "Test project for cascade delete verification", + "permalink": "cascade-test-project", + "path": "/tmp/cascade-test", + "is_active": True, + "is_default": False, + "created_at": now, + "updated_at": now + }) + project_id = result.lastrowid + + # Create test entity linked to project + entity_sql = """ + INSERT INTO entity (title, entity_type, content_type, project_id, permalink, file_path, + checksum, created_at, updated_at) + VALUES (:title, :entity_type, :content_type, :project_id, :permalink, :file_path, + :checksum, :created_at, :updated_at) + """ + + result = await session.execute(text(entity_sql), { + "title": "Cascade Test Entity", + "entity_type": "note", + "content_type": "text/markdown", + "project_id": project_id, + "permalink": "cascade-test-entity", + "file_path": "cascade-test-entity.md", + "checksum": "test-checksum", + "created_at": now, + "updated_at": now + }) + entity_id = result.lastrowid + + await session.commit() + + print(f"๐Ÿ“ Created test project (ID: {project_id}) and entity (ID: {entity_id})") + return project_id, entity_id + + async def verify_test_data_exists(self, project_id: int, entity_id: int) -> bool: + """Verify test data exists before deletion.""" + async with self.session_maker() as session: + # Check project exists + result = await session.execute( + text("SELECT COUNT(*) FROM project WHERE id = :project_id"), {"project_id": project_id} + ) + project_count = result.fetchone()[0] + + # Check entity exists + result = await session.execute( + text("SELECT COUNT(*) FROM entity WHERE id = :entity_id"), {"entity_id": entity_id} + ) + entity_count = result.fetchone()[0] + + exists = project_count > 0 and entity_count > 0 + if exists: + print(f"โœ… Test data verified: project ({project_count}) and entity ({entity_count}) exist") + else: + print(f"โŒ Test data missing: project ({project_count}) and entity ({entity_count})") + + return exists + + async def test_cascade_delete(self, project_id: int, entity_id: int) -> bool: + """Test if deleting project cascades to delete entity.""" + async with self.session_maker() as session: + await session.execute(text("PRAGMA foreign_keys=ON")) + + try: + # Attempt to delete project + print(f"๐Ÿ—‘๏ธ Attempting to delete project (ID: {project_id})...") + + result = await session.execute( + text("DELETE FROM project WHERE id = :project_id"), {"project_id": project_id} + ) + + if result.rowcount == 0: + print("โŒ Project deletion failed - no rows affected") + return False + + await session.commit() + print("โœ… Project deletion succeeded") + + # Check if entity was cascade deleted + result = await session.execute( + text("SELECT COUNT(*) FROM entity WHERE id = :entity_id"), {"entity_id": entity_id} + ) + entity_count = result.fetchone()[0] + + if entity_count == 0: + print("โœ… CASCADE DELETE working: Entity was automatically deleted") + return True + else: + print("โŒ CASCADE DELETE NOT working: Entity still exists after project deletion") + return False + + except Exception as e: + await session.rollback() + print(f"โŒ Project deletion failed with error: {e}") + + # Check if it's a foreign key constraint error + if "FOREIGN KEY constraint failed" in str(e): + print("๐Ÿ” This confirms foreign key constraints are enforced but CASCADE DELETE is not configured") + + return False + + async def cleanup_test_data(self, project_id: int, entity_id: int): + """Clean up any remaining test data.""" + async with self.session_maker() as session: + await session.execute(text("PRAGMA foreign_keys=ON")) + + try: + # Delete entity first (in case cascade didn't work) + await session.execute(text("DELETE FROM entity WHERE id = :entity_id"), {"entity_id": entity_id}) + + # Delete project + await session.execute(text("DELETE FROM project WHERE id = :project_id"), {"project_id": project_id}) + + await session.commit() + print("๐Ÿงน Cleaned up any remaining test data") + + except Exception as e: + print(f"โš ๏ธ Error during cleanup: {e}") + await session.rollback() + + async def restore_backup(self): + """Restore database from backup.""" + if self.backup_path.exists(): + print(f"๐Ÿ”„ Restoring database from backup...") + import shutil + shutil.copy2(self.backup_path, self.db_path) + print("โœ… Database restored from backup") + + # Remove backup file + self.backup_path.unlink() + print("๐Ÿ—‘๏ธ Backup file removed") + else: + print("โš ๏ธ No backup file found to restore") + + async def run_test(self) -> bool: + """Run the complete cascade delete test.""" + print("๐Ÿงช Production Database CASCADE DELETE Test") + print("=" * 50) + + try: + await self.setup() + + # Check if foreign keys are enabled + fk_enabled = await self.check_foreign_keys_enabled() + print(f"๐Ÿ” Foreign keys enabled: {fk_enabled}") + + if not fk_enabled: + print("โŒ Foreign keys are not enabled - this test requires foreign key enforcement") + return False + + # Check current schema + has_cascade = await self.check_schema() + + # Create test data + project_id, entity_id = await self.create_test_data() + + # Verify test data exists + if not await self.verify_test_data_exists(project_id, entity_id): + return False + + # Test cascade delete + cascade_works = await self.test_cascade_delete(project_id, entity_id) + + # Clean up any remaining test data + await self.cleanup_test_data(project_id, entity_id) + + print("\n" + "=" * 50) + print("๐Ÿงช TEST RESULTS:") + print(f" Schema has CASCADE DELETE: {has_cascade}") + print(f" CASCADE DELETE works: {cascade_works}") + + if has_cascade and cascade_works: + print("โœ… PASS: Foreign key constraints are properly configured with CASCADE DELETE") + elif not has_cascade and not cascade_works: + print("โŒ FAIL: Foreign key constraints are missing CASCADE DELETE configuration") + print("๐Ÿ’ก This confirms issue #254 - migration a1b2c3d4e5f6 is needed") + else: + print("โš ๏ธ MIXED: Unexpected result combination") + + return cascade_works + + except Exception as e: + print(f"๐Ÿ’ฅ Test failed with error: {e}") + return False + finally: + await self.cleanup() + # Always restore backup to avoid leaving test data + await self.restore_backup() + + +async def main(): + """Main test function.""" + import argparse + + parser = argparse.ArgumentParser(description="Test cascade delete on production database") + parser.add_argument("--db-path", type=Path, help="Path to database file (default: ~/.basic-memory/memory.db)") + parser.add_argument("--no-backup", action="store_true", help="Skip creating backup (dangerous)") + + args = parser.parse_args() + + if args.no_backup: + print("โš ๏ธ WARNING: Running without backup!") + response = input("Are you sure? Type 'yes' to continue: ") + if response.lower() != 'yes': + print("โŒ Aborted") + return + + test = ProductionCascadeTest(args.db_path) + success = await test.run_test() + + sys.exit(0 if success else 1) + + +if __name__ == "__main__": + asyncio.run(main()) \ No newline at end of file