diff --git a/src/basic_memory/repository/search_repository.py b/src/basic_memory/repository/search_repository.py index e58085b6..67dae08f 100644 --- a/src/basic_memory/repository/search_repository.py +++ b/src/basic_memory/repository/search_repository.py @@ -237,19 +237,24 @@ class SearchRepository: # Handle text search for title and content if search_text: - # Check for explicit boolean operators - only detect them in proper boolean contexts - has_boolean = any(op in f" {search_text} " for op in [" AND ", " OR ", " NOT "]) - - if has_boolean: - # If boolean operators are present, use the raw query - # No need to prepare it, FTS5 will understand the operators - params["text"] = search_text - conditions.append("(title MATCH :text OR content_stems MATCH :text)") + # Skip FTS for wildcard-only queries that would cause "unknown special query" errors + if search_text.strip() == "*" or search_text.strip() == "": + # For wildcard searches, don't add any text conditions - return all results + pass else: - # Standard search with term preparation - processed_text = self._prepare_search_term(search_text.strip()) - params["text"] = processed_text - conditions.append("(title MATCH :text OR content_stems MATCH :text)") + # Check for explicit boolean operators - only detect them in proper boolean contexts + has_boolean = any(op in f" {search_text} " for op in [" AND ", " OR ", " NOT "]) + + if has_boolean: + # If boolean operators are present, use the raw query + # No need to prepare it, FTS5 will understand the operators + params["text"] = search_text + conditions.append("(title MATCH :text OR content_stems MATCH :text)") + else: + # Standard search with term preparation + processed_text = self._prepare_search_term(search_text.strip()) + params["text"] = processed_text + conditions.append("(title MATCH :text OR content_stems MATCH :text)") # Handle title match search if title: @@ -453,4 +458,4 @@ class SearchRepository: end_time = time.perf_counter() elapsed_time = end_time - start_time logger.debug(f"Query executed successfully in {elapsed_time:.2f}s.") - return result + return result \ No newline at end of file diff --git a/src/basic_memory/services/entity_service.py b/src/basic_memory/services/entity_service.py index dd3ea7a9..d94dac2c 100644 --- a/src/basic_memory/services/entity_service.py +++ b/src/basic_memory/services/entity_service.py @@ -299,7 +299,16 @@ class EntityService(BaseService[EntityModel]): # Mark as incomplete because we still need to add relations model.checksum = None # Repository will set project_id automatically - return await self.repository.add(model) + try: + return await self.repository.add(model) + except IntegrityError as e: + # Handle race condition where entity was created by another process + if "UNIQUE constraint failed: entity.file_path" in str(e) or "UNIQUE constraint failed: entity.permalink" in str(e): + logger.info(f"Entity already exists for file_path={file_path} (file_path or permalink conflict), updating instead of creating") + return await self.update_entity_and_observations(file_path, markdown) + else: + # Re-raise if it's a different integrity error + raise async def update_entity_and_observations( self, file_path: Path, markdown: EntityMarkdown diff --git a/src/basic_memory/sync/sync_service.py b/src/basic_memory/sync/sync_service.py index 05d2a5d2..b66d2c25 100644 --- a/src/basic_memory/sync/sync_service.py +++ b/src/basic_memory/sync/sync_service.py @@ -364,18 +364,41 @@ class SyncService: content_type = self.file_service.content_type(path) file_path = Path(path) - entity = await self.entity_repository.add( - Entity( - entity_type="file", - file_path=path, - checksum=checksum, - title=file_path.name, - created_at=created, - updated_at=modified, - content_type=content_type, + try: + entity = await self.entity_repository.add( + Entity( + entity_type="file", + file_path=path, + checksum=checksum, + title=file_path.name, + created_at=created, + updated_at=modified, + content_type=content_type, + ) ) - ) - return entity, checksum + return entity, checksum + except IntegrityError as e: + # Handle race condition where entity was created by another process + if "UNIQUE constraint failed: entity.file_path" in str(e): + logger.info(f"Entity already exists for file_path={path}, updating instead of creating") + # Treat as update instead of create + entity = await self.entity_repository.get_by_file_path(path) + if entity is None: # pragma: no cover + logger.error(f"Entity not found after constraint violation, path={path}") + raise ValueError(f"Entity not found after constraint violation: {path}") + + updated = await self.entity_repository.update( + entity.id, {"file_path": path, "checksum": checksum} + ) + + if updated is None: # pragma: no cover + logger.error(f"Failed to update entity, entity_id={entity.id}, path={path}") + raise ValueError(f"Failed to update entity with ID {entity.id}") + + return updated, checksum + else: + # Re-raise if it's a different integrity error + raise else: entity = await self.entity_repository.get_by_file_path(path) if entity is None: # pragma: no cover diff --git a/tests/repository/test_search_repository.py b/tests/repository/test_search_repository.py index 30712155..8261a52a 100644 --- a/tests/repository/test_search_repository.py +++ b/tests/repository/test_search_repository.py @@ -483,3 +483,37 @@ class TestSearchTermPreparation: # Test with other problematic patterns results3 = await search_repository.search(search_text="node.js version") assert isinstance(results3, list) # Should not crash + + @pytest.mark.asyncio + async def test_wildcard_only_search(self, search_repository, search_entity): + """Test that wildcard-only search '*' doesn't cause FTS5 errors (line 243 coverage).""" + # Index an entity for testing + search_row = SearchIndexRow( + id=search_entity.id, + type=SearchItemType.ENTITY.value, + title="Test Entity", + content_stems="test entity content", + content_snippet="This is a test entity", + permalink=search_entity.permalink, + file_path=search_entity.file_path, + entity_id=search_entity.id, + metadata={"entity_type": search_entity.entity_type}, + created_at=search_entity.created_at, + updated_at=search_entity.updated_at, + project_id=search_repository.project_id, + ) + + await search_repository.index_item(search_row) + + # Test wildcard-only search - should not crash and should return results + results = await search_repository.search(search_text="*") + assert isinstance(results, list) # Should not crash + assert len(results) >= 1 # Should return all results, including our test entity + + # Test empty string search - should also not crash + results_empty = await search_repository.search(search_text="") + assert isinstance(results_empty, list) # Should not crash + + # Test whitespace-only search + results_whitespace = await search_repository.search(search_text=" ") + assert isinstance(results_whitespace, list) # Should not crash diff --git a/tests/services/test_entity_service.py b/tests/services/test_entity_service.py index 8c88642b..e8b02488 100644 --- a/tests/services/test_entity_service.py +++ b/tests/services/test_entity_service.py @@ -869,6 +869,109 @@ async def test_edit_entity_with_observations_and_relations( assert new_rel.relation_type == "relates to" +@pytest.mark.asyncio +async def test_create_entity_from_markdown_race_condition_handling( + entity_service: EntityService, file_service: FileService +): + """Test that create_entity_from_markdown handles race condition with IntegrityError (lines 304-311).""" + from unittest.mock import patch, AsyncMock + from sqlalchemy.exc import IntegrityError + + file_path = Path("test/race-condition.md") + + # Create a mock EntityMarkdown object + from basic_memory.markdown.schemas import EntityFrontmatter, EntityMarkdown as RealEntityMarkdown + from datetime import datetime, timezone + + frontmatter = EntityFrontmatter(metadata={"title": "Race Condition Test", "type": "test"}) + markdown = RealEntityMarkdown( + frontmatter=frontmatter, + observations=[], + relations=[], + created=datetime.now(timezone.utc), + modified=datetime.now(timezone.utc) + ) + + # Mock the repository.add to raise IntegrityError on first call, then succeed on second + original_add = entity_service.repository.add + original_update = entity_service.update_entity_and_observations + + call_count = 0 + + async def mock_add(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + # Simulate race condition - another process created the entity + raise IntegrityError("UNIQUE constraint failed: entity.file_path", None, None) + else: + return await original_add(*args, **kwargs) + + # Mock update method to return a dummy entity + async def mock_update(*args, **kwargs): + from basic_memory.models import Entity + from datetime import datetime, timezone + + return Entity( + id=1, + title="Race Condition Test", + entity_type="test", + file_path=str(file_path), + permalink="test/race-condition-test", + content_type="text/markdown", + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + with patch.object(entity_service.repository, 'add', side_effect=mock_add), \ + patch.object(entity_service, 'update_entity_and_observations', side_effect=mock_update) as mock_update_call: + + # Call the method + result = await entity_service.create_entity_from_markdown(file_path, markdown) + + # Verify it handled the race condition gracefully + assert result is not None + assert result.title == "Race Condition Test" + assert result.file_path == str(file_path) + + # Verify that update_entity_and_observations was called as fallback + mock_update_call.assert_called_once_with(file_path, markdown) + + +@pytest.mark.asyncio +async def test_create_entity_from_markdown_integrity_error_reraise( + entity_service: EntityService, file_service: FileService +): + """Test that create_entity_from_markdown re-raises IntegrityError for non-race-condition cases.""" + from unittest.mock import patch + from sqlalchemy.exc import IntegrityError + + file_path = Path("test/integrity-error.md") + + # Create a mock EntityMarkdown object + from basic_memory.markdown.schemas import EntityFrontmatter, EntityMarkdown as RealEntityMarkdown + from datetime import datetime, timezone + + frontmatter = EntityFrontmatter(metadata={"title": "Integrity Error Test", "type": "test"}) + markdown = RealEntityMarkdown( + frontmatter=frontmatter, + observations=[], + relations=[], + created=datetime.now(timezone.utc), + modified=datetime.now(timezone.utc) + ) + + # Mock the repository.add to raise a different IntegrityError (not file_path/permalink constraint) + async def mock_add(*args, **kwargs): + # Simulate a different constraint violation + raise IntegrityError("UNIQUE constraint failed: entity.some_other_field", None, None) + + with patch.object(entity_service.repository, 'add', side_effect=mock_add): + # Should re-raise the IntegrityError since it's not a file_path/permalink constraint + with pytest.raises(IntegrityError, match="UNIQUE constraint failed: entity.some_other_field"): + await entity_service.create_entity_from_markdown(file_path, markdown) + + # Edge case tests for find_replace operation @pytest.mark.asyncio async def test_edit_entity_find_replace_not_found(entity_service: EntityService): diff --git a/tests/sync/test_sync_service.py b/tests/sync/test_sync_service.py index a2c7bc71..e3c38249 100644 --- a/tests/sync/test_sync_service.py +++ b/tests/sync/test_sync_service.py @@ -1089,3 +1089,204 @@ permalink: note """.strip() == file_one_content ) + + +@pytest.mark.asyncio +async def test_sync_regular_file_race_condition_handling( + sync_service: SyncService, project_config: ProjectConfig +): + """Test that sync_regular_file handles race condition with IntegrityError (lines 380-401).""" + from unittest.mock import patch, AsyncMock + from sqlalchemy.exc import IntegrityError + from pathlib import Path + from datetime import datetime, timezone + + # Create a test file + test_file = project_config.home / "test_race.md" + test_content = """ +--- +type: knowledge +--- +# Test Race Condition +This is a test file for race condition handling. +""" + await create_test_file(test_file, test_content) + + # Mock the entity_repository.add to raise IntegrityError on first call + original_add = sync_service.entity_repository.add + original_get_by_file_path = sync_service.entity_repository.get_by_file_path + original_update = sync_service.entity_repository.update + + call_count = 0 + + async def mock_add(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + # Simulate race condition - another process created the entity + raise IntegrityError("UNIQUE constraint failed: entity.file_path", None, None) + else: + return await original_add(*args, **kwargs) + + # Mock get_by_file_path to return an existing entity (simulating the race condition result) + async def mock_get_by_file_path(file_path): + from basic_memory.models import Entity + return Entity( + id=1, + title="Test Race Condition", + entity_type="knowledge", + file_path=str(file_path), + permalink="test-race-condition", + content_type="text/markdown", + checksum="old_checksum", + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + # Mock update to return the updated entity + async def mock_update(entity_id, updates): + from basic_memory.models import Entity + return Entity( + id=entity_id, + title="Test Race Condition", + entity_type="knowledge", + file_path=updates["file_path"], + permalink="test-race-condition", + content_type="text/markdown", + checksum=updates["checksum"], + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + with patch.object(sync_service.entity_repository, 'add', side_effect=mock_add), \ + patch.object(sync_service.entity_repository, 'get_by_file_path', side_effect=mock_get_by_file_path) as mock_get, \ + patch.object(sync_service.entity_repository, 'update', side_effect=mock_update) as mock_update_call: + + # Call sync_regular_file + entity, checksum = await sync_service.sync_regular_file(str(test_file.relative_to(project_config.home)), new=True) + + # Verify it handled the race condition gracefully + assert entity is not None + assert entity.title == "Test Race Condition" + assert entity.file_path == str(test_file.relative_to(project_config.home)) + + # Verify that get_by_file_path and update were called as fallback + assert mock_get.call_count >= 1 # May be called multiple times + mock_update_call.assert_called_once() + + +@pytest.mark.asyncio +async def test_sync_regular_file_integrity_error_reraise( + sync_service: SyncService, project_config: ProjectConfig +): + """Test that sync_regular_file re-raises IntegrityError for non-race-condition cases.""" + from unittest.mock import patch + from sqlalchemy.exc import IntegrityError + + # Create a test file + test_file = project_config.home / "test_integrity.md" + test_content = """ +--- +type: knowledge +--- +# Test Integrity Error +This is a test file for integrity error handling. +""" + await create_test_file(test_file, test_content) + + # Mock the entity_repository.add to raise a different IntegrityError (not file_path constraint) + async def mock_add(*args, **kwargs): + # Simulate a different constraint violation + raise IntegrityError("UNIQUE constraint failed: entity.some_other_field", None, None) + + with patch.object(sync_service.entity_repository, 'add', side_effect=mock_add): + # Should re-raise the IntegrityError since it's not a file_path constraint + with pytest.raises(IntegrityError, match="UNIQUE constraint failed: entity.some_other_field"): + await sync_service.sync_regular_file(str(test_file.relative_to(project_config.home)), new=True) + + +@pytest.mark.asyncio +async def test_sync_regular_file_race_condition_entity_not_found( + sync_service: SyncService, project_config: ProjectConfig +): + """Test handling when entity is not found after IntegrityError (pragma: no cover case).""" + from unittest.mock import patch + from sqlalchemy.exc import IntegrityError + + # Create a test file + test_file = project_config.home / "test_not_found.md" + test_content = """ +--- +type: knowledge +--- +# Test Not Found +This is a test file for entity not found after constraint violation. +""" + await create_test_file(test_file, test_content) + + # Mock the entity_repository.add to raise IntegrityError + async def mock_add(*args, **kwargs): + raise IntegrityError("UNIQUE constraint failed: entity.file_path", None, None) + + # Mock get_by_file_path to return None (entity not found) + async def mock_get_by_file_path(file_path): + return None + + with patch.object(sync_service.entity_repository, 'add', side_effect=mock_add), \ + patch.object(sync_service.entity_repository, 'get_by_file_path', side_effect=mock_get_by_file_path): + + # Should raise ValueError when entity is not found after constraint violation + with pytest.raises(ValueError, match="Entity not found after constraint violation"): + await sync_service.sync_regular_file(str(test_file.relative_to(project_config.home)), new=True) + + +@pytest.mark.asyncio +async def test_sync_regular_file_race_condition_update_failed( + sync_service: SyncService, project_config: ProjectConfig +): + """Test handling when update fails after IntegrityError (pragma: no cover case).""" + from unittest.mock import patch + from sqlalchemy.exc import IntegrityError + from datetime import datetime, timezone + + # Create a test file + test_file = project_config.home / "test_update_fail.md" + test_content = """ +--- +type: knowledge +--- +# Test Update Fail +This is a test file for update failure after constraint violation. +""" + await create_test_file(test_file, test_content) + + # Mock the entity_repository.add to raise IntegrityError + async def mock_add(*args, **kwargs): + raise IntegrityError("UNIQUE constraint failed: entity.file_path", None, None) + + # Mock get_by_file_path to return an existing entity + async def mock_get_by_file_path(file_path): + from basic_memory.models import Entity + return Entity( + id=1, + title="Test Update Fail", + entity_type="knowledge", + file_path=str(file_path), + permalink="test-update-fail", + content_type="text/markdown", + checksum="old_checksum", + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + # Mock update to return None (failure) + async def mock_update(entity_id, updates): + return None + + with patch.object(sync_service.entity_repository, 'add', side_effect=mock_add), \ + patch.object(sync_service.entity_repository, 'get_by_file_path', side_effect=mock_get_by_file_path), \ + patch.object(sync_service.entity_repository, 'update', side_effect=mock_update): + + # Should raise ValueError when update fails + with pytest.raises(ValueError, match="Failed to update entity with ID"): + await sync_service.sync_regular_file(str(test_file.relative_to(project_config.home)), new=True)