diff --git a/application/single_app/config.py b/application/single_app/config.py index d5f127d2a..40b5a360a 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -98,7 +98,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.052" +VERSION = "0.261.053" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_documents.py b/application/single_app/functions_documents.py index e4adf2677..4d16d2862 100644 --- a/application/single_app/functions_documents.py +++ b/application/single_app/functions_documents.py @@ -28,6 +28,11 @@ DataManagementSearchWritesFrozenError, hold_data_management_search_write_slot, ) +from utils_cache import ( + invalidate_group_search_cache, + invalidate_personal_search_cache, + invalidate_public_workspace_search_cache, +) from functions_visio import build_visio_page_markdown, parse_vsdx_pages from functions_content import * from functions_office_media import extract_office_embedded_images_with_diagnostics @@ -4828,6 +4833,44 @@ def _read_document_for_search_sync(record): return document_item if belongs_to_scope else None +def _approved_search_share_principals(entries): + """Return the users or groups whose approved share lets them find a document in search.""" + principals = [] + for entry in _normalize_search_acl_entries(entries): + principal, _separator, status = entry.partition(',') + if principal and status == 'approved': + principals.append(principal) + return principals + + +def _invalidate_document_search_results_cache(document_item, group_id=None, public_workspace_id=None): + """Drop cached search results that can include a document once a sync has changed its chunks. + + Routes clear the cache when a change is saved, but a search that ran while the sync was still + merging could otherwise keep serving the old values until its cache entry expires. Recipients of + approved shares cache their own results, so theirs are cleared too. Best effort. + """ + try: + if public_workspace_id is not None: + invalidate_public_workspace_search_cache(public_workspace_id) + elif group_id is not None: + invalidate_group_search_cache(group_id) + for shared_group_id in _approved_search_share_principals(document_item.get('shared_group_ids')): + invalidate_group_search_cache(shared_group_id) + else: + owner_user_id = document_item.get('user_id') + if owner_user_id: + invalidate_personal_search_cache(owner_user_id) + for shared_user_id in _approved_search_share_principals(document_item.get('shared_user_ids')): + invalidate_personal_search_cache(shared_user_id) + except Exception as exc: + log_event( + "[DOCUMENT_SEARCH_SYNC] Cached search results were not cleared after a search metadata sync.", + extra={'document_id': document_item.get('id'), 'error_type': type(exc).__name__}, + level=logging.WARNING, + ) + + def run_document_search_metadata_sync(record_id, lease_wait_seconds=0): """Project a document's pending metadata to all of its search chunks. @@ -4870,6 +4913,12 @@ def run_document_search_metadata_sync(record_id, lease_wait_seconds=0): group_id, public_workspace_id, ) + if chunks_updated: + _invalidate_document_search_results_cache( + document_item, + group_id=group_id, + public_workspace_id=public_workspace_id, + ) except DocumentSearchSyncLeaseLostError: log_event( "[DOCUMENT_SEARCH_SYNC] Search metadata sync stopped after another worker took over.", diff --git a/docs/explanation/fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md b/docs/explanation/fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md index bc338e6c9..932460dd7 100644 --- a/docs/explanation/fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md +++ b/docs/explanation/fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md @@ -1,6 +1,6 @@ # Document Search Metadata Sync Fix -Fixed/Implemented in version: **0.261.052** +Fixed/Implemented in version: **0.261.052** (search cache refresh after a sync added in **0.261.053**) Related issue: [#1657](https://github.com/microsoft/simplechat/issues/1657). Supersedes the request-batching approach in [Document Metadata Chunk Sync Batching Fix](DOCUMENT_METADATA_CHUNK_SYNC_BATCHING_FIX.md) (`0.261.051`, #1645). @@ -52,6 +52,7 @@ Two access-control problems shared the same code path: - A pass only completes the request when the document carries the latest request's token, so a save that has not landed yet is re-checked instead of being skipped. A token left by an earlier request never counts. - If an edit lands during a sync, the same worker projects it next. On success the record is deleted with an etag check. On failure the record keeps a retry time with backoff (1, 5, 15, and 60 minutes, then every 6 hours), the exception type is recorded, and `[DOCUMENT_SEARCH_SYNC]` is logged. Search writes frozen by a Data Management migration retry every 5 minutes without climbing the backoff. - The worker never writes sync state back onto the document. The document `_ts` (which drives list sorting and the access index) and its etag only change when users save. +- After a pass that changed chunks, the worker clears cached search results for the document's workspace and for every user or group with an approved share (added in `0.261.053`). Routes clear the cache when the change is saved, but a search that runs while the sync is still merging could otherwise cache the old values for the search cache lifetime. - Tag changes also refresh blob metadata tags from the worker. - `run_document_search_metadata_sync_loop()` in `background_tasks.py` runs every 60 seconds under a distributed lock. It picks up requests that failed, never started because the process restarted, or lost their lease. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index f7cbc60fa..a238db7e2 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,15 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.053)** + +#### Bug Fixes + +* **Search Results Refresh After a Background Metadata Sync** + * When the background search sync finishes merging a tag or metadata change, it now clears cached search results for the document's workspace and for every user or group with an approved share. + * Previously, a chat or search that ran while the sync was still in progress could cache the old values and keep returning them for up to the search cache lifetime (5 minutes by default) after the sync completed. + * (Ref: `run_document_search_metadata_sync()`, `utils_cache.py`, [Document Search Metadata Sync Fix](fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md), #1657) + ### **(v0.261.052)** #### Bug Fixes diff --git a/functional_tests/test_document_search_metadata_sync.py b/functional_tests/test_document_search_metadata_sync.py index 7e8139849..3bd681fa0 100644 --- a/functional_tests/test_document_search_metadata_sync.py +++ b/functional_tests/test_document_search_metadata_sync.py @@ -2,8 +2,9 @@ # test_document_search_metadata_sync.py """ Functional test for durable, batched document search metadata sync. -Version: 0.261.052 +Version: 0.261.053 Implemented in: 0.261.052 +Updated in: 0.261.053 This test ensures that document metadata edits no longer do per-chunk Search work inside the request: update_document() records a durable sync request in the same save, a background worker @@ -71,6 +72,8 @@ "_complete_document_search_sync_pass", "_delete_document_search_sync_record", "_read_document_for_search_sync", + "_approved_search_share_principals", + "_invalidate_document_search_results_cache", "run_document_search_metadata_sync", "process_due_document_search_metadata_syncs", } @@ -254,6 +257,7 @@ def __init__(self): self.events = [] self.logs = [] self.blob_tag_calls = [] + self.cache_invalidations = [] self.search_writes_frozen = False self.settings = FakeSettingsContainer() self.user_documents = FakeDocumentsContainer("user_id") @@ -321,6 +325,11 @@ def upsert_document(container, document_item, operation): "add_file_task_to_file_processing_log": lambda **_kwargs: None, "calculate_processing_percentage": lambda document_item: document_item.get("percentage_complete", 0), "propagate_tags_to_blob_metadata": lambda *args: harness.blob_tag_calls.append(args), + "invalidate_personal_search_cache": lambda user_id: harness.cache_invalidations.append(("personal", user_id)), + "invalidate_group_search_cache": lambda group_id: harness.cache_invalidations.append(("group", group_id)), + "invalidate_public_workspace_search_cache": ( + lambda workspace_id: harness.cache_invalidations.append(("public", workspace_id)) + ), "log_event": lambda message, extra=None, **kwargs: harness.logs.append((message, extra, kwargs)), } module = ast.Module(body=nodes, type_ignores=[]) @@ -501,6 +510,35 @@ def test_worker_projects_latest_pending_values_and_clears_the_request(): assert harness.blob_tag_calls == [("doc-1", ["bills"], "owner-1", None, None)] +def test_worker_clears_cached_search_results_for_the_owner_and_approved_recipients(): + """A sync that changed chunks clears cached results for every scope that can see the document.""" + harness = Harness() + harness.add_personal_document( + chunk_count=3, + shared_user_ids=["viewer-1,approved", "viewer-2,not_approved"], + ) + harness["update_document"](document_id="doc-1", user_id="owner-1", tags=["bills"]) + + harness["run_document_search_metadata_sync"]("document_search_metadata_sync:personal:doc-1") + + assert harness.cache_invalidations == [("personal", "owner-1"), ("personal", "viewer-1")] + + harness.cache_invalidations.clear() + harness.add_group_document(shared_group_ids=["group-2,approved", "group-3,not_approved"]) + harness["update_document"](document_id="group-doc-1", group_id="group-1", user_id="editor-1", tags=["q3"]) + harness["run_document_search_metadata_sync"]("document_search_metadata_sync:group:group-doc-1") + + assert harness.cache_invalidations == [("group", "group-1"), ("group", "group-2")] + + harness.cache_invalidations.clear() + harness.add_personal_document(document_id="doc-empty", chunk_count=0) + harness["update_document"](document_id="doc-empty", user_id="owner-1", tags=["bills"]) + result = harness["run_document_search_metadata_sync"]("document_search_metadata_sync:personal:doc-empty") + + assert result["status"] == "complete" and result["chunks_updated"] == 0 + assert harness.cache_invalidations == [] + + def test_worker_projects_a_newer_edit_that_arrives_mid_sync(): """A save during a running sync is picked up by the same worker instead of being lost.""" harness = Harness()