diff --git a/src/basic_memory/fileio.py b/src/basic_memory/fileio.py index dd3371c5..b3d5f09e 100644 --- a/src/basic_memory/fileio.py +++ b/src/basic_memory/fileio.py @@ -98,7 +98,7 @@ async def write_entity_file(project_entities_path: Path, entity_id: str, entity: except Exception as e: raise FileOperationError(f"Failed to finalize entity file: {str(e)}") from e - logger.debug(f"Wrote entity file: {entity_id}") + logger.debug(f"Wrote entity {entity_id} file: {entity_path}") return True diff --git a/src/basic_memory/repository/relation_repository.py b/src/basic_memory/repository/relation_repository.py index 5db2be0c..f17ccb5f 100644 --- a/src/basic_memory/repository/relation_repository.py +++ b/src/basic_memory/repository/relation_repository.py @@ -11,7 +11,13 @@ class RelationRepository(Repository[Relation]): def __init__(self, session): super().__init__(session, Relation) - + + async def find_by_entity(self, from_entity_id: str) -> Sequence[Relation]: + """Find all relations from a specific entity.""" + query = select(Relation).filter(Relation.from_id == from_entity_id) + result = await self.execute_query(query) + return result.scalars().all() + async def find_by_entities(self, from_id: str, to_id: str) -> Sequence[Relation]: """Find all relations between two entities.""" query = select(Relation).filter( diff --git a/src/basic_memory/services/memory_service.py b/src/basic_memory/services/memory_service.py index 43410464..5bddfe81 100644 --- a/src/basic_memory/services/memory_service.py +++ b/src/basic_memory/services/memory_service.py @@ -7,7 +7,7 @@ from basic_memory.models import Entity as EntityModel, Observation as Observatio from basic_memory.schemas import ( AddObservationsRequest, Entity, Relation ) -from basic_memory.fileio import write_entity_file, read_entity_file, EntityNotFoundError +from basic_memory.fileio import write_entity_file, read_entity_file, EntityNotFoundError, get_entity_path from basic_memory.services import EntityService, RelationService, ObservationService from loguru import logger @@ -32,6 +32,9 @@ class MemoryService: self.observation_service = observation_service logger.debug(f"Initialized MemoryService with path: {project_path}") + def get_entity_file_path(self, entity_id: str) -> Path: + return get_entity_path(self.entities_path, entity_id) + async def create_entities(self, entities_in: List[Entity]) -> List[EntityModel]: """Create multiple entities with their observations.""" logger.debug(f"Creating {len(entities_in)} entities") @@ -183,14 +186,111 @@ class MemoryService: logger.exception(f"Failed to add observations to entity: {observations_in.entity_id}") raise - async def delete_entities(self, entity_names: List[str]) -> None: - pass + async def delete_entities(self, entity_ids: List[str]) -> bool: + """Delete entities and their files.""" + logger.debug(f"Deleting entities: {entity_ids}") + try: + deleted = False + for entity_id in entity_ids: + # First read the entity to make sure it exists + try: + entity = await read_entity_file(self.entities_path, entity_id) + except EntityNotFoundError: + logger.debug(f"Entity file not found: {entity_id}") + continue - async def delete_observations(self, deletions: List[Dict[str, Any]]) -> None: - pass + # Delete the file first since it's source of truth + file_path = self.get_entity_file_path(entity_id) + if file_path.exists(): + file_path.unlink() + logger.debug(f"Deleted entity {entity_id} file: {file_path}") + + # Then update database + result = await self.entity_service.delete_entity(entity_id) + if result: + deleted = True + logger.debug(f"Deleted entity from database: {entity_id}") - async def delete_relations(self, relations: List[Dict[str, Any]]) -> None: - pass + return deleted + except Exception: + logger.exception("Failed to delete entities") + raise + + async def delete_observations(self, entity_id: str, contents: List[str]) -> bool: + """Delete specific observations from an entity.""" + logger.debug(f"Deleting observations from entity: {entity_id}") + try: + # First read the entity + entity = await read_entity_file(self.entities_path, entity_id) + + # Remove observations from entity + original_count = len(entity.observations) + entity.observations = [ + obs for obs in entity.observations + if obs not in contents + ] + + # Only write file if we actually removed anything + if len(entity.observations) < original_count: + # Write updated entity file first (source of truth) + await write_entity_file(self.entities_path, entity_id, entity) + logger.debug(f"Updated entity file: {entity_id}") + + # Then update database + result = await self.observation_service.delete_observations(entity_id, contents) + logger.debug(f"Deleted observations from database: {result}") + return True + + return False + except Exception: + logger.exception("Failed to delete observations") + raise + + async def delete_relations(self, relations: List[Dict[str, Any]]) -> bool: + """Delete relations between entities.""" + logger.debug(f"Deleting relations: {relations}") + try: + deleted = False + for relation in relations: + # First read the source entity + try: + from_entity = await read_entity_file(self.entities_path, relation['from_id']) + except EntityNotFoundError: + logger.debug(f"Source entity not found: {relation['from_id']}") + continue + + # Update the entity's relations + if hasattr(from_entity, 'relations'): + original_count = len(from_entity.relations) + relation_type = relation.get('relation_type') + + if relation_type: + from_entity.relations = [ + r for r in from_entity.relations + if not (r.to_id == relation['to_id'] and r.relation_type == relation_type) + ] + else: + from_entity.relations = [ + r for r in from_entity.relations + if r.to_id != relation['to_id'] + ] + + # Only write file if we actually removed any relations + if len(from_entity.relations) < original_count: + # Write updated entity file first (source of truth) + await write_entity_file(self.entities_path, from_entity.id, from_entity) + logger.debug(f"Updated source entity file: {from_entity.id}") + + # Then update database + result = await self.relation_service.delete_relations([relation]) + if result: + deleted = True + logger.debug("Deleted relations from database") + + return deleted + except Exception: + logger.exception("Failed to delete relations") + raise async def read_graph(self) -> Sequence[EntityModel]: """Read the entire knowledge graph.""" @@ -214,32 +314,31 @@ class MemoryService: logger.exception(f"Failed to search nodes with query: {query}") raise - async def open_nodes(self, names: List[str]) -> List[Entity]: + async def open_nodes(self, entity_ids: List[str]) -> List[Entity]: """Get specific nodes and their relationships.""" - logger.debug(f"Opening nodes: {names}") + logger.debug(f"Opening nodes entity_ids: {entity_ids}") - async def read_node(name: str) -> Optional[Entity]: + async def read_node(id: str) -> Optional[Entity]: try: # Get ID from name first - logger.debug(f"Looking up entity: {name}") - db_entity = await self.entity_service.get_entity(name) + logger.debug(f"Looking up entity: {id}") + db_entity = await self.entity_service.get_entity(id) if db_entity: logger.debug(f"Found entity in DB: {db_entity.id}") entity = await read_entity_file(self.entities_path, db_entity.id) logger.debug(f"Read entity from filesystem: {entity.id}") return entity - logger.debug(f"Entity not found: {name}") + logger.debug(f"Entity not found: {id}") return None except Exception: - logger.exception(f"Failed to read node: {name}") + logger.exception(f"Failed to read node: {id}") return None try: - entities = [entity for entity in await asyncio.gather(*(read_node(name) for name in names)) + entities = [entity for entity in await asyncio.gather(*(read_node(id) for id in entity_ids)) if entity is not None] logger.debug(f"Opened {len(entities)} entities") return entities except Exception: logger.exception("Failed to open nodes") - raise - + raise \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index e78e41d9..4324de88 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -121,27 +121,3 @@ async def test_entity(entity_service): ) return await entity_service.create_entity(entity_data) -# Test data fixtures -@pytest_asyncio.fixture -def test_entity_data(): - """Sample data for creating a test entity using camelCase (like MCP will).""" - return { - "entities": [{ - "name": "Test Entity", - "entityType": "test", - "description": "", # Empty string instead of None - "observations": [{"content": "This is a test observation"}] - }] - } - -@pytest_asyncio.fixture -def test_entity_snake_case(): - """Same test data but using snake_case to test schema flexibility.""" - return { - "entities": [{ - "name": "Test Entity", - "entity_type": "test", - "description": "", # Empty string instead of None - "observations": [{"content": "This is a test observation"}] - }] - } diff --git a/tests/test_entity_service.py b/tests/services/test_entity_service.py similarity index 100% rename from tests/test_entity_service.py rename to tests/services/test_entity_service.py diff --git a/tests/test_memory_service.py b/tests/services/test_memory_service.py similarity index 50% rename from tests/test_memory_service.py rename to tests/services/test_memory_service.py index 06442abd..e3556ddf 100644 --- a/tests/test_memory_service.py +++ b/tests/services/test_memory_service.py @@ -1,10 +1,13 @@ -"""Tests for the MemoryService class.""" +"""Tests for MemoryService delete operations.""" + import pytest from basic_memory.services import MemoryService from basic_memory.fileio import read_entity_file from basic_memory.schemas import CreateEntityRequest, CreateRelationsRequest, AddObservationsRequest, Relation -test_entities_data = [ +from basic_memory.models import Relation + +test_create_entity_input = [ { "name": "Test_Entity_1", "entity_type": "test", @@ -17,11 +20,12 @@ test_entities_data = [ } ] + @pytest.mark.asyncio async def test_create_entities(memory_service: MemoryService): """Should create multiple entities in parallel with their observations.""" - entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data}) + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) entities = await memory_service.create_entities(entity_input.entities) # Verify the SQLAlchemy models were created @@ -44,14 +48,20 @@ async def test_create_entities(memory_service: MemoryService): # Verify files were created (returns Pydantic Entity) file_entity1 = await read_entity_file(memory_service.entities_path, entities[0].id) file_entity2 = await read_entity_file(memory_service.entities_path, entities[1].id) - + assert file_entity1.name == "Test_Entity_1" assert file_entity2.name == "Test_Entity_2" + # Verify files are present + for entity in entities: + entity_file_path = memory_service.get_entity_file_path(entity.id) + assert entity_file_path.exists() + + @pytest.mark.asyncio async def test_add_observations(memory_service: MemoryService): """Should add observations to an existing entity.""" - entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data}) + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) entities = await memory_service.create_entities([entity_input.entities[0]]) entity = entities[0] @@ -72,19 +82,20 @@ async def test_add_observations(memory_service: MemoryService): assert len(added_observations) == 2 assert added_observations[0].content == "New observation 1" assert added_observations[0].context is None - assert added_observations[1].content == "New observation 2" + assert added_observations[1].content == "New observation 2" assert added_observations[1].context is None # Verify file was updated - returns Pydantic Entity updated_entity = await read_entity_file(memory_service.entities_path, entity.id) assert len(updated_entity.observations) == 4 # 2 original + 2 new - #assert updated_entity.observations[2] == "New observation 1" - #assert updated_entity.observations[3] == "New observation 2" + # assert updated_entity.observations[2] == "New observation 1" + # assert updated_entity.observations[3] == "New observation 2" # Verify database - returns SQLAlchemy Entity db_entity = await memory_service.entity_service.get_entity(entity.id) assert len(db_entity.observations) == 4 + @pytest.mark.asyncio async def test_add_observations_nonexistent_entity(memory_service: MemoryService): """Should raise an appropriate error when adding observations to a non-existent entity.""" @@ -92,15 +103,16 @@ async def test_add_observations_nonexistent_entity(memory_service: MemoryService "entity_id": "nonexistent-id", "observations": ["Test observation"] } - + with pytest.raises(Exception) as exc: # We might want to define a specific error type observation_input = AddObservationsRequest.model_validate(observations_data) await memory_service.add_observations(observation_input) + @pytest.mark.asyncio async def test_create_relations(memory_service: MemoryService): """Should create relations between entities and update both filesystem and database.""" - entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data}) + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) entities = await memory_service.create_entities(entity_input.entities) entity1, entity2 = entities @@ -179,11 +191,12 @@ async def test_create_relations(memory_service: MemoryService): assert len(db_entity2.outgoing_relations) == 1 assert len(db_entity2.incoming_relations) == 1 + @pytest.mark.asyncio async def test_create_relations_with_invalid_entity_id(memory_service: MemoryService): """Should raise an appropriate error when trying to create relations with non-existent entity IDs.""" # Create one entity - returns SQLAlchemy Entity - entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data}) + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) entities = await memory_service.create_entities([entity_input.entities[0]]) entity1 = entities[0] @@ -193,6 +206,192 @@ async def test_create_relations_with_invalid_entity_id(memory_service: MemorySer "to_id": "nonexistent-id", "relation_type": "connects_to" } - + with pytest.raises(Exception) as exc: # We might want to define a specific error type - await memory_service.create_relations([Relation.model_validate(bad_relation)]) \ No newline at end of file + await memory_service.create_relations([Relation.model_validate(bad_relation)]) + + +@pytest.mark.asyncio +async def test_delete_entities(memory_service: MemoryService): + """Test deleting an entity deletes file and database record.""" + # Write the entity files + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) + entities = await memory_service.create_entities(entity_input.entities) + + # Verify files are present + for entity in entities: + entity_file_path = memory_service.get_entity_file_path(entity.id) + assert entity_file_path.exists() + + # Delete the entities + result = await memory_service.delete_entities([entity.id for entity in entities]) + assert result is True + + # Verify files are gone + for entity in entities: + assert not memory_service.get_entity_file_path(entity.id).exists() + + # Verify database records are deleted + for entity in entities: + deleted = await memory_service.entity_service.entity_repo.find_by_id(entity.id) + assert deleted is None + +@pytest.mark.asyncio +async def test_delete_entity_cascades(memory_service): + """Test deleting an entity cascades to observations.""" + + # Write the entity files + create_entity_input = test_create_entity_input[0] + entity_input = CreateEntityRequest.model_validate({"entities": [create_entity_input]}) + entities = await memory_service.create_entities(entity_input.entities) + assert len(entities) == 1 + test_entity = entities[0] + + # Delete the entity + result = await memory_service.delete_entities([test_entity.id]) + assert result is True + + # Verify file is gone + assert not memory_service.get_entity_file_path(test_entity.id).exists() + + # Verify observations are gone from database + obsv = await memory_service.observation_service.observation_repo.find_by_entity(test_entity.id) + assert len(obsv) == 0 + + # Verify observations are gone from database + rels = await memory_service.relation_service.relation_repo.find_by_entity(test_entity.id) + assert len(rels) == 0 + +@pytest.mark.asyncio +async def test_delete_observations(memory_service): + """Test deleting specific observations.""" + + # Set up entity file with observations + create_entity_input = test_create_entity_input[0] + create_entity_input['observations'] = ["First observation", "Second observation", "Third observation"] + create_entity = CreateEntityRequest.model_validate({"entities": [create_entity_input]}) + + entities = await memory_service.create_entities(create_entity.entities) + assert len(entities) == 1 + test_entity = entities[0] + + # Delete two observations + to_delete = ["First observation", "Second observation"] + result = await memory_service.delete_observations(test_entity.id, to_delete) + assert result is True + + # Verify file updated + updated_entities = await memory_service.open_nodes([test_entity.id]) + assert len(updated_entities) == 1 + updated_entity = updated_entities[0] + + assert len(updated_entity.observations) == 1 + assert updated_entity.observations[0] == "Third observation" + + # Verify database updated + remaining_db = await memory_service.observation_service.observation_repo.find_by_entity(test_entity.id) + assert len(remaining_db) == 1 + assert remaining_db[0].content == "Third observation" + +@pytest.mark.asyncio +async def test_delete_relations(memory_service): + """Test deleting relations between entities.""" + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) + entities = await memory_service.create_entities(entity_input.entities) + entity1, entity2 = entities + + # Create test relations data using actual entity IDs + test_relations_data = [ + { + "from_id": entity1.id, + "to_id": entity2.id, + "relation_type": "connects_to" + }, + { + "from_id": entity2.id, + "to_id": entity1.id, + "relation_type": "references", + "context": "test context" + } + ] + + # Create relations - returns List[models.Relation] + input_args = CreateRelationsRequest.model_validate({"relations": test_relations_data}) + relations = await memory_service.create_relations(input_args.relations) + + # Delete the relation + to_delete = [{ + "from_id": entity1.id, + "to_id": entity2.id, + "relation_type": "connects_to" + }] + result = await memory_service.delete_relations(to_delete) + assert result is True + + # Verify relation removed from source entity file + # Verify file updated + updated_entities = await memory_service.open_nodes([entity1.id]) + assert len(updated_entities) == 1 + updated_entity = updated_entities[0] + + assert len(updated_entity.relations) == 0 + + # Verify relation removed from database + relations = await memory_service.relation_service.relation_repo.find_by_entities(entity1.id, entity2.id) + assert len(relations) == 0 + +@pytest.mark.asyncio +async def test_delete_nonexistent_entity(memory_service): + """Test deleting an entity that doesn't exist.""" + result = await memory_service.delete_entities(["nonexistent/id"]) + assert result is False + +@pytest.mark.asyncio +async def test_delete_nonexistent_observations(memory_service): + """Test deleting observations that don't exist.""" + + # Set up entity file with observations + create_entity_input = test_create_entity_input[0] + create_entity_input['observations'] = ["First observation", "Second observation", "Third observation"] + create_entity = CreateEntityRequest.model_validate({"entities": [create_entity_input]}) + + entities = await memory_service.create_entities(create_entity.entities) + assert len(entities) == 1 + test_entity = entities[0] + + result = await memory_service.delete_observations(test_entity.id, ["Nonexistent"]) + assert result is False + +@pytest.mark.asyncio +async def test_delete_nonexistent_relations(memory_service): + """Test deleting relations that don't exist.""" + entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input}) + entities = await memory_service.create_entities(entity_input.entities) + entity1, entity2 = entities + + # Create test relations data using actual entity IDs + test_relations_data = [ + { + "from_id": entity1.id, + "to_id": entity2.id, + "relation_type": "connects_to" + }, + { + "from_id": entity2.id, + "to_id": entity1.id, + "relation_type": "references", + "context": "test context" + } + ] + + # Create relations - returns List[models.Relation] + input_args = CreateRelationsRequest.model_validate({"relations": test_relations_data}) + relations = await memory_service.create_relations(input_args.relations) + + to_delete = [{ + "from_id": entity1.id, + "to_id": entity2.id, + "relation_type": "nonexistent" + }] + result = await memory_service.delete_relations(to_delete) + assert result is False \ No newline at end of file diff --git a/tests/services/test_observation_service.py b/tests/services/test_observation_service.py index 1adbfa4d..21e99057 100644 --- a/tests/services/test_observation_service.py +++ b/tests/services/test_observation_service.py @@ -6,7 +6,6 @@ from pathlib import Path from basic_memory.models import Entity, Observation from basic_memory.repository.observation_repository import ObservationRepository from basic_memory.services.observation_service import ObservationService -from basic_memory.services import DatabaseSyncError @pytest_asyncio.fixture @@ -30,6 +29,60 @@ async def test_entity(session): return entity +@pytest_asyncio.fixture +async def test_add_observation_success(observation_service, test_entity): + """Test successful observation addition.""" + + # Act + observations = await observation_service.add_observations(test_entity.id, ["New observation"]) + + # Assert + assert len(observations) == 1 + assert isinstance(observations[0], Observation) + assert observations[0].content == "New observation" + + # Verify database index + db_observations = await observation_service.observation_repo.find_by_entity(test_entity.id) + assert len(db_observations) == 1 + assert any(obs.content == "New observation" + for obs in db_observations) + + +@pytest_asyncio.fixture +async def test_search_observations(observation_service, test_entity): + """Test searching observations across entities.""" + # Arrange + await observation_service.add_observations( + test_entity.id, + ["Unique test content", "Other content"] + ) + + # Act + results = await observation_service.search_observations("unique") + + # Assert + assert len(results) == 1 + assert results[0].content == "Unique test content" + + +@pytest_asyncio.fixture +async def test_observation_with_special_characters(observation_service, test_entity): + """Test handling observations with special characters.""" + content = "Test & observation with @#$% special chars!" + + observations = await observation_service.add_observations(test_entity.id, [content]) + assert observations[0].content == content + + +@pytest_asyncio.fixture +async def test_very_long_observation(observation_service, test_entity): + """Test handling very long observation content.""" + long_content = "Very long observation " * 100 # ~1800 characters + + observations = await observation_service.add_observations(test_entity.id, [long_content]) + assert observations[0].content == long_content + + @pytest_asyncio.fixture async def test_observations(session, test_entity): """Create test observations.""" @@ -100,15 +153,6 @@ async def test_delete_observations_invalid_entity(observation_service): assert result is False -@pytest.mark.asyncio -async def test_search_observations(observation_service, test_observations): - """Test searching observations.""" - results = await observation_service.search_observations("First") - - assert len(results) == 1 - assert results[0].content == "First observation" - - @pytest.mark.asyncio async def test_get_observations_by_context(observation_service, session, test_entity): """Test getting observations by context.""" diff --git a/tests/test_relation_service.py b/tests/services/test_relation_service.py similarity index 100% rename from tests/test_relation_service.py rename to tests/services/test_relation_service.py diff --git a/tests/test_observation_service.py b/tests/test_observation_service.py deleted file mode 100644 index bfe79bb0..00000000 --- a/tests/test_observation_service.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Tests for ObservationService.""" -import pytest - -from basic_memory.models import Observation - -pytestmark = pytest.mark.asyncio - - -async def test_add_observation_success(observation_service, test_entity): - """Test successful observation addition.""" - - # Act - observations = await observation_service.add_observations(test_entity.id, ["New observation"]) - - # Assert - assert len(observations) == 1 - assert isinstance(observations[0], Observation) - assert observations[0].content == "New observation" - - # Verify database index - db_observations = await observation_service.observation_repo.find_by_entity(test_entity.id) - assert len(db_observations) == 1 - assert any(obs.content == "New observation" - for obs in db_observations) - - - -async def test_search_observations(observation_service, test_entity): - """Test searching observations across entities.""" - # Arrange - await observation_service.add_observations( - test_entity.id, - ["Unique test content", "Other content"] - ) - - # Act - results = await observation_service.search_observations("unique") - - # Assert - assert len(results) == 1 - assert results[0].content == "Unique test content" - - - - -# Edge Cases - -async def test_observation_with_special_characters(observation_service, test_entity): - """Test handling observations with special characters.""" - content = "Test & observation with @#$% special chars!" - - observations = await observation_service.add_observations(test_entity.id, [content]) - assert observations[0].content == content - - -async def test_very_long_observation(observation_service, test_entity): - """Test handling very long observation content.""" - long_content = "Very long observation " * 100 # ~1800 characters - - observations = await observation_service.add_observations(test_entity.id, [long_content]) - assert observations[0].content == long_content - \ No newline at end of file