mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: enhance character conflict detection and error handling for sync operations (#201)
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
This commit is contained in:
@@ -15,6 +15,7 @@ from basic_memory.markdown.entity_parser import EntityParser
|
||||
from basic_memory.markdown.utils import entity_model_from_markdown, schema_to_markdown
|
||||
from basic_memory.models import Entity as EntityModel
|
||||
from basic_memory.models import Observation, Relation
|
||||
from basic_memory.models.knowledge import Entity
|
||||
from basic_memory.repository import ObservationRepository, RelationRepository
|
||||
from basic_memory.repository.entity_repository import EntityRepository
|
||||
from basic_memory.schemas import Entity as EntitySchema
|
||||
@@ -44,6 +45,39 @@ class EntityService(BaseService[EntityModel]):
|
||||
self.file_service = file_service
|
||||
self.link_resolver = link_resolver
|
||||
|
||||
async def detect_file_path_conflicts(self, file_path: str) -> List[Entity]:
|
||||
"""Detect potential file path conflicts for a given file path.
|
||||
|
||||
This checks for entities with similar file paths that might cause conflicts:
|
||||
- Case sensitivity differences (Finance/file.md vs finance/file.md)
|
||||
- Character encoding differences
|
||||
- Hyphen vs space differences
|
||||
- Unicode normalization differences
|
||||
|
||||
Args:
|
||||
file_path: The file path to check for conflicts
|
||||
|
||||
Returns:
|
||||
List of entities that might conflict with the given file path
|
||||
"""
|
||||
from basic_memory.utils import detect_potential_file_conflicts
|
||||
|
||||
conflicts = []
|
||||
|
||||
# Get all existing file paths
|
||||
all_entities = await self.repository.find_all()
|
||||
existing_paths = [entity.file_path for entity in all_entities]
|
||||
|
||||
# Use the enhanced conflict detection utility
|
||||
conflicting_paths = detect_potential_file_conflicts(file_path, existing_paths)
|
||||
|
||||
# Find the entities corresponding to conflicting paths
|
||||
for entity in all_entities:
|
||||
if entity.file_path in conflicting_paths:
|
||||
conflicts.append(entity)
|
||||
|
||||
return conflicts
|
||||
|
||||
async def resolve_permalink(
|
||||
self, file_path: Permalink | Path, markdown: Optional[EntityMarkdown] = None
|
||||
) -> str:
|
||||
@@ -54,18 +88,30 @@ class EntityService(BaseService[EntityModel]):
|
||||
2. If markdown has permalink but it's used by another file -> make unique
|
||||
3. For existing files, keep current permalink from db
|
||||
4. Generate new unique permalink from file path
|
||||
|
||||
Enhanced to detect and handle character-related conflicts.
|
||||
"""
|
||||
file_path_str = str(file_path)
|
||||
|
||||
# Check for potential file path conflicts before resolving permalink
|
||||
conflicts = await self.detect_file_path_conflicts(file_path_str)
|
||||
if conflicts:
|
||||
logger.warning(
|
||||
f"Detected potential file path conflicts for '{file_path_str}': "
|
||||
f"{[entity.file_path for entity in conflicts]}"
|
||||
)
|
||||
|
||||
# If markdown has explicit permalink, try to validate it
|
||||
if markdown and markdown.frontmatter.permalink:
|
||||
desired_permalink = markdown.frontmatter.permalink
|
||||
existing = await self.repository.get_by_permalink(desired_permalink)
|
||||
|
||||
# If no conflict or it's our own file, use as is
|
||||
if not existing or existing.file_path == str(file_path):
|
||||
if not existing or existing.file_path == file_path_str:
|
||||
return desired_permalink
|
||||
|
||||
# For existing files, try to find current permalink
|
||||
existing = await self.repository.get_by_file_path(str(file_path))
|
||||
existing = await self.repository.get_by_file_path(file_path_str)
|
||||
if existing:
|
||||
return existing.permalink
|
||||
|
||||
@@ -75,7 +121,7 @@ class EntityService(BaseService[EntityModel]):
|
||||
else:
|
||||
desired_permalink = generate_permalink(file_path)
|
||||
|
||||
# Make unique if needed
|
||||
# Make unique if needed - enhanced to handle character conflicts
|
||||
permalink = desired_permalink
|
||||
suffix = 1
|
||||
while await self.repository.get_by_permalink(permalink):
|
||||
|
||||
@@ -453,6 +453,36 @@ class SyncService:
|
||||
|
||||
entity = await self.entity_repository.get_by_file_path(old_path)
|
||||
if entity:
|
||||
# Check if destination path is already occupied by another entity
|
||||
existing_at_destination = await self.entity_repository.get_by_file_path(new_path)
|
||||
if existing_at_destination and existing_at_destination.id != entity.id:
|
||||
# Handle the conflict - this could be a file swap or replacement scenario
|
||||
logger.warning(
|
||||
f"File path conflict detected during move: "
|
||||
f"entity_id={entity.id} trying to move from '{old_path}' to '{new_path}', "
|
||||
f"but entity_id={existing_at_destination.id} already occupies '{new_path}'"
|
||||
)
|
||||
|
||||
# Check if this is a file swap (the destination entity is being moved to our old path)
|
||||
# This would indicate a simultaneous move operation
|
||||
old_path_after_swap = await self.entity_repository.get_by_file_path(old_path)
|
||||
if old_path_after_swap and old_path_after_swap.id == existing_at_destination.id:
|
||||
logger.info(f"Detected file swap between '{old_path}' and '{new_path}'")
|
||||
# This is a swap scenario - both moves should succeed
|
||||
# We'll allow this to proceed since the other file has moved out
|
||||
else:
|
||||
# This is a conflict where the destination is occupied
|
||||
raise ValueError(
|
||||
f"Cannot move entity from '{old_path}' to '{new_path}': "
|
||||
f"destination path is already occupied by another file. "
|
||||
f"This may be caused by: "
|
||||
f"1. Conflicting file names with different character encodings, "
|
||||
f"2. Case sensitivity differences (e.g., 'Finance/' vs 'finance/'), "
|
||||
f"3. Character conflicts between hyphens in filenames and generated permalinks, "
|
||||
f"4. Files with similar names containing special characters. "
|
||||
f"Try renaming one of the conflicting files to resolve this issue."
|
||||
)
|
||||
|
||||
# Update file_path in all cases
|
||||
updates = {"file_path": new_path}
|
||||
|
||||
@@ -477,7 +507,26 @@ class SyncService:
|
||||
f"new_checksum={new_checksum}"
|
||||
)
|
||||
|
||||
updated = await self.entity_repository.update(entity.id, updates)
|
||||
try:
|
||||
updated = await self.entity_repository.update(entity.id, updates)
|
||||
except Exception as e:
|
||||
# Catch any database integrity errors and provide helpful context
|
||||
if "UNIQUE constraint failed" in str(e):
|
||||
logger.error(
|
||||
f"Database constraint violation during move: "
|
||||
f"entity_id={entity.id}, old_path='{old_path}', new_path='{new_path}'"
|
||||
)
|
||||
raise ValueError(
|
||||
f"Cannot complete move from '{old_path}' to '{new_path}': "
|
||||
f"a database constraint was violated. This usually indicates "
|
||||
f"a file path or permalink conflict. Please check for: "
|
||||
f"1. Duplicate file names, "
|
||||
f"2. Case sensitivity issues (e.g., 'File.md' vs 'file.md'), "
|
||||
f"3. Character encoding conflicts in file names."
|
||||
) from e
|
||||
else:
|
||||
# Re-raise other exceptions as-is
|
||||
raise
|
||||
|
||||
if updated is None: # pragma: no cover
|
||||
logger.error(
|
||||
|
||||
@@ -181,6 +181,79 @@ def setup_logging(
|
||||
logging.getLogger(logger_name).setLevel(level)
|
||||
|
||||
|
||||
def normalize_file_path_for_comparison(file_path: str) -> str:
|
||||
"""Normalize a file path for conflict detection.
|
||||
|
||||
This function normalizes file paths to help detect potential conflicts:
|
||||
- Converts to lowercase for case-insensitive comparison
|
||||
- Normalizes Unicode characters
|
||||
- Handles path separators consistently
|
||||
|
||||
Args:
|
||||
file_path: The file path to normalize
|
||||
|
||||
Returns:
|
||||
Normalized file path for comparison purposes
|
||||
"""
|
||||
import unicodedata
|
||||
|
||||
# Convert to lowercase for case-insensitive comparison
|
||||
normalized = file_path.lower()
|
||||
|
||||
# Normalize Unicode characters (NFD normalization)
|
||||
normalized = unicodedata.normalize('NFD', normalized)
|
||||
|
||||
# Replace path separators with forward slashes
|
||||
normalized = normalized.replace('\\', '/')
|
||||
|
||||
# Remove multiple slashes
|
||||
normalized = re.sub(r'/+', '/', normalized)
|
||||
|
||||
return normalized
|
||||
|
||||
|
||||
def detect_potential_file_conflicts(file_path: str, existing_paths: List[str]) -> List[str]:
|
||||
"""Detect potential conflicts between a file path and existing paths.
|
||||
|
||||
This function checks for various types of conflicts:
|
||||
- Case sensitivity differences
|
||||
- Unicode normalization differences
|
||||
- Path separator differences
|
||||
- Permalink generation conflicts
|
||||
|
||||
Args:
|
||||
file_path: The file path to check
|
||||
existing_paths: List of existing file paths to check against
|
||||
|
||||
Returns:
|
||||
List of existing paths that might conflict with the given file path
|
||||
"""
|
||||
conflicts = []
|
||||
|
||||
# Normalize the input file path
|
||||
normalized_input = normalize_file_path_for_comparison(file_path)
|
||||
input_permalink = generate_permalink(file_path)
|
||||
|
||||
for existing_path in existing_paths:
|
||||
# Skip identical paths
|
||||
if existing_path == file_path:
|
||||
continue
|
||||
|
||||
# Check for case-insensitive path conflicts
|
||||
normalized_existing = normalize_file_path_for_comparison(existing_path)
|
||||
if normalized_input == normalized_existing:
|
||||
conflicts.append(existing_path)
|
||||
continue
|
||||
|
||||
# Check for permalink conflicts
|
||||
existing_permalink = generate_permalink(existing_path)
|
||||
if input_permalink == existing_permalink:
|
||||
conflicts.append(existing_path)
|
||||
continue
|
||||
|
||||
return conflicts
|
||||
|
||||
|
||||
def parse_tags(tags: Union[List[str], str, None]) -> List[str]:
|
||||
"""Parse tags from various input formats into a consistent list.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user