Skip to content

Commit 2a3adc1

Browse files
groksrcclaude[bot]
andauthored
fix: scope entity queries by project_id in upsert_entity method (#168)
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
1 parent a52ce1c commit 2a3adc1

2 files changed

Lines changed: 209 additions & 5 deletions

File tree

src/basic_memory/repository/entity_repository.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ async def upsert_entity(self, entity: Entity) -> Entity:
142142
await session.flush()
143143
# Return with relationships loaded
144144
query = (
145-
select(Entity)
145+
self.select()
146146
.where(Entity.file_path == entity.file_path)
147147
.options(*self.get_load_options())
148148
)
@@ -162,7 +162,7 @@ async def upsert_entity(self, entity: Entity) -> Entity:
162162

163163
# Return with relationships loaded
164164
query = (
165-
select(Entity)
165+
self.select()
166166
.where(Entity.file_path == entity.file_path)
167167
.options(*self.get_load_options())
168168
)
@@ -203,7 +203,7 @@ async def upsert_entity(self, entity: Entity) -> Entity:
203203
await session.flush()
204204
# Return the updated entity with relationships loaded
205205
query = (
206-
select(Entity)
206+
self.select()
207207
.where(Entity.file_path == entity.file_path)
208208
.options(*self.get_load_options())
209209
)
@@ -243,7 +243,7 @@ async def _handle_permalink_conflict(self, entity: Entity, session: AsyncSession
243243

244244
# Return the inserted entity with relationships loaded
245245
query = (
246-
select(Entity)
246+
self.select()
247247
.where(Entity.file_path == entity.file_path)
248248
.options(*self.get_load_options())
249249
)

tests/repository/test_entity_repository_upsert.py

Lines changed: 205 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
from datetime import datetime, timezone
55

66
from basic_memory.models.knowledge import Entity
7+
from basic_memory.models.project import Project
78
from basic_memory.repository.entity_repository import EntityRepository
9+
from basic_memory.repository.project_repository import ProjectRepository
810

911

1012
@pytest.mark.asyncio
@@ -244,5 +246,207 @@ async def test_upsert_entity_gap_in_suffixes(entity_repository: EntityRepository
244246

245247
# Should get the next available suffix - our implementation finds gaps
246248
# so it should be "test/gap-2" (filling the gap)
247-
assert result.permalink == "test/gap-2"
249+
assert result.permalink == "test/gap-2"
248250
assert result.title == "Gap Filler"
251+
252+
253+
@pytest.mark.asyncio
254+
async def test_upsert_entity_project_scoping_isolation(session_maker):
255+
"""Test that upsert_entity properly scopes entities by project_id.
256+
257+
This test ensures that the fix for issue #167 works correctly by verifying:
258+
1. Entities with same permalinks/file_paths can exist in different projects
259+
2. Upsert operations properly scope queries by project_id
260+
3. No "multiple rows" errors occur when similar entities exist across projects
261+
"""
262+
# Create two separate projects
263+
project_repository = ProjectRepository(session_maker)
264+
265+
project1_data = {
266+
"name": "project-1",
267+
"description": "First test project",
268+
"path": "/tmp/project1",
269+
"is_active": True,
270+
"is_default": False,
271+
}
272+
project1 = await project_repository.create(project1_data)
273+
274+
project2_data = {
275+
"name": "project-2",
276+
"description": "Second test project",
277+
"path": "/tmp/project2",
278+
"is_active": True,
279+
"is_default": False,
280+
}
281+
project2 = await project_repository.create(project2_data)
282+
283+
# Create entity repositories for each project
284+
repo1 = EntityRepository(session_maker, project_id=project1.id)
285+
repo2 = EntityRepository(session_maker, project_id=project2.id)
286+
287+
# Create entities with identical permalinks and file_paths in different projects
288+
entity1 = Entity(
289+
project_id=project1.id,
290+
title="Shared Entity",
291+
entity_type="note",
292+
permalink="docs/shared-name",
293+
file_path="docs/shared-name.md",
294+
content_type="text/markdown",
295+
created_at=datetime.now(timezone.utc),
296+
updated_at=datetime.now(timezone.utc),
297+
)
298+
299+
entity2 = Entity(
300+
project_id=project2.id,
301+
title="Shared Entity",
302+
entity_type="note",
303+
permalink="docs/shared-name", # Same permalink
304+
file_path="docs/shared-name.md", # Same file_path
305+
content_type="text/markdown",
306+
created_at=datetime.now(timezone.utc),
307+
updated_at=datetime.now(timezone.utc),
308+
)
309+
310+
# These should succeed without "multiple rows" errors
311+
result1 = await repo1.upsert_entity(entity1)
312+
result2 = await repo2.upsert_entity(entity2)
313+
314+
# Verify both entities were created successfully
315+
assert result1.id is not None
316+
assert result2.id is not None
317+
assert result1.id != result2.id # Different entities
318+
assert result1.project_id == project1.id
319+
assert result2.project_id == project2.id
320+
assert result1.permalink == "docs/shared-name"
321+
assert result2.permalink == "docs/shared-name"
322+
323+
# Test updating entities in different projects (should also work without conflicts)
324+
entity1_update = Entity(
325+
project_id=project1.id,
326+
title="Updated Shared Entity",
327+
entity_type="note",
328+
permalink="docs/shared-name",
329+
file_path="docs/shared-name.md",
330+
content_type="text/markdown",
331+
created_at=datetime.now(timezone.utc),
332+
updated_at=datetime.now(timezone.utc),
333+
)
334+
335+
entity2_update = Entity(
336+
project_id=project2.id,
337+
title="Also Updated Shared Entity",
338+
entity_type="note",
339+
permalink="docs/shared-name",
340+
file_path="docs/shared-name.md",
341+
content_type="text/markdown",
342+
created_at=datetime.now(timezone.utc),
343+
updated_at=datetime.now(timezone.utc),
344+
)
345+
346+
# Updates should work without conflicts
347+
updated1 = await repo1.upsert_entity(entity1_update)
348+
updated2 = await repo2.upsert_entity(entity2_update)
349+
350+
# Should update existing entities (same IDs)
351+
assert updated1.id == result1.id
352+
assert updated2.id == result2.id
353+
assert updated1.title == "Updated Shared Entity"
354+
assert updated2.title == "Also Updated Shared Entity"
355+
356+
# Verify cross-project queries don't interfere
357+
found_in_project1 = await repo1.get_by_permalink("docs/shared-name")
358+
found_in_project2 = await repo2.get_by_permalink("docs/shared-name")
359+
360+
assert found_in_project1 is not None
361+
assert found_in_project2 is not None
362+
assert found_in_project1.id == updated1.id
363+
assert found_in_project2.id == updated2.id
364+
assert found_in_project1.title == "Updated Shared Entity"
365+
assert found_in_project2.title == "Also Updated Shared Entity"
366+
367+
368+
@pytest.mark.asyncio
369+
async def test_upsert_entity_permalink_conflict_within_project_only(session_maker):
370+
"""Test that permalink conflicts only occur within the same project.
371+
372+
This ensures that the project scoping fix allows entities with identical
373+
permalinks to exist across different projects without triggering
374+
permalink conflict resolution.
375+
"""
376+
# Create two separate projects
377+
project_repository = ProjectRepository(session_maker)
378+
379+
project1_data = {
380+
"name": "conflict-project-1",
381+
"description": "First conflict test project",
382+
"path": "/tmp/conflict1",
383+
"is_active": True,
384+
"is_default": False,
385+
}
386+
project1 = await project_repository.create(project1_data)
387+
388+
project2_data = {
389+
"name": "conflict-project-2",
390+
"description": "Second conflict test project",
391+
"path": "/tmp/conflict2",
392+
"is_active": True,
393+
"is_default": False,
394+
}
395+
project2 = await project_repository.create(project2_data)
396+
397+
# Create entity repositories for each project
398+
repo1 = EntityRepository(session_maker, project_id=project1.id)
399+
repo2 = EntityRepository(session_maker, project_id=project2.id)
400+
401+
# Create first entity in project1
402+
entity1 = Entity(
403+
project_id=project1.id,
404+
title="Original Entity",
405+
entity_type="note",
406+
permalink="test/conflict-permalink",
407+
file_path="test/original.md",
408+
content_type="text/markdown",
409+
created_at=datetime.now(timezone.utc),
410+
updated_at=datetime.now(timezone.utc),
411+
)
412+
413+
result1 = await repo1.upsert_entity(entity1)
414+
assert result1.permalink == "test/conflict-permalink"
415+
416+
# Create entity with same permalink in project2 (should NOT get suffix)
417+
entity2 = Entity(
418+
project_id=project2.id,
419+
title="Cross-Project Entity",
420+
entity_type="note",
421+
permalink="test/conflict-permalink", # Same permalink, different project
422+
file_path="test/cross-project.md",
423+
content_type="text/markdown",
424+
created_at=datetime.now(timezone.utc),
425+
updated_at=datetime.now(timezone.utc),
426+
)
427+
428+
result2 = await repo2.upsert_entity(entity2)
429+
# Should keep original permalink (no suffix) since it's in a different project
430+
assert result2.permalink == "test/conflict-permalink"
431+
432+
# Now create entity with same permalink in project1 (should get suffix)
433+
entity3 = Entity(
434+
project_id=project1.id,
435+
title="Conflict Entity",
436+
entity_type="note",
437+
permalink="test/conflict-permalink", # Same permalink, same project
438+
file_path="test/conflict.md",
439+
content_type="text/markdown",
440+
created_at=datetime.now(timezone.utc),
441+
updated_at=datetime.now(timezone.utc),
442+
)
443+
444+
result3 = await repo1.upsert_entity(entity3)
445+
# Should get suffix since it conflicts within the same project
446+
assert result3.permalink == "test/conflict-permalink-1"
447+
448+
# Verify all entities exist correctly
449+
assert result1.id != result2.id != result3.id
450+
assert result1.project_id == project1.id
451+
assert result2.project_id == project2.id
452+
assert result3.project_id == project1.id

0 commit comments

Comments
 (0)