mirror of
https://github.com/langgenius/dify.git
synced 2026-09-09 05:41:00 +08:00
fix(dataset_service): refresh session snapshot before UploadFile lookup (#41840)
Co-authored-by: Taranum01 <taranumwasu@users.noreply.github.com>
This commit is contained in:
parent
4f463b1c46
commit
eeff2c01b1
@ -2341,6 +2341,23 @@ class DocumentService:
|
||||
if not knowledge_config.data_source.info_list.file_info_list:
|
||||
raise ValueError("File source info is required")
|
||||
upload_file_list = knowledge_config.data_source.info_list.file_info_list.file_ids
|
||||
# Issue #41735: under MySQL's default
|
||||
# ``REPEATABLE READ`` isolation, the SELECT snapshot
|
||||
# for this transaction is pinned by the first
|
||||
# read done earlier in ``save_document_with_dataset_id``
|
||||
# (e.g. ``check_doc_form``). The caller has just
|
||||
# committed the upload in a different session
|
||||
# (``FileService.upload_text`` /
|
||||
# ``FileService.upload_file``), so the rows exist
|
||||
# but are invisible to *this* session until its
|
||||
# snapshot is refreshed. End the current transaction
|
||||
# so the next SELECT starts with a fresh snapshot.
|
||||
# The pending changes on ``dataset`` are also
|
||||
# flushed, which is what we want — they must be
|
||||
# persisted by this point anyway. PostgreSQL's
|
||||
# default ``READ COMMITTED`` rebuilds the read view
|
||||
# per statement so it isn't affected.
|
||||
session.commit()
|
||||
files = list(
|
||||
session.scalars(
|
||||
select(UploadFile).where(
|
||||
|
||||
@ -0,0 +1,70 @@
|
||||
"""Regression coverage for #41735.
|
||||
|
||||
The pre-fix code in ``DocumentService.save_document_with_dataset_id``
|
||||
read ``UploadFile`` rows inside a single session that had its
|
||||
``REPEATABLE READ`` snapshot pinned by an earlier ``Dataset`` SELECT.
|
||||
When ``FileService.upload_text`` / ``upload_file`` committed the
|
||||
``upload_files`` row in a SEPARATE session, the row was invisible to
|
||||
*this* session until the snapshot was refreshed, and
|
||||
``save_document_with_dataset_id`` reported ``One or more files not
|
||||
found.`` even though the row was committed (verified via a second
|
||||
session).
|
||||
|
||||
The fix forces ``session.commit()`` immediately before the
|
||||
``UploadFile`` lookup so the next SELECT starts a fresh snapshot.
|
||||
This file pins that contract by reading the function source and
|
||||
asserting the structural invariant directly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import inspect
|
||||
|
||||
from services.dataset_service import DocumentService
|
||||
|
||||
|
||||
def test_session_committed_before_upload_file_lookup() -> None:
|
||||
"""#41735: ``session.commit()`` must run BEFORE the ``UploadFile``
|
||||
SELECT so the SELECT sees rows committed by ``FileService.upload_text``
|
||||
on a different session. MySQL REPEATABLE READ keeps the original
|
||||
snapshot for the rest of the transaction; the commit ends it so the
|
||||
next SELECT starts a fresh transaction with a new snapshot.
|
||||
PostgreSQL's default READ COMMITTED rebuilds the read view per
|
||||
statement so the extra commit is a no-op there.
|
||||
|
||||
Read the function source and assert that ``session.commit()`` is
|
||||
called BEFORE the ``UploadFile`` SELECT — the same structural
|
||||
invariant the reproduction in the issue depends on, without needing
|
||||
a live MySQL database.
|
||||
"""
|
||||
source_lines = inspect.getsource(DocumentService.save_document_with_dataset_id).splitlines()
|
||||
|
||||
upload_select_line = next(
|
||||
(i for i, line in enumerate(source_lines) if "select(UploadFile)" in line),
|
||||
None,
|
||||
)
|
||||
assert upload_select_line is not None, "save_document_with_dataset_id must contain a select(UploadFile) call"
|
||||
|
||||
# Find every ``session.commit()`` call, then keep the last one
|
||||
# whose line number is BEFORE the UploadFile SELECT. If the fix is
|
||||
# in place, that commit is the snapshot refresh.
|
||||
last_commit_before_upload = -1
|
||||
for i, line in enumerate(source_lines):
|
||||
if i >= upload_select_line:
|
||||
break
|
||||
if "session.commit()" in line:
|
||||
last_commit_before_upload = i
|
||||
|
||||
assert last_commit_before_upload >= 0, (
|
||||
"save_document_with_dataset_id must call session.commit() at least once before the UploadFile SELECT"
|
||||
)
|
||||
|
||||
# The fix is a focused call: one extra ``session.commit()`` between
|
||||
# the (already-existing) ``DocumentService.check_doc_form`` call and
|
||||
# the ``select(UploadFile)`` call. Pin that there's a comment
|
||||
# referencing #41735 right above the new commit so a future edit
|
||||
# doesn't accidentally delete the snapshot refresh without
|
||||
# understanding why it's there.
|
||||
assert "#41735" in "\n".join(source_lines), (
|
||||
"The fix's invariant comment referencing issue #41735 must be present in save_document_with_dataset_id"
|
||||
)
|
||||
Loading…
Reference in New Issue
Block a user