From 648806bc9c32eeb9e7f2307e067435f7de4e0840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Mon, 10 Aug 2026 16:07:54 +0800 Subject: [PATCH] refactor(api): extract explore banner query service (#40053) --- api/.importlinter | 14 + api/controllers/console/explore/banner.py | 23 +- api/extensions/ext_application_services.py | 7 + .../explore_banner_query_repository.py | 47 +++ api/services/explore_banner_query_service.py | 44 +++ .../console/explore/test_banner.py | 304 ++++++++++-------- .../controllers/console/test_feature.py | 2 + 7 files changed, 288 insertions(+), 153 deletions(-) create mode 100644 api/repositories/explore_banner_query_repository.py create mode 100644 api/services/explore_banner_query_service.py diff --git a/api/.importlinter b/api/.importlinter index 922f614128a..22130ee9d73 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -65,6 +65,20 @@ forbidden_modules = sqlalchemy werkzeug +[importlinter:contract:explore-banner-query-service-boundary] +name = Explore banner query application service is framework and persistence neutral +type = forbidden +source_modules = + services.explore_banner_query_service +forbidden_modules = + controllers + extensions + flask + models + repositories + sqlalchemy + werkzeug + [importlinter:contract:feature-query-service-boundary] name = Feature query application service is framework and persistence neutral type = forbidden diff --git a/api/controllers/console/explore/banner.py b/api/controllers/console/explore/banner.py index b63e025b609..9b1e3a106a5 100644 --- a/api/controllers/console/explore/banner.py +++ b/api/controllers/console/explore/banner.py @@ -3,17 +3,14 @@ from typing import cast from flask_restx import Namespace, Resource from pydantic import BaseModel, Field, RootModel, field_validator -from sqlalchemy import select from controllers.common.schema import query_params_from_model, register_response_schema_models from controllers.console import api from controllers.console.wraps import model_validate -from extensions.ext_database import db +from extensions.ext_application_services import application_services from fields.base import ResponseModel from libs.helper import dump_response from models.enums import BannerStatus -from models.model import ExporleBanner -from services.feature_service import FeatureService class BannerListQuery(BaseModel): @@ -67,23 +64,7 @@ class BannerApi(Resource): @model_validate(BannerListQuery) def get(self, req_data: BannerListQuery): """Get banner list.""" - if not FeatureService.is_explore_banner_enabled(): - return dump_response(BannerListResponse, []) - - # Build base query for enabled banners - base_query = select(ExporleBanner).where(ExporleBanner.status == BannerStatus.ENABLED) - - # Try to get banners in the requested language - banners = db.session.scalars( - base_query.where(ExporleBanner.language == req_data.language).order_by(ExporleBanner.sort) - ).all() - - # Fallback to en-US if no banners found and language is not en-US - if not banners and req_data.language != "en-US": - banners = db.session.scalars( - base_query.where(ExporleBanner.language == "en-US").order_by(ExporleBanner.sort) - ).all() - + banners = application_services().explore_banner_queries.list_for_language(req_data.language) return dump_response(BannerListResponse, banners) diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index 63a91b2cfee..8c4316b8146 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -12,9 +12,11 @@ from core.db.session_factory import get_session_maker from core.schemas.schema_manager import SchemaManager from enums.deployment_edition import DeploymentEdition from extensions.ext_redis import RedisClientWrapper, redis_client +from repositories.explore_banner_query_repository import ExploreBannerQueryRepository from repositories.installation_state_repository import InstallationStateRepository from repositories.workspace_member_query_repository import WorkspaceMemberQueryRepository from repositories.workspace_query_repository import WorkspaceQueryRepository +from services.explore_banner_query_service import ExploreBannerQueryService from services.feature_query_service import FeatureQueryService from services.feature_service import FeatureService from services.feature_service_gateway import FeatureServiceGateway @@ -31,6 +33,7 @@ _EXTENSION_KEY = "application_services" @dataclass(frozen=True, slots=True) class ApplicationServices: + explore_banner_queries: ExploreBannerQueryService schema_definitions: SchemaDefinitionService setup: SetupService feature_queries: FeatureQueryService @@ -46,6 +49,10 @@ def build_application_services( ) -> ApplicationServices: installation_state = InstallationStateRepository(client=database_client) return ApplicationServices( + explore_banner_queries=ExploreBannerQueryService( + banners=ExploreBannerQueryRepository(client=database_client), + is_enabled=FeatureService.is_explore_banner_enabled, + ), schema_definitions=SchemaDefinitionService(source_factory=SchemaManager), setup=SetupService( state=installation_state, diff --git a/api/repositories/explore_banner_query_repository.py b/api/repositories/explore_banner_query_repository.py new file mode 100644 index 00000000000..011828b7977 --- /dev/null +++ b/api/repositories/explore_banner_query_repository.py @@ -0,0 +1,47 @@ +"""Database repository for the Explore banner read model.""" + +from typing import override + +from sqlalchemy import select +from sqlalchemy.orm import Session, sessionmaker + +from models.enums import BannerStatus +from models.model import ExporleBanner +from services.explore_banner_query_service import ExploreBannerQuery, ExploreBannerRecord + + +class ExploreBannerQueryRepository(ExploreBannerQuery): + def __init__(self, client: sessionmaker[Session]) -> None: + self._client = client + + @override + def list_enabled(self, language: str) -> tuple[ExploreBannerRecord, ...]: + stmt = ( + select( + ExporleBanner.id, + ExporleBanner.content, + ExporleBanner.link, + ExporleBanner.sort, + ExporleBanner.status, + ExporleBanner.created_at, + ) + .where( + ExporleBanner.status == BannerStatus.ENABLED, + ExporleBanner.language == language, + ) + .order_by(ExporleBanner.sort) + ) + + with self._client() as session: + rows = session.execute(stmt).all() + return tuple( + ExploreBannerRecord( + id=banner_id, + content=content, + link=link, + sort=sort, + status=status.value, + created_at=created_at, + ) + for banner_id, content, link, sort, status, created_at in rows + ) diff --git a/api/services/explore_banner_query_service.py b/api/services/explore_banner_query_service.py new file mode 100644 index 00000000000..5a64245bb94 --- /dev/null +++ b/api/services/explore_banner_query_service.py @@ -0,0 +1,44 @@ +"""Application service for listing Explore banners. + +ExploreBanner is the legacy contract name shared by the API, feature flag, and database model. +""" + +from collections.abc import Callable, Sequence +from datetime import datetime +from typing import Any, NamedTuple, Protocol + +_DEFAULT_LANGUAGE = "en-US" + + +class ExploreBannerRecord(NamedTuple): + id: str + content: dict[str, Any] + link: str + sort: int + status: str + created_at: datetime + + +class ExploreBannerQuery(Protocol): + def list_enabled(self, language: str) -> Sequence[ExploreBannerRecord]: ... + + +class ExploreBannerQueryService: + def __init__( + self, + *, + banners: ExploreBannerQuery, + is_enabled: Callable[[], bool], + ) -> None: + self._banners = banners + self._is_enabled = is_enabled + + def list_for_language(self, language: str) -> tuple[ExploreBannerRecord, ...]: + if not self._is_enabled(): + return () + + banners = tuple(self._banners.list_enabled(language)) + if banners or language == _DEFAULT_LANGUAGE: + return banners + + return tuple(self._banners.list_enabled(_DEFAULT_LANGUAGE)) diff --git a/api/tests/unit_tests/controllers/console/explore/test_banner.py b/api/tests/unit_tests/controllers/console/explore/test_banner.py index 25432a3f944..594be28f0df 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_banner.py +++ b/api/tests/unit_tests/controllers/console/explore/test_banner.py @@ -1,22 +1,25 @@ from collections.abc import Iterator from datetime import datetime -from unittest.mock import MagicMock +from types import SimpleNamespace +from unittest.mock import MagicMock, call from uuid import uuid4 import pytest from flask import Flask from pydantic import ValidationError from sqlalchemy.engine import Engine -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, scoped_session, sessionmaker import controllers.console.explore.banner as banner_module from models.base import TypeBase from models.enums import BannerStatus from models.model import ExporleBanner +from repositories.explore_banner_query_repository import ExploreBannerQueryRepository +from services.explore_banner_query_service import ExploreBannerQueryService, ExploreBannerRecord @pytest.fixture -def banner_session(sqlite_engine: Engine) -> Iterator[Session]: +def banner_session(sqlite_engine: Engine) -> Iterator[scoped_session[Session]]: """Create the banner table without its PostgreSQL-cast server defaults.""" table = TypeBase.metadata.tables[ExporleBanner.__tablename__] status_default = table.c.status.server_default @@ -29,21 +32,41 @@ def banner_session(sqlite_engine: Engine) -> Iterator[Session]: table.c.status.server_default = status_default table.c.language.server_default = language_default - with Session(sqlite_engine, expire_on_commit=False) as session: + session = scoped_session(sessionmaker(bind=sqlite_engine, expire_on_commit=False)) + try: yield session + finally: + session.remove() -def _banner(*, title: str, language: str, link: str, created_at: datetime) -> ExporleBanner: +def _content( + title: str, + *, + category: str = "Featured", + description: str = "Banner description", +) -> dict[str, str]: + return { + "category": category, + "title": title, + "description": description, + "img-src": "https://example.com/banner.png", + } + + +def _banner( + *, + title: str, + language: str, + link: str, + created_at: datetime, + sort: int = 1, + status: BannerStatus = BannerStatus.ENABLED, +) -> ExporleBanner: banner = ExporleBanner( - content={ - "category": "Featured", - "title": title, - "description": "Banner description", - "img-src": "https://example.com/banner.png", - }, + content=_content(title), link=link, - sort=1, - status=BannerStatus.ENABLED, + sort=sort, + status=status, language=language, ) banner.id = str(uuid4()) @@ -51,32 +74,123 @@ def _banner(*, title: str, language: str, link: str, created_at: datetime) -> Ex return banner -class TestBannerApi: - def test_get_banners_with_requested_language( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - banner_session: Session, - ): - api = banner_module.BannerApi() +def _record( + *, + title: str = "hello", + category: str = "Featured", + description: str = "Banner description", +) -> ExploreBannerRecord: + return ExploreBannerRecord( + id="banner-1", + content=_content(title, category=category, description=description), + link="https://example.com", + sort=1, + status=BannerStatus.ENABLED.value, + created_at=datetime(2024, 1, 1), + ) - banner = _banner( - title="hello", + +class TestExploreBannerQueryService: + def test_returns_empty_without_querying_when_disabled(self) -> None: + banners = MagicMock() + service = ExploreBannerQueryService(banners=banners, is_enabled=lambda: False) + + assert service.list_for_language("fr-FR") == () + banners.list_enabled.assert_not_called() + + def test_returns_requested_language(self) -> None: + record = _record() + banners = MagicMock() + banners.list_enabled.return_value = (record,) + service = ExploreBannerQueryService(banners=banners, is_enabled=lambda: True) + + assert service.list_for_language("fr-FR") == (record,) + banners.list_enabled.assert_called_once_with("fr-FR") + + def test_falls_back_to_en_us(self) -> None: + record = _record(title="fallback") + banners = MagicMock() + banners.list_enabled.side_effect = [(), (record,)] + service = ExploreBannerQueryService(banners=banners, is_enabled=lambda: True) + + assert service.list_for_language("es-ES") == (record,) + assert banners.list_enabled.call_args_list == [ + call("es-ES"), + call("en-US"), + ] + + def test_does_not_repeat_default_language_query(self) -> None: + banners = MagicMock() + banners.list_enabled.return_value = () + service = ExploreBannerQueryService(banners=banners, is_enabled=lambda: True) + + assert service.list_for_language("en-US") == () + banners.list_enabled.assert_called_once_with("en-US") + + +class TestExploreBannerQueryRepository: + def test_filters_language_and_status_and_orders_by_sort(self, banner_session: scoped_session[Session]) -> None: + created_at = datetime(2024, 1, 1) + second = _banner( + title="second", language="fr-FR", - link="https://example.com", - created_at=datetime(2024, 1, 1), + link="https://example.com/second", + created_at=created_at, + sort=2, ) - banner_session.add(banner) + first = _banner( + title="first", + language="fr-FR", + link="https://example.com/first", + created_at=created_at, + ) + disabled = _banner( + title="disabled", + language="fr-FR", + link="https://example.com/disabled", + created_at=created_at, + sort=0, + status=BannerStatus.DISABLED, + ) + english = _banner( + title="english", + language="en-US", + link="https://example.com/english", + created_at=created_at, + ) + banner_session.add_all([second, first, disabled, english]) banner_session.commit() - monkeypatch.setattr(banner_module.db, "session", banner_session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: True) + + repository = ExploreBannerQueryRepository(banner_session.session_factory) + result = repository.list_enabled("fr-FR") + + assert [banner.id for banner in result] == [first.id, second.id] + assert result[0] == ExploreBannerRecord( + id=first.id, + content=_content("first"), + link="https://example.com/first", + sort=1, + status=BannerStatus.ENABLED.value, + created_at=created_at, + ) + + +class TestBannerApi: + def test_get_serializes_requested_language(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + queries = MagicMock() + queries.list_for_language.return_value = (_record(),) + monkeypatch.setattr( + banner_module, + "application_services", + lambda: SimpleNamespace(explore_banner_queries=queries), + ) with app.test_request_context("/?language=fr-FR"): - result = api.get() + result = banner_module.BannerApi().get() assert result == [ { - "id": banner.id, + "id": "banner-1", "content": { "category": "Featured", "title": "hello", @@ -89,125 +203,51 @@ class TestBannerApi: "created_at": "2024-01-01T00:00:00", } ] + queries.list_for_language.assert_called_once_with("fr-FR") - def test_get_banners_fallback_to_en_us( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - banner_session: Session, - ): - api = banner_module.BannerApi() - - banner = _banner( - title="fallback", - language="en-US", - link="https://example.com/fallback", - created_at=datetime(2024, 1, 2), + def test_get_uses_default_language(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + queries = MagicMock() + queries.list_for_language.return_value = () + monkeypatch.setattr( + banner_module, + "application_services", + lambda: SimpleNamespace(explore_banner_queries=queries), ) - banner_session.add(banner) - banner_session.commit() - monkeypatch.setattr(banner_module.db, "session", banner_session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: True) - - with app.test_request_context("/?language=es-ES"): - result = api.get() - - assert result == [ - { - "id": banner.id, - "content": { - "category": "Featured", - "title": "fallback", - "description": "Banner description", - "img-src": "https://example.com/banner.png", - }, - "link": "https://example.com/fallback", - "sort": 1, - "status": "enabled", - "created_at": "2024-01-02T00:00:00", - } - ] - - def test_get_banners_default_language_en_us( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - banner_session: Session, - ): - api = banner_module.BannerApi() - monkeypatch.setattr(banner_module.db, "session", banner_session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: True) with app.test_request_context("/"): - result = api.get() + result = banner_module.BannerApi().get() assert result == [] + queries.list_for_language.assert_called_once_with("en-US") - def test_get_banners_allows_empty_supporting_copy( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - banner_session: Session, - ): - api = banner_module.BannerApi() - banner = _banner( - title="title only", - language="en-US", - link="https://example.com", - created_at=datetime(2024, 1, 3), + def test_get_allows_empty_supporting_copy(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + queries = MagicMock() + queries.list_for_language.return_value = (_record(category="", description=""),) + monkeypatch.setattr( + banner_module, + "application_services", + lambda: SimpleNamespace(explore_banner_queries=queries), ) - banner.content["category"] = "" - banner.content["description"] = "" - banner_session.add(banner) - banner_session.commit() - monkeypatch.setattr(banner_module.db, "session", banner_session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: True) - with app.test_request_context("/?language=en-US"): - result = api.get() + with app.test_request_context("/"): + result = banner_module.BannerApi().get() assert result[0]["content"] == { "category": "", - "title": "title only", + "title": "hello", "description": "", "img-src": "https://example.com/banner.png", } - def test_get_banners_returns_empty_without_querying_when_disabled( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - ): - api = banner_module.BannerApi() - session = MagicMock() - monkeypatch.setattr(banner_module.db, "session", session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: False) - - with app.test_request_context("/"): - result = api.get() - - assert result == [] - session.scalars.assert_not_called() - - def test_get_banners_rejects_invalid_content( - self, - app: Flask, - monkeypatch: pytest.MonkeyPatch, - banner_session: Session, - ): - api = banner_module.BannerApi() - banner = _banner( - title="invalid", - language="en-US", - link="https://example.com", - created_at=datetime(2024, 1, 4), + def test_get_rejects_invalid_content(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + queries = MagicMock() + queries.list_for_language.return_value = (_record()._replace(content={"title": "invalid"}),) + monkeypatch.setattr( + banner_module, + "application_services", + lambda: SimpleNamespace(explore_banner_queries=queries), ) - banner.content = {"title": "invalid"} - banner_session.add(banner) - banner_session.commit() - monkeypatch.setattr(banner_module.db, "session", banner_session) - monkeypatch.setattr(banner_module.FeatureService, "is_explore_banner_enabled", lambda: True) with app.test_request_context("/"): with pytest.raises(ValidationError): - api.get() + banner_module.BannerApi().get() diff --git a/api/tests/unit_tests/controllers/console/test_feature.py b/api/tests/unit_tests/controllers/console/test_feature.py index f8435c7b89c..8491ad89d66 100644 --- a/api/tests/unit_tests/controllers/console/test_feature.py +++ b/api/tests/unit_tests/controllers/console/test_feature.py @@ -15,6 +15,7 @@ from services.entities.feature_entities import ( SystemFeatureModel, VectorSpaceLimitationModel, ) +from services.explore_banner_query_service import ExploreBannerQueryService from services.feature_query_service import FeatureQueryService from services.schema_definition_service import SchemaDefinitionService from services.setup_service import SetupService @@ -34,6 +35,7 @@ def _request_context() -> RequestContext: def _install_application_services(mocker: MockerFixture): feature_queries = create_autospec(FeatureQueryService, instance=True, spec_set=True) services = ApplicationServices( + explore_banner_queries=create_autospec(ExploreBannerQueryService, instance=True, spec_set=True), schema_definitions=create_autospec(SchemaDefinitionService, instance=True, spec_set=True), setup=create_autospec(SetupService, instance=True, spec_set=True), feature_queries=feature_queries,