-
Notifications
You must be signed in to change notification settings - Fork 716
fix(api): support delete_memory by user_id/conversation_id (Fixes #1103) #1104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
2bae2fd
8d84817
dcf8c18
725ca99
b43ac36
6bc1fd0
7cad766
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,108 @@ | ||||||||||||||||||||||
| from unittest.mock import Mock | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from memos.api.handlers.memory_handler import handle_delete_memories | ||||||||||||||||||||||
| from memos.api.product_models import DeleteMemoryRequest | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _build_naive_mem_cube() -> Mock: | ||||||||||||||||||||||
| naive_mem_cube = Mock() | ||||||||||||||||||||||
| naive_mem_cube.text_mem = Mock() | ||||||||||||||||||||||
| naive_mem_cube.pref_mem = Mock() | ||||||||||||||||||||||
| return naive_mem_cube | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def test_delete_memories_quick_by_user_id(): | ||||||||||||||||||||||
| naive_mem_cube = _build_naive_mem_cube() | ||||||||||||||||||||||
| req = DeleteMemoryRequest(user_id="u_1") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| resp = handle_delete_memories(req, naive_mem_cube) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| assert resp.data["status"] == "success" | ||||||||||||||||||||||
| naive_mem_cube.text_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| writable_cube_ids=None, | ||||||||||||||||||||||
| filter={"and": [{"user_id": "u_1"}]}, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| naive_mem_cube.pref_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| filter={"and": [{"user_id": "u_1"}]} | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
Comment on lines
+14
to
+27
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def test_delete_memories_quick_by_conversation_alias(): | ||||||||||||||||||||||
| naive_mem_cube = _build_naive_mem_cube() | ||||||||||||||||||||||
| req = DeleteMemoryRequest(conversation_id="conv_1") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| assert req.session_id == "conv_1" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| resp = handle_delete_memories(req, naive_mem_cube) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| assert resp.data["status"] == "success" | ||||||||||||||||||||||
| naive_mem_cube.text_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| writable_cube_ids=None, | ||||||||||||||||||||||
| filter={"and": [{"session_id": "conv_1"}]}, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| naive_mem_cube.pref_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| filter={"and": [{"session_id": "conv_1"}]} | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def test_delete_memories_filter_and_quick_conditions(): | ||||||||||||||||||||||
| naive_mem_cube = _build_naive_mem_cube() | ||||||||||||||||||||||
| req = DeleteMemoryRequest( | ||||||||||||||||||||||
| filter={"and": [{"memory_type": "WorkingMemory"}]}, | ||||||||||||||||||||||
| user_id="u_1", | ||||||||||||||||||||||
| session_id="s_1", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| resp = handle_delete_memories(req, naive_mem_cube) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| assert resp.data["status"] == "success" | ||||||||||||||||||||||
| naive_mem_cube.text_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| writable_cube_ids=None, | ||||||||||||||||||||||
| filter={ | ||||||||||||||||||||||
| "and": [ | ||||||||||||||||||||||
| {"memory_type": "WorkingMemory"}, | ||||||||||||||||||||||
| {"user_id": "u_1", "session_id": "s_1"}, | ||||||||||||||||||||||
| ] | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| naive_mem_cube.pref_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| filter={ | ||||||||||||||||||||||
| "and": [ | ||||||||||||||||||||||
| {"memory_type": "WorkingMemory"}, | ||||||||||||||||||||||
| {"user_id": "u_1", "session_id": "s_1"}, | ||||||||||||||||||||||
| ] | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def test_delete_memories_filter_or_with_distribution(): | ||||||||||||||||||||||
| naive_mem_cube = _build_naive_mem_cube() | ||||||||||||||||||||||
| req = DeleteMemoryRequest( | ||||||||||||||||||||||
| filter={"or": [{"memory_type": "WorkingMemory"}, {"memory_type": "UserMemory"}]}, | ||||||||||||||||||||||
| user_id="u_1", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| resp = handle_delete_memories(req, naive_mem_cube) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| assert resp.data["status"] == "success" | ||||||||||||||||||||||
| naive_mem_cube.text_mem.delete_by_filter.assert_called_once_with( | ||||||||||||||||||||||
| writable_cube_ids=None, | ||||||||||||||||||||||
| filter={ | ||||||||||||||||||||||
| "or": [ | ||||||||||||||||||||||
| {"memory_type": "WorkingMemory", "user_id": "u_1"}, | ||||||||||||||||||||||
| {"memory_type": "UserMemory", "user_id": "u_1"}, | ||||||||||||||||||||||
| ] | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
||||||||||||||||||||||
| ) | |
| ) | |
| naive_mem_cube.pref_mem.delete_by_filter.assert_called_once_with( | |
| filter={ | |
| "or": [ | |
| {"memory_type": "WorkingMemory", "user_id": "u_1"}, | |
| {"memory_type": "UserMemory", "user_id": "u_1"}, | |
| ] | |
| } | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation logic doesn't properly handle the edge case where filter is set to an empty dict. If a user sends
{"filter": {}}, the checkdelete_mem_req.filter is not Noneon line 480 will be True (an empty dict is not None), but when passed to the database layer, an empty filter dict would match ALL memories in the system, which is dangerous.Consider adding validation to reject empty filter dicts, or explicitly check for non-empty filters in the has_filter_mode calculation.