From 039284fbe2f2d6750a8af9a3424be7c3a8f3234a Mon Sep 17 00:00:00 2001 From: Vincent Vatelot Date: Fri, 2 Oct 2026 13:24:19 +0200 Subject: [PATCH 1/2] feat(api): persist RWEB best practices with catalog and result tables Store best-practice definitions and per-analysis evaluation snapshots, expose them via GET /ecoindexes/{id}/best-practices, and opt in with include_best_practices on task creation. Co-authored-by: Cursor --- bases/ecoindex/backend/routers/ecoindex.py | 44 +++++ bases/ecoindex/backend/routers/tasks.py | 10 + bases/ecoindex/worker/tasks.py | 8 + components/ecoindex/database/engine.py | 7 +- .../ecoindex/database/models/__init__.py | 183 ++++++++++++++++-- .../database/repositories/best_practices.py | 75 +++++++ .../database/repositories/ecoindex.py | 61 +++++- .../ecoindex/database/repositories/worker.py | 15 ++ projects/ecoindex_api/alembic/env.py | 7 +- .../a1b2c3d4e5f6_add_best_practices_tables.py | 148 ++++++++++++++ .../database/test_repository_queries.py | 97 ++++++++++ 11 files changed, 636 insertions(+), 19 deletions(-) create mode 100644 components/ecoindex/database/repositories/best_practices.py create mode 100644 projects/ecoindex_api/alembic/versions/a1b2c3d4e5f6_add_best_practices_tables.py diff --git a/bases/ecoindex/backend/routers/ecoindex.py b/bases/ecoindex/backend/routers/ecoindex.py index a8ebd52..a447a62 100644 --- a/bases/ecoindex/backend/routers/ecoindex.py +++ b/bases/ecoindex/backend/routers/ecoindex.py @@ -12,9 +12,11 @@ from ecoindex.database.engine import get_session from ecoindex.database.models import ( ApiEcoindex, + BestPracticesAnalysisResponse, PageApiEcoindexes, ) from ecoindex.database.repositories.ecoindex import ( + get_best_practice_results_by_analysis_id_db, get_count_analysis_db, get_ecoindex_result_by_id_db, get_ecoindex_result_list_db, @@ -184,6 +186,48 @@ async def get_ecoindex_analysis_requests_by_id( ) +@router.get( + name="Get ecoindex analysis best practices by id", + path="/{id}/best-practices", + response_model=BestPracticesAnalysisResponse, + response_description="Best practices results of the ecoindex analysis", + responses={ + status.HTTP_204_NO_CONTENT: { + "description": ( + "Analysis exists but best practices were not collected" + ) + }, + status.HTTP_404_NOT_FOUND: example_ecoindex_not_found, + }, + description=( + "This returns the RWEB best practices evaluation for the analysis. " + "Returns 204 when the analysis exists but best practices were not collected." + ), +) +async def get_ecoindex_analysis_best_practices_by_id( + id: IdParameter, + version: VersionParameter = Version.v1, + session: AsyncSession = Depends(get_session), +) -> BestPracticesAnalysisResponse | Response: + ecoindex = await get_ecoindex_result_by_id_db( + session=session, id=id, version=version + ) + + if not ecoindex: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Analysis {id} not found for version {version.value}", + ) + + report = await get_best_practice_results_by_analysis_id_db( + session=session, analysis_id=id + ) + if report is None: + return Response(status_code=status.HTTP_204_NO_CONTENT) + + return report + + @router.get( name="Get screenshot", path="/{id}/screenshot", diff --git a/bases/ecoindex/backend/routers/tasks.py b/bases/ecoindex/backend/routers/tasks.py index 498a844..2f5cdb1 100644 --- a/bases/ecoindex/backend/routers/tasks.py +++ b/bases/ecoindex/backend/routers/tasks.py @@ -98,6 +98,15 @@ async def add_ecoindex_analysis_task( example=False, ), ] = False, + include_best_practices: Annotated[ + bool, + Body( + description=( + "If true, evaluate and store RWEB best practices for the page" + ), + example=False, + ), + ] = False, session: AsyncSession = Depends(get_session), ) -> str: if Settings().DAILY_LIMIT_PER_HOST: @@ -151,6 +160,7 @@ async def add_ecoindex_analysis_task( height=web_page.height, custom_headers=headers, include_requests_detail=include_requests_detail, + include_best_practices=include_best_practices, **_enqueue_settings(), ) diff --git a/bases/ecoindex/worker/tasks.py b/bases/ecoindex/worker/tasks.py index f715ce6..9f1c78c 100644 --- a/bases/ecoindex/worker/tasks.py +++ b/bases/ecoindex/worker/tasks.py @@ -44,6 +44,7 @@ def ecoindex_task( height: int, custom_headers: dict[str, str], include_requests_detail: bool = False, + include_best_practices: bool = False, ) -> str: queue_task_result = run( async_ecoindex_task( @@ -53,6 +54,7 @@ def ecoindex_task( height=height, custom_headers=custom_headers, include_requests_detail=include_requests_detail, + include_best_practices=include_best_practices, ) ) @@ -66,6 +68,7 @@ async def async_ecoindex_task( height: int, custom_headers: dict[str, str], include_requests_detail: bool = False, + include_best_practices: bool = False, ) -> QueueTaskResult: try: settings = Settings() @@ -91,6 +94,7 @@ async def async_ecoindex_task( screenshot_gid=settings.SCREENSHOTS_GID, screenshot_uid=settings.SCREENSHOTS_UID, custom_headers=custom_headers, + best_practices=include_best_practices, ) ecoindex = await scraper.get_page_analysis() request_details = ( @@ -101,6 +105,9 @@ async def async_ecoindex_task( if include_requests_detail else None ) + best_practices_report = ( + await scraper.get_best_practices() if include_best_practices else None + ) if screenshot: persist_screenshot(screenshot=screenshot, version=Version.v1.value) @@ -110,6 +117,7 @@ async def async_ecoindex_task( id=task_id, ecoindex_result=ecoindex, requests=request_details, + best_practices=best_practices_report, ) return QueueTaskResult(status=TaskStatus.SUCCESS, detail=db_result) diff --git a/components/ecoindex/database/engine.py b/components/ecoindex/database/engine.py index ff8cb8a..42ec4f7 100644 --- a/components/ecoindex/database/engine.py +++ b/components/ecoindex/database/engine.py @@ -1,7 +1,12 @@ from typing import AsyncGenerator from ecoindex.config import Settings -from ecoindex.database.models import ApiEcoindex, ApiEcoindexRequest # noqa: F401 +from ecoindex.database.models import ( # noqa: F401 + ApiEcoindex, + ApiEcoindexBestPractice, + ApiEcoindexBestPracticeResult, + ApiEcoindexRequest, +) from ecoindex.models.api import * # noqa: F401, F403 from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from sqlalchemy.pool import NullPool diff --git a/components/ecoindex/database/models/__init__.py b/components/ecoindex/database/models/__init__.py index 77b5ec3..a49c93f 100644 --- a/components/ecoindex/database/models/__init__.py +++ b/components/ecoindex/database/models/__init__.py @@ -2,30 +2,31 @@ from ecoindex.models.compute import Result from ecoindex.models.scraper import RequestDetail -from pydantic import BaseModel -from sqlalchemy import Column, Text -from sqlmodel import Field, SQLModel +from pydantic import BaseModel, Field +from sqlalchemy import JSON, Column, Text, UniqueConstraint +from sqlmodel import Field as SQLField +from sqlmodel import SQLModel class ApiEcoindex(SQLModel, Result, table=True): # type: ignore - id: UUID | None = Field( + id: UUID | None = SQLField( default=None, description="Analysis ID of type `UUID`", primary_key=True, index=True, ) - host: str = Field( + host: str = SQLField( default=..., title="Web page host", description="Host name of the web page", index=True, ) - version: int = Field( + version: int = SQLField( default=1, title="API version", description="Version number of the API used to run the test", ) - initial_ranking: int | None = Field( + initial_ranking: int | None = SQLField( default=..., title="Analysis rank", description=( @@ -34,7 +35,7 @@ class ApiEcoindex(SQLModel, Result, table=True): # type: ignore "time of the analysis for a given version." ), ) - initial_total_results: int | None = Field( + initial_total_results: int | None = SQLField( default=..., title="Total number of analysis", description=( @@ -43,7 +44,7 @@ class ApiEcoindex(SQLModel, Result, table=True): # type: ignore "at the time of the analysis for a given version." ), ) - source: str | None = Field( + source: str | None = SQLField( default="ecoindex.fr", title="Source of the analysis", description="Source of the analysis", @@ -51,45 +52,195 @@ class ApiEcoindex(SQLModel, Result, table=True): # type: ignore class ApiEcoindexRequest(SQLModel, table=True): - id: UUID = Field( + id: UUID = SQLField( default_factory=uuid4, primary_key=True, description="Request detail ID of type `UUID`", ) - analysis_id: UUID = Field( + analysis_id: UUID = SQLField( default=..., foreign_key="apiecoindex.id", index=True, description="ID of the related ecoindex analysis", ) - category: str = Field( + category: str = SQLField( default=..., title="Request category", description="Category of the resource (html, css, javascript, image, ...)", ) - domain: str = Field( + domain: str = SQLField( default=..., title="Request domain", description="Domain that served the resource", ) - status: int = Field( + status: int = SQLField( default=..., title="HTTP status", description="HTTP status code of the resource response", ) - url: str = Field( + url: str = SQLField( default=..., sa_column=Column(Text(), nullable=False), title="Request URL", description="URL of the resource without query parameters", ) - size: float = Field( + size: float = SQLField( default=..., title="Request size", description="Transfer size of the resource in bytes", ) +class ApiEcoindexBestPractice(SQLModel, table=True): + __tablename__ = "apiecoindexbestpractices" + __table_args__ = ( + UniqueConstraint("rule_id", name="uq_apiecoindexbestpractices_rule_id"), + ) + + id: UUID = SQLField( + default_factory=uuid4, + primary_key=True, + description="Best practice ID of type `UUID`", + ) + rule_id: str = SQLField( + default=..., + index=True, + title="Rule ID", + description="Stable rule identifier from the YAML config (e.g. http_requests)", + ) + rweb_id: str | None = SQLField( + default=None, + title="RWEB ID", + description="Official RWEB reference (e.g. RWEB_0047)", + ) + category: str = SQLField( + default=..., + title="Category", + description="RWEB tier / category (network, user-device, ...)", + ) + title: str = SQLField( + default=..., + title="Title", + description="Official best practice title", + ) + description: str = SQLField( + default="", + sa_column=Column(Text(), nullable=False, server_default=""), + title="Description", + description="Best practice description", + ) + url: str | None = SQLField( + default=None, + sa_column=Column(Text(), nullable=True), + title="RWEB URL", + description="URL of the official RWEB fiche", + ) + enabled: bool = SQLField( + default=True, + title="Enabled", + description="Whether the rule is currently enabled in the catalog", + ) + threshold_warn: float | None = SQLField( + default=None, + title="Warn threshold", + description="Current warn threshold from the YAML config", + ) + threshold_fail: float | None = SQLField( + default=None, + title="Fail threshold", + description="Current fail threshold from the YAML config (RWEB maxValue)", + ) + higher_is_worse: bool = SQLField( + default=True, + title="Higher is worse", + description="Whether higher measured values are worse for this rule", + ) + + +class ApiEcoindexBestPracticeResult(SQLModel, table=True): + __tablename__ = "apiecoindexbestpracticeresults" + __table_args__ = ( + UniqueConstraint( + "analysis_id", + "best_practice_id", + name="uq_apiecoindexbestpracticeresults_analysis_practice", + ), + ) + + id: UUID = SQLField( + default_factory=uuid4, + primary_key=True, + description="Best practice result ID of type `UUID`", + ) + analysis_id: UUID = SQLField( + default=..., + foreign_key="apiecoindex.id", + index=True, + description="ID of the related ecoindex analysis", + ) + best_practice_id: UUID = SQLField( + default=..., + foreign_key="apiecoindexbestpractices.id", + index=True, + description="ID of the related best practice definition", + ) + status: str = SQLField( + default=..., + title="Status", + description="Evaluation status: ok, warn or fail", + ) + value: float = SQLField( + default=..., + title="Measured value", + description="Measured value used for the rule evaluation", + ) + threshold_warn: float | None = SQLField( + default=None, + title="Warn threshold snapshot", + description="Warn threshold at analysis time", + ) + threshold_fail: float | None = SQLField( + default=None, + title="Fail threshold snapshot", + description="Fail threshold at analysis time", + ) + message: str = SQLField( + default=..., + sa_column=Column(Text(), nullable=False), + title="Message", + description="Human-readable evaluation message", + ) + details: list[str] = SQLField( + default_factory=list, + sa_column=Column(JSON, nullable=False), + title="Details", + description="Optional list of detail strings (URLs, domains, ...)", + ) + + +class BestPracticeResultItem(BaseModel): + id: UUID | None = None + rule_id: str + rweb_id: str | None = None + category: str + title: str + description: str = "" + url: str | None = None + status: str + value: float + threshold_warn: float | None = None + threshold_fail: float | None = None + message: str + details: list[str] = Field(default_factory=list) + + +class BestPracticesAnalysisResponse(BaseModel): + results: list[BestPracticeResultItem] = Field(default_factory=list) + ok_count: int = 0 + warn_count: int = 0 + fail_count: int = 0 + + class ApiEcoindexBatchItem(Result): id: UUID | None = None host: str diff --git a/components/ecoindex/database/repositories/best_practices.py b/components/ecoindex/database/repositories/best_practices.py new file mode 100644 index 0000000..e8928f9 --- /dev/null +++ b/components/ecoindex/database/repositories/best_practices.py @@ -0,0 +1,75 @@ +from ecoindex.best_practices import BestPracticesReport, load_rules_config +from ecoindex.database.models import ( + ApiEcoindexBestPractice, + ApiEcoindexBestPracticeResult, +) +from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession + + +async def sync_best_practices_catalog( + session: AsyncSession, +) -> dict[str, ApiEcoindexBestPractice]: + """Upsert best practice definitions from the default YAML config.""" + config = load_rules_config() + result = await session.exec(select(ApiEcoindexBestPractice)) + existing = {row.rule_id: row for row in result.all()} + + for rule in config.rules: + url = str(rule.url) if rule.url is not None else None + if rule.id in existing: + practice = existing[rule.id] + practice.rweb_id = rule.rweb_id + practice.category = rule.category.value + practice.title = rule.title + practice.description = rule.description + practice.url = url + practice.enabled = rule.enabled + practice.threshold_warn = rule.thresholds.warn + practice.threshold_fail = rule.thresholds.fail + practice.higher_is_worse = rule.higher_is_worse + session.add(practice) + else: + practice = ApiEcoindexBestPractice( + rule_id=rule.id, + rweb_id=rule.rweb_id, + category=rule.category.value, + title=rule.title, + description=rule.description, + url=url, + enabled=rule.enabled, + threshold_warn=rule.thresholds.warn, + threshold_fail=rule.thresholds.fail, + higher_is_worse=rule.higher_is_worse, + ) + session.add(practice) + existing[rule.id] = practice + + await session.flush() + return existing + + +def build_best_practice_result_rows( + *, + analysis_id, + report: BestPracticesReport, + catalog: dict[str, ApiEcoindexBestPractice], +) -> list[ApiEcoindexBestPracticeResult]: + rows: list[ApiEcoindexBestPracticeResult] = [] + for rule_result in report.results: + practice = catalog.get(rule_result.id) + if practice is None or practice.id is None: + continue + rows.append( + ApiEcoindexBestPracticeResult( + analysis_id=analysis_id, + best_practice_id=practice.id, + status=rule_result.status.value, + value=rule_result.value, + threshold_warn=rule_result.thresholds.warn, + threshold_fail=rule_result.thresholds.fail, + message=rule_result.message, + details=list(rule_result.details), + ) + ) + return rows diff --git a/components/ecoindex/database/repositories/ecoindex.py b/components/ecoindex/database/repositories/ecoindex.py index f72d438..b289f97 100644 --- a/components/ecoindex/database/repositories/ecoindex.py +++ b/components/ecoindex/database/repositories/ecoindex.py @@ -3,7 +3,14 @@ from uuid import UUID from ecoindex.database.helper import date_filter -from ecoindex.database.models import ApiEcoindex, ApiEcoindexRequest +from ecoindex.database.models import ( + ApiEcoindex, + ApiEcoindexBestPractice, + ApiEcoindexBestPracticeResult, + ApiEcoindexRequest, + BestPracticeResultItem, + BestPracticesAnalysisResponse, +) from ecoindex.models import Result from ecoindex.models.enums import Version from ecoindex.models.sort import Sort @@ -111,6 +118,58 @@ async def get_requests_by_analysis_id_db( return list(result.all()) +async def get_best_practice_results_by_analysis_id_db( + session: AsyncSession, analysis_id: UUID +) -> BestPracticesAnalysisResponse | None: + statement = ( + select(ApiEcoindexBestPracticeResult, ApiEcoindexBestPractice) + .join( + ApiEcoindexBestPractice, + ApiEcoindexBestPractice.id + == ApiEcoindexBestPracticeResult.best_practice_id, + ) + .where(ApiEcoindexBestPracticeResult.analysis_id == analysis_id) + ) + result = await session.exec(statement) + rows = list(result.all()) + if not rows: + return None + + items: list[BestPracticeResultItem] = [] + ok_count = warn_count = fail_count = 0 + for result_row, practice in rows: + items.append( + BestPracticeResultItem( + id=result_row.id, + rule_id=practice.rule_id, + rweb_id=practice.rweb_id, + category=practice.category, + title=practice.title, + description=practice.description, + url=practice.url, + status=result_row.status, + value=result_row.value, + threshold_warn=result_row.threshold_warn, + threshold_fail=result_row.threshold_fail, + message=result_row.message, + details=list(result_row.details or []), + ) + ) + if result_row.status == "ok": + ok_count += 1 + elif result_row.status == "warn": + warn_count += 1 + elif result_row.status == "fail": + fail_count += 1 + + return BestPracticesAnalysisResponse( + results=items, + ok_count=ok_count, + warn_count=warn_count, + fail_count=fail_count, + ) + + async def get_count_daily_request_per_host(session: AsyncSession, host: str) -> int: statement = select(ApiEcoindex).where( func.date(ApiEcoindex.date) == date.today(), ApiEcoindex.host == host diff --git a/components/ecoindex/database/repositories/worker.py b/components/ecoindex/database/repositories/worker.py index 695fd5b..78f8e03 100644 --- a/components/ecoindex/database/repositories/worker.py +++ b/components/ecoindex/database/repositories/worker.py @@ -1,6 +1,11 @@ from uuid import UUID +from ecoindex.best_practices import BestPracticesReport from ecoindex.database.models import ApiEcoindex, ApiEcoindexRequest +from ecoindex.database.repositories.best_practices import ( + build_best_practice_result_rows, + sync_best_practices_catalog, +) from ecoindex.database.repositories.ecoindex import ( get_count_analysis_db, get_rank_analysis_db, @@ -18,6 +23,7 @@ async def save_ecoindex_result_db( version: Version = Version.v1, source: str | None = None, requests: list[RequestDetail] | None = None, + best_practices: BestPracticesReport | None = None, ) -> ApiEcoindex: ranking = await get_rank_analysis_db( session=session, ecoindex=ecoindex_result, version=version @@ -61,6 +67,15 @@ async def save_ecoindex_result_db( for item in requests ] ) + if best_practices is not None and best_practices.results: + catalog = await sync_best_practices_catalog(session=session) + session.add_all( + build_best_practice_result_rows( + analysis_id=id, + report=best_practices, + catalog=catalog, + ) + ) try: await session.commit() await session.refresh(db_ecoindex) diff --git a/projects/ecoindex_api/alembic/env.py b/projects/ecoindex_api/alembic/env.py index 6358739..e82c33a 100644 --- a/projects/ecoindex_api/alembic/env.py +++ b/projects/ecoindex_api/alembic/env.py @@ -3,7 +3,12 @@ from alembic import context from ecoindex.config import Settings -from ecoindex.database.models import ApiEcoindex, ApiEcoindexRequest # noqa: F401 +from ecoindex.database.models import ( # noqa: F401 + ApiEcoindex, + ApiEcoindexBestPractice, + ApiEcoindexBestPracticeResult, + ApiEcoindexRequest, +) from ecoindex.models.api import * # noqa: F403 from sqlalchemy import pool from sqlalchemy.engine import Connection diff --git a/projects/ecoindex_api/alembic/versions/a1b2c3d4e5f6_add_best_practices_tables.py b/projects/ecoindex_api/alembic/versions/a1b2c3d4e5f6_add_best_practices_tables.py new file mode 100644 index 0000000..b501535 --- /dev/null +++ b/projects/ecoindex_api/alembic/versions/a1b2c3d4e5f6_add_best_practices_tables.py @@ -0,0 +1,148 @@ +"""Add best practices catalog and results tables + +Revision ID: a1b2c3d4e5f6 +Revises: c3e8f1a90b12 +Create Date: 2026-10-02 13:20:00.000000 + +""" + +import sqlalchemy as sa +import sqlmodel +from alembic import op +from ecoindex.database.helper import index_exists, table_exists + +revision = "a1b2c3d4e5f6" +down_revision = "c3e8f1a90b12" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + if not table_exists(op.get_bind(), "apiecoindexbestpractices"): + op.create_table( + "apiecoindexbestpractices", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("rule_id", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("rweb_id", sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column("category", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("title", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column( + "description", + sa.Text(), + nullable=False, + server_default="", + ), + sa.Column("url", sa.Text(), nullable=True), + sa.Column("enabled", sa.Boolean(), nullable=False, server_default=sa.true()), + sa.Column("threshold_warn", sa.Float(), nullable=True), + sa.Column("threshold_fail", sa.Float(), nullable=True), + sa.Column( + "higher_is_worse", + sa.Boolean(), + nullable=False, + server_default=sa.true(), + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "rule_id", name="uq_apiecoindexbestpractices_rule_id" + ), + ) + + if not index_exists( + op.get_bind(), "apiecoindexbestpractices", "ix_apiecoindexbestpractices_rule_id" + ): + op.create_index( + op.f("ix_apiecoindexbestpractices_rule_id"), + "apiecoindexbestpractices", + ["rule_id"], + unique=False, + ) + + if not table_exists(op.get_bind(), "apiecoindexbestpracticeresults"): + op.create_table( + "apiecoindexbestpracticeresults", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("analysis_id", sa.Uuid(), nullable=False), + sa.Column("best_practice_id", sa.Uuid(), nullable=False), + sa.Column("status", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("value", sa.Float(), nullable=False), + sa.Column("threshold_warn", sa.Float(), nullable=True), + sa.Column("threshold_fail", sa.Float(), nullable=True), + sa.Column("message", sa.Text(), nullable=False), + sa.Column("details", sa.JSON(), nullable=False), + sa.ForeignKeyConstraint( + ["analysis_id"], + ["apiecoindex.id"], + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["best_practice_id"], + ["apiecoindexbestpractices.id"], + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "analysis_id", + "best_practice_id", + name="uq_apiecoindexbestpracticeresults_analysis_practice", + ), + ) + + if not index_exists( + op.get_bind(), + "apiecoindexbestpracticeresults", + "ix_apiecoindexbestpracticeresults_analysis_id", + ): + op.create_index( + op.f("ix_apiecoindexbestpracticeresults_analysis_id"), + "apiecoindexbestpracticeresults", + ["analysis_id"], + unique=False, + ) + + if not index_exists( + op.get_bind(), + "apiecoindexbestpracticeresults", + "ix_apiecoindexbestpracticeresults_best_practice_id", + ): + op.create_index( + op.f("ix_apiecoindexbestpracticeresults_best_practice_id"), + "apiecoindexbestpracticeresults", + ["best_practice_id"], + unique=False, + ) + + +def downgrade() -> None: + if index_exists( + op.get_bind(), + "apiecoindexbestpracticeresults", + "ix_apiecoindexbestpracticeresults_best_practice_id", + ): + op.drop_index( + op.f("ix_apiecoindexbestpracticeresults_best_practice_id"), + table_name="apiecoindexbestpracticeresults", + ) + + if index_exists( + op.get_bind(), + "apiecoindexbestpracticeresults", + "ix_apiecoindexbestpracticeresults_analysis_id", + ): + op.drop_index( + op.f("ix_apiecoindexbestpracticeresults_analysis_id"), + table_name="apiecoindexbestpracticeresults", + ) + + if table_exists(op.get_bind(), "apiecoindexbestpracticeresults"): + op.drop_table("apiecoindexbestpracticeresults") + + if index_exists( + op.get_bind(), "apiecoindexbestpractices", "ix_apiecoindexbestpractices_rule_id" + ): + op.drop_index( + op.f("ix_apiecoindexbestpractices_rule_id"), + table_name="apiecoindexbestpractices", + ) + + if table_exists(op.get_bind(), "apiecoindexbestpractices"): + op.drop_table("apiecoindexbestpractices") diff --git a/test/components/ecoindex/database/test_repository_queries.py b/test/components/ecoindex/database/test_repository_queries.py index 3f5a9f6..e0c4280 100644 --- a/test/components/ecoindex/database/test_repository_queries.py +++ b/test/components/ecoindex/database/test_repository_queries.py @@ -20,6 +20,9 @@ def __init__(self, value: int): def one(self) -> int: return self.value + def all(self) -> list: + return [] + class FakeListResult: def __init__(self, value: list): @@ -63,6 +66,9 @@ async def rollback(self) -> None: async def close(self) -> None: self.closed = True + async def flush(self) -> None: + return None + @pytest.mark.asyncio async def test_get_count_analysis_db_parameterizes_host(): @@ -183,3 +189,94 @@ async def fake_count(*_args, **_kwargs): assert request_rows[0].url == "https://cdn.example.com/app.js" assert request_rows[0].domain == "cdn.example.com" assert request_rows[0].category == "javascript" + + +@pytest.mark.asyncio +async def test_save_ecoindex_result_db_persists_best_practices(monkeypatch): + from ecoindex.best_practices.models import ( + BestPracticesReport, + RuleCategory, + RuleResult, + RuleStatus, + Thresholds, + ) + from ecoindex.database.models import ( + ApiEcoindexBestPractice, + ApiEcoindexBestPracticeResult, + ) + + analysis_id = uuid4() + session = FakeSession() + + async def fake_rank(*_args, **_kwargs): + return 1 + + async def fake_count(*_args, **_kwargs): + return 1 + + async def fake_flush() -> None: + for item in session.added: + if isinstance(item, ApiEcoindexBestPractice) and getattr(item, "id", None): + continue + if isinstance(item, ApiEcoindexBestPractice): + item.id = uuid4() + + monkeypatch.setattr( + "ecoindex.database.repositories.worker.get_rank_analysis_db", + fake_rank, + ) + monkeypatch.setattr( + "ecoindex.database.repositories.worker.get_count_analysis_db", + fake_count, + ) + session.flush = fake_flush # type: ignore[method-assign] + + report = BestPracticesReport( + results=[ + RuleResult( + id="http_requests", + category=RuleCategory.network, + title="Limit the number of HTTP requests", + rweb_id="RWEB_0047", + status=RuleStatus.ok, + value=5, + thresholds=Thresholds(warn=26, fail=40), + message="5 HTTP request(s)", + details=[], + ) + ] + ) + + await save_ecoindex_result_db( + session=session, + id=analysis_id, + ecoindex_result=Result( + size=119, + nodes=45, + requests=2, + url="https://www.ecoindex.fr", + width=1920, + height=1080, + grade="A", + score=89, + ges=1.22, + water=1.89, + ), + best_practices=report, + ) + + practices = [ + item for item in session.added if isinstance(item, ApiEcoindexBestPractice) + ] + results = [ + item + for item in session.added + if isinstance(item, ApiEcoindexBestPracticeResult) + ] + assert len(practices) >= 1 + assert any(p.rule_id == "http_requests" for p in practices) + assert len(results) == 1 + assert results[0].analysis_id == analysis_id + assert results[0].status == "ok" + assert results[0].value == 5 + assert results[0].threshold_fail == 40 From 55add7fe8dc4eb20936a062a36b3f21b1be686fd Mon Sep 17 00:00:00 2001 From: Vincent Vatelot Date: Mon, 5 Oct 2026 10:00:54 +0200 Subject: [PATCH 2/2] fix(api): use FK-inferred join for best practices query Avoid an explicit column equality onclause that ty types as bool. Co-authored-by: Cursor --- components/ecoindex/database/repositories/ecoindex.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/ecoindex/database/repositories/ecoindex.py b/components/ecoindex/database/repositories/ecoindex.py index b289f97..52ac6c4 100644 --- a/components/ecoindex/database/repositories/ecoindex.py +++ b/components/ecoindex/database/repositories/ecoindex.py @@ -123,11 +123,7 @@ async def get_best_practice_results_by_analysis_id_db( ) -> BestPracticesAnalysisResponse | None: statement = ( select(ApiEcoindexBestPracticeResult, ApiEcoindexBestPractice) - .join( - ApiEcoindexBestPractice, - ApiEcoindexBestPractice.id - == ApiEcoindexBestPracticeResult.best_practice_id, - ) + .join(ApiEcoindexBestPractice) .where(ApiEcoindexBestPracticeResult.analysis_id == analysis_id) ) result = await session.exec(statement)