Kimirock/dify
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: pass session to workflow comment account accessors (#41117)

Browse Source 
Co-authored-by: geneachina <geneachina@gmail.com>
This commit is contained in:
酒诗
2026-08-24 03:20:32 +00:00
committed by GitHub
parent 11c922b5d4
commit aed1672366
4 changed files with 132 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
api/controllers/console/app/workflow_comment.py
View File

@@ -3,6 +3,7 @@ from datetime import datetime
from flask_restx import Resource
from pydantic import BaseModel, Field, TypeAdapter, computed_field, field_validator
from sqlalchemy.orm import Session
from controllers.common.schema import register_response_schema_models, register_schema_models
from controllers.console import console_ns
@@ -21,6 +22,15 @@ from fields.member_fields import AccountWithRole
from libs.helper import build_avatar_url, dump_response, to_timestamp
from libs.login import login_required
from models import Account, App
from models.comment import (
WorkflowComment as WorkflowCommentModel,
)
from models.comment import (
WorkflowCommentMention as WorkflowCommentMentionModel,
)
from models.comment import (
WorkflowCommentReply as WorkflowCommentReplyModel,
)
from services.account_service import TenantService
from services.workflow_comment_service import WorkflowCommentService
@@ -133,6 +143,73 @@ class WorkflowCommentDetail(ResponseModel):
return to_timestamp(value)
def _workflow_comment_reply_response(reply: WorkflowCommentReplyModel, *, session: Session) -> WorkflowCommentReply:
return WorkflowCommentReply.model_validate(
{
"id": reply.id,
"content": reply.content,
"created_by": reply.created_by,
"created_by_account": reply.created_by_account(session),
"created_at": reply.created_at,
}
)
def _workflow_comment_mention_response(
mention: WorkflowCommentMentionModel, *, session: Session
) -> WorkflowCommentMention:
return WorkflowCommentMention.model_validate(
{
"mentioned_user_id": mention.mentioned_user_id,
"mentioned_user_account": mention.mentioned_user_account(session),
"reply_id": mention.reply_id,
}
)
def _workflow_comment_basic_response(comment: WorkflowCommentModel, *, session: Session) -> WorkflowCommentBasic:
return WorkflowCommentBasic.model_validate(
{
"id": comment.id,
"position_x": comment.position_x,
"position_y": comment.position_y,
"content": comment.content,
"created_by": comment.created_by,
"created_by_account": comment.created_by_account(session),
"created_at": comment.created_at,
"updated_at": comment.updated_at,
"resolved": comment.resolved,
"resolved_at": comment.resolved_at,
"resolved_by": comment.resolved_by,
"resolved_by_account": comment.resolved_by_account(session),
"reply_count": comment.reply_count,
"mention_count": comment.mention_count,
"participants": comment.participants(session),
}
)
def _workflow_comment_detail_response(comment: WorkflowCommentModel, *, session: Session) -> WorkflowCommentDetail:
return WorkflowCommentDetail.model_validate(
{
"id": comment.id,
"position_x": comment.position_x,
"position_y": comment.position_y,
"content": comment.content,
"created_by": comment.created_by,
"created_by_account": comment.created_by_account(session),
"created_at": comment.created_at,
"updated_at": comment.updated_at,
"resolved": comment.resolved,
"resolved_at": comment.resolved_at,
"resolved_by": comment.resolved_by,
"resolved_by_account": comment.resolved_by_account(session),
"replies": [_workflow_comment_reply_response(reply, session=session) for reply in comment.replies],
"mentions": [_workflow_comment_mention_response(mention, session=session) for mention in comment.mentions],
}
)
class WorkflowCommentCreate(ResponseModel):
id: str
created_at: int | None = None
@@ -225,8 +302,11 @@ class WorkflowCommentListApi(Resource):
def get(self, current_tenant_id: str, app_model: App):
"""Get all comments for a workflow."""
comments = WorkflowCommentService.get_comments(tenant_id=current_tenant_id, app_id=app_model.id)
session = db.session()
return WorkflowCommentBasicList.model_validate({"data": comments}).model_dump(mode="json")
return WorkflowCommentBasicList.model_validate(
{"data": [_workflow_comment_basic_response(comment, session=session) for comment in comments]}
).model_dump(mode="json")
@console_ns.doc("create_workflow_comment")
@console_ns.doc(description="Create a new workflow comment")
@@ -281,8 +361,9 @@ class WorkflowCommentDetailApi(Resource):
comment = WorkflowCommentService.get_comment(
tenant_id=current_tenant_id, app_id=app_model.id, comment_id=comment_id
)
session = db.session()
return dump_response(WorkflowCommentDetail, comment)
return dump_response(WorkflowCommentDetail, _workflow_comment_detail_response(comment, session=session))
@console_ns.doc("update_workflow_comment")
@console_ns.doc(description="Update a workflow comment")

34
api/models/comment.py
View File

@@ -6,13 +6,12 @@ from datetime import datetime
import sqlalchemy as sa
from sqlalchemy import Index, func
from sqlalchemy.orm import Mapped, mapped_column, relationship
from sqlalchemy.orm import Mapped, Session, mapped_column, relationship
from models.base import TypeBase
from .account import Account
from .base import gen_uuidv7_string
from .engine import db
from .types import StringUUID
@@ -74,24 +73,22 @@ class WorkflowComment(TypeBase):
lambda: WorkflowCommentMention, back_populates="comment", cascade="all, delete-orphan", init=False
)
@property
def created_by_account(self):
def created_by_account(self, session: Session) -> Account | None:
"""Get creator account."""
if hasattr(self, "_created_by_account_cache"):
return self._created_by_account_cache
return db.session.get(Account, self.created_by)
return session.get(Account, self.created_by)
def cache_created_by_account(self, account: Account | None) -> None:
"""Cache creator account to avoid extra queries."""
self._created_by_account_cache = account
@property
def resolved_by_account(self):
def resolved_by_account(self, session: Session) -> Account | None:
"""Get resolver account."""
if hasattr(self, "_resolved_by_account_cache"):
return self._resolved_by_account_cache
if self.resolved_by:
return db.session.get(Account, self.resolved_by)
return session.get(Account, self.resolved_by)
return None
def cache_resolved_by_account(self, account: Account | None) -> None:
@@ -108,16 +105,15 @@ class WorkflowComment(TypeBase):
"""Get mention count."""
return len(self.mentions)
@property
def participants(self):
def participants(self, session: Session) -> list[Account]:
"""Get all participants (creator + repliers + mentioned users)."""
participant_ids: set[str] = set()
participants: list[Account] = []
# Use account properties to reuse preloaded caches and avoid hidden N+1.
# Use account accessors to reuse preloaded caches and avoid hidden N+1.
if self.created_by not in participant_ids:
participant_ids.add(self.created_by)
created_by_account = self.created_by_account
created_by_account = self.created_by_account(session)
if created_by_account:
participants.append(created_by_account)
@@ -125,7 +121,7 @@ class WorkflowComment(TypeBase):
if reply.created_by in participant_ids:
continue
participant_ids.add(reply.created_by)
reply_account = reply.created_by_account
reply_account = reply.created_by_account(session)
if reply_account:
participants.append(reply_account)
@@ -133,7 +129,7 @@ class WorkflowComment(TypeBase):
if mention.mentioned_user_id in participant_ids:
continue
participant_ids.add(mention.mentioned_user_id)
mentioned_account = mention.mentioned_user_account
mentioned_account = mention.mentioned_user_account(session)
if mentioned_account:
participants.append(mentioned_account)
@@ -177,12 +173,11 @@ class WorkflowCommentReply(TypeBase):
# Relationships
comment: Mapped[WorkflowComment] = relationship(lambda: WorkflowComment, back_populates="replies", init=False)
@property
def created_by_account(self):
def created_by_account(self, session: Session) -> Account | None:
"""Get creator account."""
if hasattr(self, "_created_by_account_cache"):
return self._created_by_account_cache
return db.session.get(Account, self.created_by)
return session.get(Account, self.created_by)
def cache_created_by_account(self, account: Account | None) -> None:
"""Cache creator account to avoid extra queries."""
@@ -222,12 +217,11 @@ class WorkflowCommentMention(TypeBase):
comment: Mapped[WorkflowComment] = relationship(lambda: WorkflowComment, back_populates="mentions", init=False)
reply: Mapped[WorkflowCommentReply | None] = relationship(lambda: WorkflowCommentReply, init=False)
@property
def mentioned_user_account(self):
def mentioned_user_account(self, session: Session) -> Account | None:
"""Get mentioned account."""
if hasattr(self, "_mentioned_user_account_cache"):
return self._mentioned_user_account_cache
return db.session.get(Account, self.mentioned_user_id)
return session.get(Account, self.mentioned_user_id)
def cache_mentioned_user_account(self, account: Account | None) -> None:
"""Cache mentioned account to avoid extra queries."""

54
api/tests/unit_tests/models/test_comment_models.py
View File

@@ -3,20 +3,10 @@ from uuid import uuid4
import pytest
from sqlalchemy.orm import Session
import models.comment as comment_module
from models.account import Account
from models.comment import WorkflowComment, WorkflowCommentMention, WorkflowCommentReply
class _DatabaseBinding:
"""Expose a real SQLite session to model properties that use ``db.session``."""
session: Session
def __init__(self, session: Session) -> None:
self.session = session
def _account(name: str) -> Account:
account = Account(name=name, email=f"{name.lower()}@example.com")
account.id = str(uuid4())
@@ -39,18 +29,15 @@ COMMENT_MODELS = (Account, WorkflowComment, WorkflowCommentReply, WorkflowCommen
@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True)
def test_workflow_comment_account_properties_and_cache(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
def test_workflow_comment_account_accessors_and_cache(sqlite_session: Session) -> None:
created_account = _account("Creator")
resolved_account = _account("Resolver")
comment = _comment(created_account.id, resolved_account.id)
sqlite_session.add_all([created_account, resolved_account, comment])
sqlite_session.commit()
monkeypatch.setattr(comment_module, "db", _DatabaseBinding(sqlite_session))
assert comment.created_by_account is created_account
assert comment.resolved_by_account is resolved_account
assert comment.created_by_account(sqlite_session) is created_account
assert comment.resolved_by_account(sqlite_session) is resolved_account
comment.cache_created_by_account(created_account)
comment.cache_resolved_by_account(resolved_account)
@@ -58,17 +45,17 @@ def test_workflow_comment_account_properties_and_cache(
sqlite_session.delete(resolved_account)
sqlite_session.commit()
assert comment.created_by_account is created_account
assert comment.resolved_by_account is resolved_account
assert comment.created_by_account(sqlite_session) is created_account
assert comment.resolved_by_account(sqlite_session) is resolved_account
comment_without_resolver = _comment(str(uuid4()))
sqlite_session.add(comment_without_resolver)
sqlite_session.commit()
assert comment_without_resolver.resolved_by_account is None
assert comment_without_resolver.resolved_by_account(sqlite_session) is None
@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True)
def test_workflow_comment_counts_and_participants(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> None:
def test_workflow_comment_counts_and_participants(sqlite_session: Session) -> None:
account_1 = _account("Creator")
account_2 = _account("Replier")
account_3 = _account("Mentioned")
@@ -84,9 +71,8 @@ def test_workflow_comment_counts_and_participants(monkeypatch: pytest.MonkeyPatc
]
sqlite_session.add_all([account_1, account_2, account_3, comment])
sqlite_session.commit()
monkeypatch.setattr(comment_module, "db", _DatabaseBinding(sqlite_session))
participants = comment.participants
participants = comment.participants(sqlite_session)
assert comment.reply_count == 2
assert comment.mention_count == 2
@@ -94,9 +80,7 @@ def test_workflow_comment_counts_and_participants(monkeypatch: pytest.MonkeyPatc
@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True)
def test_workflow_comment_participants_use_cached_accounts(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
def test_workflow_comment_participants_use_cached_accounts(sqlite_session: Session) -> None:
account_1 = _account("Creator")
account_2 = _account("Replier")
account_3 = _account("Mentioned")
@@ -107,7 +91,6 @@ def test_workflow_comment_participants_use_cached_accounts(
comment.mentions = [mention]
sqlite_session.add_all([account_1, account_2, account_3, comment])
sqlite_session.commit()
monkeypatch.setattr(comment_module, "db", _DatabaseBinding(sqlite_session))
comment.cache_created_by_account(account_1)
reply.cache_created_by_account(account_2)
@@ -117,13 +100,15 @@ def test_workflow_comment_participants_use_cached_accounts(
sqlite_session.delete(account_3)
sqlite_session.commit()
assert {account.id for account in comment.participants} == {account_1.id, account_2.id, account_3.id}
assert {account.id for account in comment.participants(sqlite_session)} == {
account_1.id,
account_2.id,
account_3.id,
}
@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True)
def test_reply_and_mention_account_properties_and_cache(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
def test_reply_and_mention_account_accessors_and_cache(sqlite_session: Session) -> None:
reply_account = _account("Replier")
mention_account = _account("Mentioned")
comment = _comment(reply_account.id)
@@ -133,10 +118,9 @@ def test_reply_and_mention_account_properties_and_cache(
comment.mentions = [mention]
sqlite_session.add_all([reply_account, mention_account, comment])
sqlite_session.commit()
monkeypatch.setattr(comment_module, "db", _DatabaseBinding(sqlite_session))
assert reply.created_by_account is reply_account
assert mention.mentioned_user_account is mention_account
assert reply.created_by_account(sqlite_session) is reply_account
assert mention.mentioned_user_account(sqlite_session) is mention_account
reply.cache_created_by_account(reply_account)
mention.cache_mentioned_user_account(mention_account)
@@ -144,5 +128,5 @@ def test_reply_and_mention_account_properties_and_cache(
sqlite_session.delete(mention_account)
sqlite_session.commit()
assert reply.created_by_account is reply_account
assert mention.mentioned_user_account is mention_account
assert reply.created_by_account(sqlite_session) is reply_account
assert mention.mentioned_user_account(sqlite_session) is mention_account

22
api/tests/unit_tests/services/test_workflow_comment_service.py
View File

@@ -403,10 +403,18 @@ class TestWorkflowCommentService:
assert len(result) == 1
loaded_comment = result[0]
assert loaded_comment.id == comment.id
assert loaded_comment.created_by_account.id == OWNER_ID
assert loaded_comment.resolved_by_account.id == USER_2_ID
assert loaded_comment.replies[0].created_by_account.id == USER_3_ID
assert loaded_comment.mentions[0].mentioned_user_account.id == USER_4_ID
created_by_account = loaded_comment.created_by_account(sqlite_session)
resolved_by_account = loaded_comment.resolved_by_account(sqlite_session)
reply_account = loaded_comment.replies[0].created_by_account(sqlite_session)
mentioned_user_account = loaded_comment.mentions[0].mentioned_user_account(sqlite_session)
assert created_by_account is not None
assert created_by_account.id == OWNER_ID
assert resolved_by_account is not None
assert resolved_by_account.id == USER_2_ID
assert reply_account is not None
assert reply_account.id == USER_3_ID
assert mentioned_user_account is not None
assert mentioned_user_account.id == USER_4_ID
def test_preload_accounts_returns_early_for_empty_comments(self, sqlite_session: Session) -> None:
statements: list[str] = []
@@ -434,8 +442,10 @@ class TestWorkflowCommentService:
result = WorkflowCommentService.get_comment(TENANT_ID, APP_ID, comment.id)
assert result.id == comment.id
assert result.created_by_account.id == OWNER_ID
assert result.resolved_by_account is None
created_by_account = result.created_by_account(sqlite_session)
assert created_by_account is not None
assert created_by_account.id == OWNER_ID
assert result.resolved_by_account(sqlite_session) is None
def test_delete_comment_raises_forbidden(self, sqlite_session: Session) -> None:
comment = _comment()