Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion application/single_app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions application/single_app/functions_documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
DataManagementSearchWritesFrozenError,
hold_data_management_search_write_slot,
)
from utils_cache import (
invalidate_group_search_cache,

Check warning on line 32 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.

Check warning on line 32 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
invalidate_personal_search_cache,

Check warning on line 33 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.

Check warning on line 33 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
invalidate_public_workspace_search_cache,

Check warning on line 34 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains AI, plugin, agent, or workspace boundary marker. Recommendation%3A Check whether prompts, chat history, uploaded documents, embeddings, citations, settings, or identity can cross a new boundary.

Check warning on line 34 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.

Check warning on line 34 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
)
Comment on lines +31 to +35
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
Expand Down Expand Up @@ -4828,6 +4833,44 @@
return document_item if belongs_to_scope else None


def _approved_search_share_principals(entries):

Check warning on line 4836 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
"""Return the users or groups whose approved share lets them find a document in search."""

Check warning on line 4837 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
principals = []
for entry in _normalize_search_acl_entries(entries):

Check warning on line 4839 in application/single_app/functions_documents.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
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.

Expand Down Expand Up @@ -4870,6 +4913,12 @@
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.",
Expand Down
3 changes: 2 additions & 1 deletion docs/explanation/fixes/DOCUMENT_SEARCH_METADATA_SYNC_FIX.md
Original file line number Diff line number Diff line change
@@ -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).

Expand Down Expand Up @@ -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.

Expand Down
9 changes: 9 additions & 0 deletions docs/explanation/release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 39 additions & 1 deletion functional_tests/test_document_search_metadata_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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=[])
Expand Down Expand Up @@ -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()
Expand Down
Loading