From fecd13c12f4e7bb5b969d8e8852ae4c17b75b7b3 Mon Sep 17 00:00:00 2001 From: nicowre Date: Wed, 1 Jul 2026 10:45:31 +0200 Subject: [PATCH 1/3] feat(templates): file upload for template icons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neue Funktion, mit der Owner/Admins ein Icon-Bild für ein Template hochladen können. Bislang lief ``icon_url`` als reiner String — externe URL, ``mdi:*``/``fa:*``-Identifier oder Emoji. Für hochgeladene Assets gab es keinen Weg. Design: - Neue Tabelle ``template_icons`` mit BYTEA-Content (Bild-Bytes), ``content_type``, ``file_name``, ``size_bytes``. 1:1 zu Templates via unique FK + ON DELETE CASCADE. - ``content``-Spalte ist SQLAlchemy-``deferred`` — Blob wird nur beim Serve-Endpoint aus der DB gezogen, nie bei normalen Template-Queries. - Response bekommt ``effective_icon`` (computed): hochgeladenes Icon → ``/api/v1/templates/{id}/icon``, sonst Fallback auf ``icon_url``, sonst ``None``. Rohfeld ``icon_url`` bleibt sichtbar für Edit-UIs. ``has_uploaded_icon`` als billiges Signal fürs Frontend. - Drei neue Endpoints: POST /templates/{id}/icon (multipart, owner-or-admin) GET /templates/{id}/icon (Serve, Sichtbarkeits-Gate wie GET Template) DELETE /templates/{id}/icon (owner-or-admin, idempotent) - Validierung im Service: PNG/JPEG/WebP whitelist (415 sonst), max 5 MB (413 sonst), leere Uploads → 400. Grenzwerte via ``settings.max_icon_size_bytes`` / ``allowed_icon_content_types``. - ``POST /templates`` und ``PATCH /templates`` bleiben pure-JSON — keine Breaking Changes an bestehenden Aufrufen. Migration ``c8a3f1e9b7d5`` legt die neue Tabelle an. ``icon_url`` auf ``templates`` bleibt unverändert. Tests: 23 Unit-Tests (Service + Schema), 14 API-Tests (Routes inkl. Cascade-Delete). Ruff/Mypy grün. --- ...8a3f1e9b7d5_create_template_icons_table.py | 76 ++++ pyproject.toml | 1 + src/api/templates.py | 127 ++++++- src/core/config.py | 10 + src/models/__init__.py | 2 + src/models/template.py | 14 + src/models/template_icon.py | 71 ++++ src/repositories/template_icon_repository.py | 35 ++ src/schemas/template.py | 30 ++ src/services/template_icon_service.py | 194 ++++++++++ tests/api/test_template_icon_routes.py | 334 +++++++++++++++++ .../test_template_effective_icon_schema.py | 88 +++++ tests/unit/test_template_icon_service.py | 353 ++++++++++++++++++ uv.lock | 11 + 14 files changed, 1345 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py create mode 100644 src/models/template_icon.py create mode 100644 src/repositories/template_icon_repository.py create mode 100644 src/services/template_icon_service.py create mode 100644 tests/api/test_template_icon_routes.py create mode 100644 tests/unit/test_template_effective_icon_schema.py create mode 100644 tests/unit/test_template_icon_service.py diff --git a/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py b/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py new file mode 100644 index 0000000..80c7bb1 --- /dev/null +++ b/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py @@ -0,0 +1,76 @@ +"""create template_icons table + +Revision ID: c8a3f1e9b7d5 +Revises: e2a91d05c7b8 +Create Date: 2026-07-01 09:00:00.000000 + +Neue Tabelle für hochgeladene Template-Icons. Bilder werden als BYTEA +persistiert, ``template_id`` ist unique (1:1 Beziehung Template → Icon) +und ``ON DELETE CASCADE`` räumt das Icon auf, wenn das Template selbst +gelöscht wird. ``icon_url`` auf ``templates`` bleibt unverändert +(externe URLs, ``mdi:*``-Identifier usw.); die Response-Aggregation im +Schema entscheidet, ob das hochgeladene Icon oder ``icon_url`` an das +Frontend gegeben wird. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + + +revision: str = 'c8a3f1e9b7d5' +down_revision: Union[str, Sequence[str], None] = 'e2a91d05c7b8' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Create template_icons table.""" + op.create_table( + 'template_icons', + sa.Column('id', sa.String(length=36), nullable=False), + sa.Column( + 'template_id', + sa.String(length=36), + nullable=False, + comment='Owning template — 1:1, jedes Template hat höchstens ein Icon.', + ), + sa.Column( + 'content', + sa.LargeBinary(), + nullable=False, + comment='Rohbytes des Bildes (PNG/JPEG/WebP).', + ), + sa.Column( + 'content_type', + sa.String(length=64), + nullable=False, + comment='MIME-Typ, wird beim Ausliefern als Content-Type-Header verwendet.', + ), + sa.Column( + 'file_name', + sa.String(length=255), + nullable=True, + comment='Original-Dateiname (für Content-Disposition).', + ), + sa.Column( + 'size_bytes', + sa.Integer(), + nullable=False, + comment='Größe von content in Bytes.', + ), + sa.Column('created_at', sa.DateTime(timezone=True), nullable=False), + sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint( + ['template_id'], + ['templates.id'], + ondelete='CASCADE', + ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('template_id', name='uq_template_icons_template_id'), + ) + + +def downgrade() -> None: + """Drop template_icons table.""" + op.drop_table('template_icons') diff --git a/pyproject.toml b/pyproject.toml index b96b2e2..cf7e1fc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,7 @@ dependencies = [ "psycopg2-binary>=2.9.0", "alembic>=1.13.0", "python-jose[cryptography]>=3.3.0", + "python-multipart>=0.0.9", "httpx>=0.25.0", "openstacksdk>=3.3.0", "python-heatclient>=3.5.0", diff --git a/src/api/templates.py b/src/api/templates.py index bdb1764..9f86a46 100644 --- a/src/api/templates.py +++ b/src/api/templates.py @@ -2,7 +2,8 @@ from typing import Optional from uuid import UUID -from fastapi import APIRouter, status, Query, Depends +from fastapi import APIRouter, Depends, File, Query, UploadFile, status +from fastapi.responses import Response from src.core.response_builder import ResponseBuilder from src.core.dependencies import DBSession, RequestID, Pagination, require_roles, CurrentUser @@ -15,6 +16,7 @@ ) from src.schemas.template_version import TemplateVersionResponse from src.services.template_service import TemplateService +from src.services.template_icon_service import TemplateIconService from src.services.github_import_service import GithubImportService from src.models.user import UserRole from src.models.template import TemplateVisibility @@ -286,3 +288,126 @@ async def import_new_version_from_github( message="Template version imported from GitHub successfully", request_id=request_id, ) + + +# --------------------------------------------------------------------------- +# Icon-Upload +# --------------------------------------------------------------------------- +# +# Der Upload lebt bewusst auf einem eigenen Endpoint statt am POST/PATCH +# /templates, damit die JSON-API rein-JSON bleibt und Clients ohne +# Anpassung weiterlaufen. Die drei Endpoints (POST/GET/DELETE) sind +# symmetrisch und respektieren die Standard-Sichtbarkeitsregeln: +# Admin darf alles, Owner darf sein eigenes, Fremde nur PUBLIC-Templates +# mit mindestens einer APPROVED Version (letzteres nur für den Serve- +# Endpoint — Upload/Delete sind owner-or-admin-only). + + +@router.post( + "/{template_id}/icon", + status_code=status.HTTP_201_CREATED, + response_model=None, +) +async def upload_template_icon( + template_id: UUID, + db: DBSession, + request_id: RequestID, + current_user: CurrentUser, + file: UploadFile = File(..., description="Icon image (PNG, JPEG or WebP, max 5 MB)"), +): + """Upload (or replace) the icon image for a template. + + Owner-or-admin-only. Erlaubte Formate: ``image/png``, ``image/jpeg``, + ``image/webp``; maximale Größe: 5 MB (konfigurierbar via + ``settings.max_icon_size_bytes``). + + Der Endpoint speichert die Bytes in der Tabelle ``template_icons`` und + setzt in der Template-Response ab sofort ``effective_icon`` auf + ``/api/v1/templates/{id}/icon`` — d.h. das Frontend braucht nur eine + URL zu rendern, egal ob externes ``icon_url`` (``mdi:*``, externe URL) + oder hochgeladenes Bild. + """ + is_admin = UserRole.ADMIN.value in current_user.get("roles", []) + content = await file.read() + service = TemplateIconService(db) + icon = service.upload_icon( + template_id=str(template_id), + content=content, + content_type=file.content_type or "application/octet-stream", + file_name=file.filename, + user_id=current_user["user_id"], + is_admin=is_admin, + ) + return ResponseBuilder.created( + data={ + "id": icon.id, + "template_id": icon.template_id, + "content_type": icon.content_type, + "file_name": icon.file_name, + "size_bytes": icon.size_bytes, + "url": f"/api/v1/templates/{icon.template_id}/icon", + }, + message="Template icon uploaded successfully", + request_id=request_id, + ) + + +@router.get("/{template_id}/icon") +async def get_template_icon( + template_id: UUID, + db: DBSession, + current_user: CurrentUser, +): + """Return the raw icon bytes for a template. + + Same visibility rules as GET /templates/{id}: admin sees everything, + owner sees their own, others only PUBLIC templates with at least one + APPROVED version. Sends the stored MIME type as ``Content-Type`` and + an ETag derived from the icon row ID for browser caching. + """ + is_admin = UserRole.ADMIN.value in current_user.get("roles", []) + service = TemplateIconService(db) + icon = service.get_icon( + template_id=str(template_id), + user_id=current_user["user_id"], + is_admin=is_admin, + ) + headers = { + # 5 Minuten private cache reichen — das Icon ändert sich selten, + # aber ein PATCH sollte binnen kurzer Zeit sichtbar sein. + "Cache-Control": "private, max-age=300", + "ETag": f'"{icon.id}"', + } + if icon.file_name: + headers["Content-Disposition"] = f'inline; filename="{icon.file_name}"' + return Response( + content=icon.content, + media_type=icon.content_type, + headers=headers, + ) + + +@router.delete( + "/{template_id}/icon", + status_code=status.HTTP_204_NO_CONTENT, +) +async def delete_template_icon( + template_id: UUID, + db: DBSession, + current_user: CurrentUser, +): + """Remove the uploaded icon for a template. + + Owner-or-admin-only. Idempotent: wenn kein Icon existiert, ist die + Antwort trotzdem 204 (Client muss nicht wissen, ob vorher eins da war). + ``icon_url`` bleibt unverändert und wird nach dem Löschen wieder das + ``effective_icon``, falls gesetzt. + """ + is_admin = UserRole.ADMIN.value in current_user.get("roles", []) + service = TemplateIconService(db) + service.delete_icon( + template_id=str(template_id), + user_id=current_user["user_id"], + is_admin=is_admin, + ) + return None diff --git a/src/core/config.py b/src/core/config.py index 12943f0..705387d 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -59,6 +59,16 @@ def ansible_ssh_private_key(self) -> str | None: github_app_state_secret: str | None = None frontend_base_url: str = "http://localhost:5173" + # Template icon uploads. Grenzwerte werden im Service gegen die + # hochgeladene Datei geprüft — 5 MB und PNG/JPEG/WebP sind das + # abgestimmte Default. + max_icon_size_bytes: int = 5 * 1024 * 1024 + allowed_icon_content_types: tuple[str, ...] = ( + "image/png", + "image/jpeg", + "image/webp", + ) + @property def database_url(self) -> str: diff --git a/src/models/__init__.py b/src/models/__init__.py index 1ab935e..46486f5 100644 --- a/src/models/__init__.py +++ b/src/models/__init__.py @@ -14,6 +14,7 @@ from src.models.template import Template from src.models.template_category import TemplateCategory from src.models.template_category_assignment import TemplateCategoryAssignment +from src.models.template_icon import TemplateIcon from src.models.template_version import TemplateVersion from src.models.template_version_file import TemplateVersionFile from src.models.user import User @@ -34,6 +35,7 @@ "Template", "TemplateCategory", "TemplateCategoryAssignment", + "TemplateIcon", "TemplateVersion", "TemplateVersionFile", "User", diff --git a/src/models/template.py b/src/models/template.py index d676a6d..057e476 100644 --- a/src/models/template.py +++ b/src/models/template.py @@ -60,4 +60,18 @@ class Template(Base): ) category_assignments: Mapped[list["TemplateCategoryAssignment"]] = relationship("TemplateCategoryAssignment", back_populates="template") + # Hochgeladenes Icon-Bild (optional). Getrennte Tabelle statt Spalte am + # Template, damit ``SELECT * FROM templates`` keinen 1-5 MB BLOB pro Row + # mitlädt. ``uselist=False`` weil per Unique-Constraint auf + # ``template_icons.template_id`` maximal ein Icon pro Template existiert. + # Die ``content``-Spalte auf ``TemplateIcon`` ist ``deferred``, wird also + # nur beim Serve-Endpoint tatsächlich aus der DB gezogen. + icon: Mapped["TemplateIcon | None"] = relationship( + "TemplateIcon", + back_populates="template", + cascade="all, delete-orphan", + passive_deletes=True, + uselist=False, + ) + diff --git a/src/models/template_icon.py b/src/models/template_icon.py new file mode 100644 index 0000000..fdbbb5e --- /dev/null +++ b/src/models/template_icon.py @@ -0,0 +1,71 @@ +"""Template Icon database model. + +Speichert hochgeladene Icon-Bilder als Binärdaten in einer eigenen Tabelle, +damit große BLOBs nicht in jedem ``SELECT * FROM templates`` mitgeschleppt +werden. Ein Template hat maximal ein Icon (1:0..1 Beziehung, via unique FK +auf ``templates.icon_file_id``); Cascade-Delete räumt die Row auf, wenn das +Template selbst gelöscht wird. + +Zulässige Bildformate und die Größenobergrenze werden im Service-Layer +validiert (siehe ``template_icon_service.py``), nicht in der DB. +""" +from datetime import datetime, timezone +from typing import TYPE_CHECKING +from uuid import uuid4 + +from sqlalchemy import DateTime, ForeignKey, Integer, LargeBinary, String +from sqlalchemy.orm import Mapped, deferred, mapped_column, relationship + +from src.core.database import Base + +if TYPE_CHECKING: + from src.models.template import Template + + +class TemplateIcon(Base): + """Persistiertes Icon-Bild für ein Template.""" + + __tablename__ = "template_icons" + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=lambda: str(uuid4())) + template_id: Mapped[str] = mapped_column( + String(36), + ForeignKey("templates.id", ondelete="CASCADE"), + nullable=False, + unique=True, + comment="Owning template — 1:1, jedes Template hat höchstens ein Icon.", + ) + content: Mapped[bytes] = deferred( + mapped_column( + LargeBinary, + nullable=False, + comment="Rohbytes des Bildes (PNG/JPEG/WebP).", + ) + ) + content_type: Mapped[str] = mapped_column( + String(64), + nullable=False, + comment="MIME-Typ, wird beim Ausliefern als Content-Type-Header verwendet.", + ) + file_name: Mapped[str | None] = mapped_column( + String(255), + nullable=True, + comment="Original-Dateiname (für Content-Disposition).", + ) + size_bytes: Mapped[int] = mapped_column( + Integer, + nullable=False, + comment="Größe von ``content`` in Bytes — redundant, aber praktisch für Listing/Debug.", + ) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(timezone.utc), + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(timezone.utc), + onupdate=lambda: datetime.now(timezone.utc), + ) + + # Relationships + template: Mapped["Template"] = relationship("Template", back_populates="icon") diff --git a/src/repositories/template_icon_repository.py b/src/repositories/template_icon_repository.py new file mode 100644 index 0000000..458a71a --- /dev/null +++ b/src/repositories/template_icon_repository.py @@ -0,0 +1,35 @@ +"""TemplateIcon repository for database operations.""" +from typing import Optional + +from sqlalchemy.orm import Session + +from src.models.template_icon import TemplateIcon +from src.repositories.base_repository import BaseRepository + + +class TemplateIconRepository(BaseRepository[TemplateIcon]): + """Repository for TemplateIcon database operations.""" + + def __init__(self, db: Session): + """Initialize TemplateIconRepository with database session.""" + super().__init__(TemplateIcon, db) + + def get_by_template_id(self, template_id: str) -> Optional[TemplateIcon]: + """Fetch the icon row for a given template, if any.""" + return ( + self.db.query(self.model) + .filter(self.model.template_id == str(template_id)) + .first() + ) + + def delete_by_template_id(self, template_id: str) -> bool: + """Remove the icon row for a given template. + + Returns True if a row was deleted, False if nothing existed. + """ + icon = self.get_by_template_id(template_id) + if not icon: + return False + self.db.delete(icon) + self.db.commit() + return True diff --git a/src/schemas/template.py b/src/schemas/template.py index 3696602..de98a93 100644 --- a/src/schemas/template.py +++ b/src/schemas/template.py @@ -81,6 +81,11 @@ class TemplateResponse(BaseModel): # `owner_username` are exposed to clients. owner: Any = Field(default=None, exclude=True, repr=False) + # Internes Feld für die ``effective_icon``-Berechnung. Wird von SQLAlchemy + # via ``from_attributes=True`` gefüllt, aus der Response aber + # ausgeblendet — Clients bekommen nur ``effective_icon``. + icon: Any = Field(default=None, exclude=True, repr=False) + @computed_field # type: ignore[prop-decorator] @property def owner_name(self) -> Optional[str]: @@ -105,6 +110,31 @@ def owner_username(self) -> Optional[str]: """Cached preferred_username of the owner; ``None`` for legacy users.""" return getattr(self.owner, "username", None) if self.owner else None + @computed_field # type: ignore[prop-decorator] + @property + def has_uploaded_icon(self) -> bool: + """True wenn ein Icon-Bild via ``POST /templates/{id}/icon`` hochgeladen wurde. + + Wird aus der ``TemplateIcon``-Relation abgeleitet und dient dem + Frontend als billiges Signal, ob ``effective_icon`` auf den + Serve-Endpoint verweist oder auf ``icon_url``. + """ + return self.icon is not None + + @computed_field # type: ignore[prop-decorator] + @property + def effective_icon(self) -> Optional[str]: + """Bevorzugter Icon-Wert für das Frontend. + + Wenn ein Icon-Bild hochgeladen wurde → ``/api/v1/templates/{id}/icon``. + Andernfalls Fallback auf ``icon_url`` (``mdi:*``, externe URL, …). + Ist beides leer, ist der Wert ``None`` — der Client rendert dann + einen Default-Placeholder. + """ + if self.icon is not None: + return f"/api/v1/templates/{self.id}/icon" + return self.icon_url + model_config = ConfigDict( from_attributes=True, json_schema_extra={ diff --git a/src/services/template_icon_service.py b/src/services/template_icon_service.py new file mode 100644 index 0000000..2ad99db --- /dev/null +++ b/src/services/template_icon_service.py @@ -0,0 +1,194 @@ +"""Template icon service. + +Kapselt Upload, Auslieferung und Löschen des hochgeladenen Icon-Bilds eines +Templates. Der Endpoint-Layer prüft Rollen und Ownership; der Service prüft +Bild-Format und -Größe und delegiert die eigentliche Datenbank-Interaktion +ans Repository. +""" +import logging +from typing import Optional + +from sqlalchemy.orm import Session + +from src.core.config import get_settings +from src.core.exceptions import BadRequestException, ForbiddenException +from src.models.template import Template +from src.models.template_icon import TemplateIcon +from src.repositories.template_icon_repository import TemplateIconRepository +from src.services.template_service import TemplateService + +logger = logging.getLogger(__name__) + + +class TemplateIconService: + """Service für den Upload/Serve/Delete-Lebenszyklus eines Template-Icons.""" + + def __init__(self, db: Session): + self.db = db + self.repo = TemplateIconRepository(db) + self.template_service = TemplateService(db) + + # ------------------------------------------------------------------ + # Read + # ------------------------------------------------------------------ + def get_icon( + self, + template_id: str, + *, + user_id: str, + is_admin: bool = False, + ) -> TemplateIcon: + """Return the icon row for a template, gated by visibility. + + Sichtbarkeitsregeln matchen ``TemplateService.get_template``: + Admin darf alles, Owner darf sein eigenes, Fremde nur PUBLIC-Templates + mit mindestens einer APPROVED Version. Wenn das Template zwar + sichtbar ist, aber kein Icon hochgeladen wurde, wird 404 geworfen. + """ + # ``get_template`` wirft NotFound/Forbidden nach denselben Regeln, + # die auch beim normalen Template-GET greifen. + self.template_service.get_template(template_id, user_id=user_id, is_admin=is_admin) + + icon = self.repo.get_by_template_id(template_id) + if not icon: + # Bewusst 404, nicht 204: der Client bekommt sonst einen + # Content-Type: application/json ohne Body und rätselt. + from src.core.exceptions import NotFoundException + + raise NotFoundException(f"Template {template_id} has no uploaded icon") + return icon + + # ------------------------------------------------------------------ + # Write + # ------------------------------------------------------------------ + def upload_icon( + self, + template_id: str, + *, + content: bytes, + content_type: str, + file_name: Optional[str], + user_id: str, + is_admin: bool = False, + ) -> TemplateIcon: + """Persist a new icon for a template (create or replace). + + Nur Owner oder Admin dürfen ein Icon setzen. Validierung: + - Content-Type muss in ``settings.allowed_icon_content_types`` sein + (Default: PNG/JPEG/WebP) → sonst 415. + - ``content`` darf ``settings.max_icon_size_bytes`` nicht überschreiten + → sonst 413. + - Leere Uploads werden abgelehnt (400). + """ + template = self._require_owner_or_admin(template_id, user_id=user_id, is_admin=is_admin) + settings = get_settings() + + # 1) Content-Type-Prüfung — via 415 statt 400, damit Clients gezielt + # reagieren können ("bitte anderes Format wählen"). + normalized = (content_type or "").split(";", 1)[0].strip().lower() + if normalized not in settings.allowed_icon_content_types: + from starlette.exceptions import HTTPException + + raise HTTPException( + status_code=415, + detail=( + f"Unsupported icon content type: {content_type!r}. " + f"Allowed: {', '.join(settings.allowed_icon_content_types)}" + ), + ) + + # 2) Größe. + size = len(content) + if size == 0: + raise BadRequestException("Uploaded icon file is empty") + if size > settings.max_icon_size_bytes: + from starlette.exceptions import HTTPException + + raise HTTPException( + status_code=413, + detail=( + f"Icon file too large: {size} bytes " + f"(max {settings.max_icon_size_bytes} bytes)" + ), + ) + + # 3) Persistieren — create-or-replace. Wir modifizieren die bestehende + # Row statt sie zu löschen+neu-anzulegen, damit ``id`` und + # ``created_at`` stabil bleiben (Cache-Buster im Frontend nutzt + # ``updated_at``). + existing = self.repo.get_by_template_id(template_id) + if existing: + existing.content = content + existing.content_type = normalized + existing.file_name = file_name + existing.size_bytes = size + self.db.commit() + self.db.refresh(existing) + icon = existing + action = "replaced" + else: + icon = self.repo.create( + template_id=template.id, + content=content, + content_type=normalized, + file_name=file_name, + size_bytes=size, + ) + action = "created" + + logger.info( + "Template icon %s", + action, + extra={ + "template_id": template_id, + "user_id": user_id, + "icon_id": icon.id, + "size_bytes": size, + "content_type": normalized, + }, + ) + return icon + + def delete_icon( + self, + template_id: str, + *, + user_id: str, + is_admin: bool = False, + ) -> bool: + """Remove the uploaded icon for a template. + + Returns True if something was deleted, False if the template + already had no icon. In both cases the endpoint returns 204; + the boolean is exposed for tests. + """ + self._require_owner_or_admin(template_id, user_id=user_id, is_admin=is_admin) + deleted = self.repo.delete_by_template_id(template_id) + if deleted: + logger.info( + "Template icon deleted", + extra={"template_id": template_id, "user_id": user_id}, + ) + return deleted + + # ------------------------------------------------------------------ + # Helpers + # ------------------------------------------------------------------ + def _require_owner_or_admin( + self, + template_id: str, + *, + user_id: str, + is_admin: bool, + ) -> Template: + """Load template + enforce owner-or-admin gate for mutating ops.""" + template = self.template_service.get_template( + template_id, + user_id=user_id, + is_admin=is_admin, + ) + if template.owner_id != user_id and not is_admin: + raise ForbiddenException( + "You do not have permission to manage the icon of this template" + ) + return template diff --git a/tests/api/test_template_icon_routes.py b/tests/api/test_template_icon_routes.py new file mode 100644 index 0000000..fbab466 --- /dev/null +++ b/tests/api/test_template_icon_routes.py @@ -0,0 +1,334 @@ +"""API-Tests für die Template-Icon-Endpoints. + +Deckt POST/GET/DELETE ab, inkl. Content-Type-Whitelist, Größenlimit, +Owner/Admin-Gate, sowie das Zusammenspiel mit der TemplateResponse +(``effective_icon`` schaltet nach dem Upload auf die Serve-URL um). +""" +import pytest +from fastapi import status +from fastapi.testclient import TestClient +from sqlalchemy import create_engine, event +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import StaticPool + +from src.core.database import Base +from src.core.dependencies import get_current_user, get_db +from src.main import app +from src.models.template import Template, TemplateVisibility +from src.models.user import User + + +SQLALCHEMY_DATABASE_URL = "sqlite:///:memory:" +engine = create_engine( + SQLALCHEMY_DATABASE_URL, + connect_args={"check_same_thread": False}, + poolclass=StaticPool, +) + + +@event.listens_for(engine, "connect") +def _sqlite_enable_fks(dbapi_connection, _conn_record): + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + +TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + + +@pytest.fixture(scope="function") +def db_session(): + import src.models.deployment # noqa + import src.models.deployment_instance # noqa + import src.models.deployment_instance_access # noqa + import src.models.deployment_log # noqa + import src.models.template_category # noqa + import src.models.template_category_assignment # noqa + import src.models.template_icon # noqa + import src.models.template_version # noqa + import src.models.course # noqa + import src.models.course_member # noqa + import src.models.course_group # noqa + import src.models.group_member # noqa + import src.models.openstack_project # noqa + + Base.metadata.create_all(bind=engine) + session = TestingSessionLocal() + try: + yield session + finally: + session.close() + Base.metadata.drop_all(bind=engine) + + +@pytest.fixture +def owner(db_session): + """Der User, dem das Sample-Template gehört.""" + user = User(id="00000000-0000-0000-0000-000000000000", external_id="ext-owner") + db_session.add(user) + db_session.commit() + db_session.refresh(user) + return user + + +@pytest.fixture +def other_user(db_session): + """Ein weiterer User, der weder Owner noch Admin ist.""" + user = User(id="11111111-1111-1111-1111-111111111111", external_id="ext-other") + db_session.add(user) + db_session.commit() + db_session.refresh(user) + return user + + +@pytest.fixture +def sample_template(db_session, owner): + template = Template( + name="Icon Template", + description="Template to test icon upload", + owner_id=owner.id, + repo_url="https://github.com/example/icon-template", + visibility=TemplateVisibility.PUBLIC, + icon_url="mdi:server", + ) + db_session.add(template) + db_session.commit() + db_session.refresh(template) + return template + + +def _make_client(db_session, user_id: str, roles: list[str]): + """Wire the TestClient with a static current-user override.""" + def override_get_db(): + try: + yield db_session + finally: + pass + + def override_get_current_user(): + return { + "sub": user_id, + "email": f"{user_id}@example.com", + "name": user_id, + "preferred_username": user_id, + "roles": roles, + "user_id": user_id, + } + + app.dependency_overrides[get_db] = override_get_db + app.dependency_overrides[get_current_user] = override_get_current_user + return TestClient(app) + + +@pytest.fixture +def owner_client(db_session, owner): + """Client authenticated as the template owner (lecturer role).""" + client = _make_client(db_session, owner.id, ["lecturer"]) + yield client + app.dependency_overrides.clear() + + +@pytest.fixture +def admin_client(db_session, other_user): + """Client authenticated as an admin user (not the owner).""" + client = _make_client(db_session, other_user.id, ["admin", "lecturer"]) + yield client + app.dependency_overrides.clear() + + +# Minimal, valid PNG (1x1 transparent pixel) — small enough that we can +# use the real Pillow-free byte sequence in tests without a dependency. +PNG_1x1 = ( + b"\x89PNG\r\n\x1a\n" + b"\x00\x00\x00\rIHDR" + b"\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89" + b"\x00\x00\x00\rIDATx\x9cc\xf8\xcf\xc0\x00\x00\x00\x03\x00\x01" + b"\x00\x18\xdd\x8d\xb4\x00\x00\x00\x00IEND\xaeB`\x82" +) + + +class TestUploadIcon: + def test_owner_can_upload_png(self, owner_client, sample_template): + response = owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + assert response.status_code == status.HTTP_201_CREATED + data = response.json()["data"] + assert data["template_id"] == sample_template.id + assert data["content_type"] == "image/png" + assert data["size_bytes"] == len(PNG_1x1) + assert data["url"] == f"/api/v1/templates/{sample_template.id}/icon" + + def test_upload_updates_effective_icon_in_template_response( + self, owner_client, sample_template + ): + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + get_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}") + assert get_resp.status_code == status.HTTP_200_OK + body = get_resp.json()["data"] + assert body["has_uploaded_icon"] is True + assert body["effective_icon"] == f"/api/v1/templates/{sample_template.id}/icon" + # icon_url bleibt als Rohfeld erhalten + assert body["icon_url"] == "mdi:server" + + def test_upload_svg_rejected_415(self, owner_client, sample_template): + response = owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.svg", b"", "image/svg+xml")}, + ) + assert response.status_code == 415 + + def test_upload_text_rejected_415(self, owner_client, sample_template): + response = owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("hello.txt", b"hello", "text/plain")}, + ) + assert response.status_code == 415 + + def test_upload_empty_file_rejected_400(self, owner_client, sample_template): + response = owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("empty.png", b"", "image/png")}, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_stranger_cannot_upload(self, db_session, sample_template, other_user): + """Ein User, der weder Owner noch Admin ist, darf kein Icon setzen. + + Der Template-Sichtbarkeits-Gate schießt hier zuerst (PUBLIC-Template + ohne APPROVED Version → 403 auf GET), also bekommen wir schon + beim Ownership-Check ein 403 zurück statt eines 200. + """ + client = _make_client(db_session, other_user.id, ["lecturer"]) + try: + response = client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + finally: + app.dependency_overrides.clear() + + def test_admin_can_upload_on_other_users_template( + self, admin_client, sample_template + ): + response = admin_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + assert response.status_code == status.HTTP_201_CREATED + + def test_reupload_replaces_bytes(self, owner_client, sample_template): + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + # Anderes Byte-Muster hochladen — Größe muss sich am GET zeigen. + larger = PNG_1x1 + b"\x00" * 32 + # Zweiter Upload — muss die vorhandene Row updaten, nicht duplizieren. + r2 = owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo2.png", larger, "image/png")}, + ) + assert r2.status_code == status.HTTP_201_CREATED + assert r2.json()["data"]["size_bytes"] == len(larger) + assert r2.json()["data"]["file_name"] == "logo2.png" + + +class TestGetIcon: + def test_get_returns_stored_bytes_with_correct_mime( + self, owner_client, sample_template + ): + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + response = owner_client.get(f"/api/v1/templates/{sample_template.id}/icon") + assert response.status_code == status.HTTP_200_OK + assert response.headers["content-type"] == "image/png" + assert response.content == PNG_1x1 + + def test_get_returns_404_when_no_icon_uploaded( + self, owner_client, sample_template + ): + response = owner_client.get(f"/api/v1/templates/{sample_template.id}/icon") + assert response.status_code == status.HTTP_404_NOT_FOUND + + +class TestDeleteIcon: + def test_owner_can_delete_icon(self, owner_client, sample_template): + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + response = owner_client.delete( + f"/api/v1/templates/{sample_template.id}/icon" + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + # Icon ist danach weg → GET liefert 404. + get_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}/icon") + assert get_resp.status_code == status.HTTP_404_NOT_FOUND + # ``effective_icon`` fällt wieder auf ``icon_url`` zurück. + tpl_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}") + body = tpl_resp.json()["data"] + assert body["has_uploaded_icon"] is False + assert body["effective_icon"] == "mdi:server" + + def test_delete_is_idempotent(self, owner_client, sample_template): + """Auch ohne vorher hochgeladenes Icon liefert DELETE 204.""" + response = owner_client.delete( + f"/api/v1/templates/{sample_template.id}/icon" + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + + def test_stranger_cannot_delete( + self, db_session, sample_template, other_user, owner_client + ): + # Setup: owner lädt zuerst ein Icon hoch, dann darf ``other_user`` + # es nicht wegnehmen. + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + app.dependency_overrides.clear() + + client = _make_client(db_session, other_user.id, ["lecturer"]) + try: + response = client.delete( + f"/api/v1/templates/{sample_template.id}/icon" + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + finally: + app.dependency_overrides.clear() + + +class TestTemplateDeletionCascadesIcon: + def test_deleting_template_removes_its_icon( + self, owner_client, sample_template, db_session + ): + from src.models.template_icon import TemplateIcon + + owner_client.post( + f"/api/v1/templates/{sample_template.id}/icon", + files={"file": ("logo.png", PNG_1x1, "image/png")}, + ) + assert ( + db_session.query(TemplateIcon) + .filter_by(template_id=sample_template.id) + .first() + is not None + ) + del_resp = owner_client.delete(f"/api/v1/templates/{sample_template.id}") + assert del_resp.status_code == status.HTTP_204_NO_CONTENT + # Cascade sollte die Icon-Row mitreißen. + db_session.expire_all() + assert ( + db_session.query(TemplateIcon) + .filter_by(template_id=sample_template.id) + .first() + is None + ) diff --git a/tests/unit/test_template_effective_icon_schema.py b/tests/unit/test_template_effective_icon_schema.py new file mode 100644 index 0000000..2a66599 --- /dev/null +++ b/tests/unit/test_template_effective_icon_schema.py @@ -0,0 +1,88 @@ +"""Tests für die ``effective_icon``-Aggregation auf TemplateResponse. + +Frontend soll nur ein Feld rendern müssen: hochgeladenes Bild → Serve-URL, +sonst Fallback auf ``icon_url``, sonst ``None``. Die rohe Icon-Relation +wird bewusst ausgeblendet. +""" +from datetime import datetime, timezone +from types import SimpleNamespace + +from src.schemas.template import TemplateResponse + + +def _orm_template(**overrides): + """Build a Template-like ORM stub for schema validation.""" + defaults = dict( + id="tmpl-1", + name="Test Template", + description=None, + owner_id="user-1", + repo_url="https://github.com/example/test", + icon_url=None, + visibility="private", + versions=None, + owner=None, + icon=None, + created_at=datetime(2026, 1, 1, tzinfo=timezone.utc), + updated_at=datetime(2026, 1, 1, tzinfo=timezone.utc), + ) + defaults.update(overrides) + return SimpleNamespace(**defaults) + + +class TestEffectiveIcon: + def test_uploaded_icon_wins_over_icon_url(self): + """Wenn beide gesetzt sind, wird die Serve-URL des Uploads zurückgegeben.""" + icon = SimpleNamespace(id="icon-42") + response = TemplateResponse.model_validate( + _orm_template(icon_url="mdi:server", icon=icon) + ) + assert response.effective_icon == "/api/v1/templates/tmpl-1/icon" + assert response.has_uploaded_icon is True + + def test_icon_url_only_returned_when_no_upload(self): + response = TemplateResponse.model_validate( + _orm_template(icon_url="mdi:server", icon=None) + ) + assert response.effective_icon == "mdi:server" + assert response.has_uploaded_icon is False + + def test_external_url_returned_when_no_upload(self): + response = TemplateResponse.model_validate( + _orm_template(icon_url="https://cdn.example.com/logo.png", icon=None) + ) + assert response.effective_icon == "https://cdn.example.com/logo.png" + assert response.has_uploaded_icon is False + + def test_none_when_neither_set(self): + response = TemplateResponse.model_validate( + _orm_template(icon_url=None, icon=None) + ) + assert response.effective_icon is None + assert response.has_uploaded_icon is False + + +class TestSerializedPayloadShape: + def test_raw_icon_object_not_leaked_into_json(self): + """Die ORM-Icon-Relation darf nicht in die Response wandern — + Clients bekommen nur ``effective_icon`` + ``has_uploaded_icon``.""" + icon = SimpleNamespace(id="icon-42", content_type="image/png") + payload = TemplateResponse.model_validate( + _orm_template(icon_url="mdi:server", icon=icon) + ).model_dump(mode="json") + + assert "icon" not in payload + assert payload["effective_icon"] == "/api/v1/templates/tmpl-1/icon" + assert payload["has_uploaded_icon"] is True + # icon_url bleibt als Rohfeld sichtbar, damit Bearbeitungs-UIs + # den ursprünglichen Wert weiter im Formular haben. + assert payload["icon_url"] == "mdi:server" + + def test_json_payload_when_only_icon_url_set(self): + payload = TemplateResponse.model_validate( + _orm_template(icon_url="mdi:server", icon=None) + ).model_dump(mode="json") + + assert payload["effective_icon"] == "mdi:server" + assert payload["has_uploaded_icon"] is False + assert payload["icon_url"] == "mdi:server" diff --git a/tests/unit/test_template_icon_service.py b/tests/unit/test_template_icon_service.py new file mode 100644 index 0000000..822c23d --- /dev/null +++ b/tests/unit/test_template_icon_service.py @@ -0,0 +1,353 @@ +"""Unit-Tests für den Template-Icon-Service. + +Testen Content-Type-Whitelist, Größenlimit, Owner/Admin-Gate, sowie den +Create-vs-Replace-Zweig. Wir vermeiden echte DB-Setups und nutzen +MagicMock-Sessions — die Zusammenarbeit mit dem Repository ist trivial +genug, dass die Interaktion pro Testfall stubbbar ist. +""" +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest +from starlette.exceptions import HTTPException + +from src.core.exceptions import BadRequestException, ForbiddenException +from src.models.template import Template, TemplateVisibility +from src.models.template_icon import TemplateIcon +from src.services.template_icon_service import TemplateIconService + + +def _tpl(owner_id: str = "owner-1") -> Template: + """Build a plain Template ORM object (no DB) with the fields the + service touches. ``visibility=PUBLIC`` because ``get_template`` also + checks the general visibility gate — for owner access that check is + a no-op, but we want to be defensive. + """ + t = Template() + t.id = str(uuid4()) + t.name = "demo" + t.description = None + t.owner_id = owner_id + t.repo_url = "https://example.com" + t.icon_url = None + t.visibility = TemplateVisibility.PRIVATE + t.publish_requested = False + t.versions = [] + return t + + +def _service_with_stubs(template: Template) -> TemplateIconService: + """Wire a service with mocked ``template_service`` + ``repo`` so we + can drive the two collaborators without a real DB.""" + svc = TemplateIconService(MagicMock()) + svc.template_service = MagicMock() + svc.template_service.get_template.return_value = template + svc.repo = MagicMock() + return svc + + +# --------------------------------------------------------------------------- +# Upload — content-type validation +# --------------------------------------------------------------------------- +class TestUploadContentTypes: + def test_png_accepted(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + svc.repo.create.return_value = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + size_bytes=1, + ) + icon = svc.upload_icon( + tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + user_id="owner-1", + ) + assert icon.content_type == "image/png" + + def test_jpeg_accepted(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + svc.repo.create.return_value = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"y", + content_type="image/jpeg", + file_name="a.jpg", + size_bytes=1, + ) + icon = svc.upload_icon( + tpl.id, + content=b"y", + content_type="image/jpeg", + file_name="a.jpg", + user_id="owner-1", + ) + assert icon.content_type == "image/jpeg" + + def test_webp_accepted(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + svc.repo.create.return_value = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"z", + content_type="image/webp", + file_name=None, + size_bytes=1, + ) + icon = svc.upload_icon( + tpl.id, + content=b"z", + content_type="image/webp", + file_name=None, + user_id="owner-1", + ) + assert icon.content_type == "image/webp" + + def test_svg_rejected_with_415(self): + """SVG ist bewusst nicht erlaubt (XML-Payload / Skript-Vektor).""" + tpl = _tpl() + svc = _service_with_stubs(tpl) + with pytest.raises(HTTPException) as exc: + svc.upload_icon( + tpl.id, + content=b"", + content_type="image/svg+xml", + file_name="a.svg", + user_id="owner-1", + ) + assert exc.value.status_code == 415 + + def test_plain_text_rejected_with_415(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + with pytest.raises(HTTPException) as exc: + svc.upload_icon( + tpl.id, + content=b"hello", + content_type="text/plain", + file_name="a.txt", + user_id="owner-1", + ) + assert exc.value.status_code == 415 + + def test_content_type_with_charset_suffix_still_accepted(self): + """Browser hängen manchmal ``; charset=…`` an — wir strippen.""" + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + svc.repo.create.return_value = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + size_bytes=1, + ) + icon = svc.upload_icon( + tpl.id, + content=b"x", + content_type="image/png; charset=binary", + file_name="a.png", + user_id="owner-1", + ) + assert icon.content_type == "image/png" + + +# --------------------------------------------------------------------------- +# Upload — size validation +# --------------------------------------------------------------------------- +class TestUploadSize: + def test_empty_upload_rejected_400(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + with pytest.raises(BadRequestException): + svc.upload_icon( + tpl.id, + content=b"", + content_type="image/png", + file_name="a.png", + user_id="owner-1", + ) + + def test_oversize_upload_rejected_413(self, monkeypatch): + tpl = _tpl() + svc = _service_with_stubs(tpl) + + # Kleiner Grenzwert, damit wir keine 5 MB im Test allokieren müssen. + from src.core import config as config_module + + fake_settings = config_module.get_settings() + # ``Settings`` ist eine Pydantic-Instanz; wir mutieren die Cache-Kopie. + # Der ``get_settings``-Cache liefert dieselbe Instanz, damit reicht das. + original = fake_settings.max_icon_size_bytes + fake_settings.max_icon_size_bytes = 10 + try: + with pytest.raises(HTTPException) as exc: + svc.upload_icon( + tpl.id, + content=b"x" * 20, + content_type="image/png", + file_name="a.png", + user_id="owner-1", + ) + assert exc.value.status_code == 413 + finally: + fake_settings.max_icon_size_bytes = original + + +# --------------------------------------------------------------------------- +# Upload — owner/admin gate +# --------------------------------------------------------------------------- +class TestUploadAuthGate: + def test_non_owner_non_admin_forbidden(self): + tpl = _tpl(owner_id="owner-1") + svc = _service_with_stubs(tpl) + with pytest.raises(ForbiddenException): + svc.upload_icon( + tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + user_id="stranger", + is_admin=False, + ) + + def test_admin_allowed_even_if_not_owner(self): + tpl = _tpl(owner_id="owner-1") + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + svc.repo.create.return_value = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + size_bytes=1, + ) + icon = svc.upload_icon( + tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + user_id="admin-99", + is_admin=True, + ) + assert icon is not None + + +# --------------------------------------------------------------------------- +# Upload — create-vs-replace behaviour +# --------------------------------------------------------------------------- +class TestUploadCreateOrReplace: + def test_first_upload_creates_new_row(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + created = TemplateIcon( + id=str(uuid4()), + template_id=tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + size_bytes=1, + ) + svc.repo.create.return_value = created + + icon = svc.upload_icon( + tpl.id, + content=b"x", + content_type="image/png", + file_name="a.png", + user_id="owner-1", + ) + assert icon is created + svc.repo.create.assert_called_once() + + def test_second_upload_replaces_content_keeps_id(self): + """Bei bereits vorhandenem Icon wird die Row in-place mutiert, + damit ``id`` und ``created_at`` stabil bleiben.""" + tpl = _tpl() + svc = _service_with_stubs(tpl) + existing = TemplateIcon( + id="stable-icon-id", + template_id=tpl.id, + content=b"old", + content_type="image/jpeg", + file_name="old.jpg", + size_bytes=3, + ) + svc.repo.get_by_template_id.return_value = existing + + icon = svc.upload_icon( + tpl.id, + content=b"NEWDATA", + content_type="image/png", + file_name="new.png", + user_id="owner-1", + ) + assert icon.id == "stable-icon-id" + assert icon.content == b"NEWDATA" + assert icon.content_type == "image/png" + assert icon.file_name == "new.png" + assert icon.size_bytes == 7 + # ``create`` darf im Replace-Pfad nicht aufgerufen werden. + svc.repo.create.assert_not_called() + + +# --------------------------------------------------------------------------- +# Get / Delete +# --------------------------------------------------------------------------- +class TestGetIcon: + def test_get_returns_icon_when_present(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + stored = TemplateIcon( + id="x", + template_id=tpl.id, + content=b"blob", + content_type="image/png", + file_name="a.png", + size_bytes=4, + ) + svc.repo.get_by_template_id.return_value = stored + icon = svc.get_icon(tpl.id, user_id="owner-1") + assert icon is stored + + def test_get_raises_404_when_no_icon(self): + from src.core.exceptions import NotFoundException + + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.get_by_template_id.return_value = None + with pytest.raises(NotFoundException): + svc.get_icon(tpl.id, user_id="owner-1") + + +class TestDeleteIcon: + def test_delete_returns_true_when_deleted(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.delete_by_template_id.return_value = True + assert svc.delete_icon(tpl.id, user_id="owner-1") is True + + def test_delete_idempotent_returns_false_when_nothing_existed(self): + tpl = _tpl() + svc = _service_with_stubs(tpl) + svc.repo.delete_by_template_id.return_value = False + assert svc.delete_icon(tpl.id, user_id="owner-1") is False + + def test_delete_non_owner_forbidden(self): + tpl = _tpl(owner_id="owner-1") + svc = _service_with_stubs(tpl) + with pytest.raises(ForbiddenException): + svc.delete_icon(tpl.id, user_id="stranger", is_admin=False) diff --git a/uv.lock b/uv.lock index 83ed3c3..c24bb86 100644 --- a/uv.lock +++ b/uv.lock @@ -71,6 +71,7 @@ dependencies = [ { name = "python-dotenv" }, { name = "python-heatclient" }, { name = "python-jose", extra = ["cryptography"] }, + { name = "python-multipart" }, { name = "pyyaml" }, { name = "sqlalchemy" }, { name = "uvicorn", extra = ["standard"] }, @@ -107,6 +108,7 @@ requires-dist = [ { name = "python-dotenv", specifier = ">=1.0.0" }, { name = "python-heatclient", specifier = ">=3.5.0" }, { name = "python-jose", extras = ["cryptography"], specifier = ">=3.3.0" }, + { name = "python-multipart", specifier = ">=0.0.9" }, { name = "pyyaml", specifier = ">=6.0.0" }, { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.1.0" }, { name = "sqlalchemy", specifier = ">=2.0.0" }, @@ -1777,6 +1779,15 @@ cryptography = [ { name = "cryptography" }, ] +[[package]] +name = "python-multipart" +version = "0.0.32" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/5b/42/55c32bb9b12693c092ad250a0e82edb5b31ddeda6eb772de5f308b3804ad/python_multipart-0.0.32.tar.gz", hash = "sha256:be54b7f3fa167bb83e4fcd936b887b708f4e57fe75911c02aebf53efaf8d938e", size = 46881, upload-time = "2026-06-04T16:18:58.647Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e1/04/e8135ebd1ad02c56ec633277529b2602ff99ff634be76cdba5744cf554fd/python_multipart-0.0.32-py3-none-any.whl", hash = "sha256:ff6d3f776f16878c894e52e107296ffc890e913c611b1a4ec6c44e2821fe2e23", size = 30042, upload-time = "2026-06-04T16:18:57.319Z" }, +] + [[package]] name = "python-swiftclient" version = "4.10.0" From 9da401687fef663941ab59fb8237a122b6edf266 Mon Sep 17 00:00:00 2001 From: nicowre Date: Wed, 1 Jul 2026 11:47:44 +0200 Subject: [PATCH 2/3] =?UTF-8?q?refactor(templates):=20drop=20icon=5Furl=20?= =?UTF-8?q?=E2=80=94=20upload-only=20icons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Icons kommen ab jetzt ausschließlich als hochgeladenes Bild. Das alte ``icon_url``-String-Feld (``mdi:*``, externe URLs, Emoji) wird komplett entfernt — an keinem Endpoint mehr entgegengenommen, nicht mehr in der Response, aus der DB gedroppt. Grund: die einzige Quelle, die ``icon_url`` je gesetzt hat, waren die zwei Seed-Templates. Die ``app.yaml``-Dateien in DoziLab/appstore-apps haben kein Icon-Feld, der AppManifestParser liest auch keins, und Nutzer haben es beim Anlegen praktisch nie manuell gepflegt. Konsistenter ist „entweder ein hochgeladenes Bild oder Placeholder". Änderungen: - Migration ``c8a3f1e9b7d5`` erweitert um ``op.drop_column('templates', 'icon_url')`` — Feature-Branch ist noch nicht deployed, also kein Bestand zu retten, eine atomare Migration. - Model, Schemas (Create/Update/Response/GithubImport) und Services entfernen das Feld. ``TemplateResponse.effective_icon`` fällt nicht mehr auf ``icon_url`` zurück — ohne Upload ist der Wert ``null``. - Seed-Daten für „Multi-User Ubuntu" und „PostgreSQL Group DB" verlieren ihre ``mdi:*``-Werte; nach diesem Change zeigen die Kacheln erstmal einen Placeholder, bis jemand ein Bild hochlädt. - Bruno-Requests (Create Template, Import Template From GitHub) senden das Feld nicht mehr mit; Docs aktualisiert. - Tests: 3 Test-Files angepasst, ``test_template_effective_icon_schema`` auf die zwei relevanten Zweige (Upload / kein Upload) reduziert. Tests: 573 passed, 4 skipped. Ruff + mypy grün. --- ...8a3f1e9b7d5_create_template_icons_table.py | 37 ++++++++++---- bruno/Templates/Create Template.bru | 8 +-- .../Templates/Import Template From GitHub.bru | 5 +- src/api/templates.py | 13 +++-- src/core/seed_data.py | 3 -- src/models/template.py | 5 -- src/schemas/template.py | 46 +++++++++-------- src/services/github_import_service.py | 2 - src/services/template_service.py | 1 - tests/api/test_template_icon_routes.py | 9 ++-- .../test_template_effective_icon_schema.py | 50 ++++++------------- tests/unit/test_template_icon_service.py | 1 - tests/unit/test_template_response_schema.py | 2 +- 13 files changed, 86 insertions(+), 96 deletions(-) diff --git a/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py b/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py index 80c7bb1..77af027 100644 --- a/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py +++ b/alembic/versions/c8a3f1e9b7d5_create_template_icons_table.py @@ -1,16 +1,20 @@ -"""create template_icons table +"""create template_icons table + drop templates.icon_url Revision ID: c8a3f1e9b7d5 Revises: e2a91d05c7b8 Create Date: 2026-07-01 09:00:00.000000 -Neue Tabelle für hochgeladene Template-Icons. Bilder werden als BYTEA -persistiert, ``template_id`` ist unique (1:1 Beziehung Template → Icon) -und ``ON DELETE CASCADE`` räumt das Icon auf, wenn das Template selbst -gelöscht wird. ``icon_url`` auf ``templates`` bleibt unverändert -(externe URLs, ``mdi:*``-Identifier usw.); die Response-Aggregation im -Schema entscheidet, ob das hochgeladene Icon oder ``icon_url`` an das -Frontend gegeben wird. +Zwei Änderungen in einer Migration, weil sie inhaltlich zusammengehören +und dieser Feature-Branch noch nicht deployt ist (kein Bestand zu retten): + +1. Neue Tabelle ``template_icons`` — hält hochgeladene Icon-Bilder als + BYTEA. ``template_id`` unique (1:1) und ``ON DELETE CASCADE``, damit + die Row automatisch mitgeht, wenn das Template gelöscht wird. + +2. Alte Spalte ``templates.icon_url`` fliegt raus. Icons kommen ab jetzt + ausschließlich als Upload; ``mdi:*``-Strings/URLs werden nicht mehr + unterstützt. Frontend zeigt für Templates ohne hochgeladenes Bild + einen Placeholder. """ from typing import Sequence, Union @@ -25,7 +29,7 @@ def upgrade() -> None: - """Create template_icons table.""" + """Create template_icons table and drop templates.icon_url.""" op.create_table( 'template_icons', sa.Column('id', sa.String(length=36), nullable=False), @@ -70,7 +74,20 @@ def upgrade() -> None: sa.UniqueConstraint('template_id', name='uq_template_icons_template_id'), ) + # icon_url wird durch hochgeladene Icons ersetzt. Kein Bestand zu retten + # (dieser Branch ist noch nicht deployed), also einfach droppen. + op.drop_column('templates', 'icon_url') + def downgrade() -> None: - """Drop template_icons table.""" + """Restore templates.icon_url and drop template_icons.""" + op.add_column( + 'templates', + sa.Column( + 'icon_url', + sa.String(length=500), + nullable=True, + comment='Icon URL or identifier (e.g., mdi:server, /icons/template.svg, 🚀)', + ), + ) op.drop_table('template_icons') diff --git a/bruno/Templates/Create Template.bru b/bruno/Templates/Create Template.bru index bb2d616..7d2a645 100644 --- a/bruno/Templates/Create Template.bru +++ b/bruno/Templates/Create Template.bru @@ -18,8 +18,7 @@ body:json { { "name": "Python Flask Template", "description": "A template for Flask web applications", - "repo_url": "https://github.com/example/flask-template", - "icon_url": "mdi:flask" + "repo_url": "https://github.com/example/flask-template" } } @@ -46,9 +45,12 @@ docs { - `name` (required, ≤ 255) - `description` (optional) - `repo_url` (required, ≤ 500) - - `icon_url` (optional, ≤ 500) — e.g. `mdi:flask`, `🚀`, `/icons/template.svg` - `visibility` (ignored on create; backend pins to `private`) + Icons: das Feld `icon_url` gibt es nicht mehr. Nach dem Anlegen kann + optional ein Icon-Bild via `POST /templates/{id}/icon` hochgeladen werden + (multipart, PNG/JPEG/WebP, max 5 MB). + ## Returns Created Template with `visibility='private'`. The post-response script saves diff --git a/bruno/Templates/Import Template From GitHub.bru b/bruno/Templates/Import Template From GitHub.bru index b375a6f..828282b 100644 --- a/bruno/Templates/Import Template From GitHub.bru +++ b/bruno/Templates/Import Template From GitHub.bru @@ -18,7 +18,6 @@ body:json { { "name": "Postgres Group DB", "description": "Provision a Postgres VM via the appstore", - "icon_url": "mdi:database", "github_url": "https://github.com/dozilab/templates", "app_yaml_path": "postgres/app.yaml" } @@ -51,11 +50,13 @@ docs { - `name` (required, ≤ 255) - `description` (optional) - - `icon_url` (optional, ≤ 500) - `github_url` (required, ≤ 1000) - `app_yaml_path` (optional) — defaults to `app.yaml` (root) if not given and the URL is the repo/branch root. + Icons: das Feld `icon_url` gibt es nicht mehr. Nach dem Import kann + optional ein Icon-Bild via `POST /templates/{id}/icon` hochgeladen werden. + ## Permissions ADMIN or LECTURER. Template is always created `visibility=private`. Admins diff --git a/src/api/templates.py b/src/api/templates.py index 9f86a46..20c7e62 100644 --- a/src/api/templates.py +++ b/src/api/templates.py @@ -235,7 +235,6 @@ async def import_template_from_github( app_yaml_path=payload.app_yaml_path, name=payload.name, description=payload.description, - icon_url=payload.icon_url, owner_user_id=current_user["user_id"], owner_user_roles=current_user.get("roles", []), # The pydantic validator normalises this to "private"/"public" (or @@ -322,10 +321,10 @@ async def upload_template_icon( ``settings.max_icon_size_bytes``). Der Endpoint speichert die Bytes in der Tabelle ``template_icons`` und - setzt in der Template-Response ab sofort ``effective_icon`` auf - ``/api/v1/templates/{id}/icon`` — d.h. das Frontend braucht nur eine - URL zu rendern, egal ob externes ``icon_url`` (``mdi:*``, externe URL) - oder hochgeladenes Bild. + setzt in der Template-Response ``effective_icon`` auf + ``/api/v1/templates/{id}/icon``. Templates ohne hochgeladenes Bild + haben ``effective_icon = null`` — das Frontend zeigt dann einen + Placeholder. """ is_admin = UserRole.ADMIN.value in current_user.get("roles", []) content = await file.read() @@ -400,8 +399,8 @@ async def delete_template_icon( Owner-or-admin-only. Idempotent: wenn kein Icon existiert, ist die Antwort trotzdem 204 (Client muss nicht wissen, ob vorher eins da war). - ``icon_url`` bleibt unverändert und wird nach dem Löschen wieder das - ``effective_icon``, falls gesetzt. + Danach fällt ``effective_icon`` auf ``null`` zurück — Frontend + rendert einen Placeholder. """ is_admin = UserRole.ADMIN.value in current_user.get("roles", []) service = TemplateIconService(db) diff --git a/src/core/seed_data.py b/src/core/seed_data.py index 39ecb73..94af99f 100644 --- a/src/core/seed_data.py +++ b/src/core/seed_data.py @@ -2032,7 +2032,6 @@ def load_servers_for_user(email: str, servers: dict) -> None: "Ubuntu VM mit mehreren Benutzerkonten, verwaltet durch Ansible. " "Pro Gruppe wird ein Linux-Account mit eigenem Arbeitsverzeichnis erstellt." ), - "icon_url": "mdi:server-network", "version": "2.1.0", "files": [ {"name": "app.yaml", "type": FileType.APP_MANIFEST, "path": "app.yaml", "content": MULTIUSER_APP_YAML, "primary": False}, @@ -2051,7 +2050,6 @@ def load_servers_for_user(email: str, servers: dict) -> None: "eine eigene Datenbank und einen eigenen DB-Rollen-Account. Der Dozent " "hat lesenden/schreibenden Zugriff auf alle Gruppen-DBs." ), - "icon_url": "mdi:database", "version": "2.0.0", "files": [ {"name": "app.yaml", "type": FileType.APP_MANIFEST, "path": "app.yaml", "content": POSTGRES_APP_YAML, "primary": False}, @@ -2127,7 +2125,6 @@ def _seed_one_app(db: Session, owner_id: str, app: dict) -> None: name=app["name"], description=app["description"], repo_url="https://github.com/dozilab/appstore-templates", - icon_url=app["icon_url"], visibility=TemplateVisibility.PUBLIC, ) db.add(template) diff --git a/src/models/template.py b/src/models/template.py index 057e476..8251bb3 100644 --- a/src/models/template.py +++ b/src/models/template.py @@ -25,11 +25,6 @@ class Template(Base): description: Mapped[str | None] = mapped_column(Text, nullable=True) owner_id: Mapped[str] = mapped_column(String(36), ForeignKey("users.id"), nullable=False) repo_url: Mapped[str] = mapped_column(String(500), nullable=False) - icon_url: Mapped[str | None] = mapped_column( - String(500), - nullable=True, - comment="Icon URL or identifier (e.g., mdi:server, /icons/template.svg, 🚀)" - ) visibility: Mapped[TemplateVisibility] = mapped_column( SQLEnum(TemplateVisibility), default=TemplateVisibility.PRIVATE diff --git a/src/schemas/template.py b/src/schemas/template.py index de98a93..73b74f0 100644 --- a/src/schemas/template.py +++ b/src/schemas/template.py @@ -13,11 +13,15 @@ class TemplateCreate(BaseModel): - """Schema for creating a template.""" + """Schema for creating a template. + + Icons werden nicht mehr im Metadata-Body übergeben — der Client legt + das Template zunächst ohne Icon an und lädt anschließend optional ein + Bild via ``POST /templates/{id}/icon`` hoch. + """ name: str = Field(..., description="Name of the template", max_length=255) description: Optional[str] = Field(None, description="Template description") repo_url: str = Field(..., description="Git repository URL", max_length=500) - icon_url: Optional[str] = Field(None, description="Icon URL or identifier (mdi:server, fa:server, 🚀, /icons/template.svg)", max_length=500) visibility: str = Field(default="private", description="Template visibility (private/public)") model_config = ConfigDict( @@ -26,7 +30,6 @@ class TemplateCreate(BaseModel): "name": "Python Flask Template", "description": "A template for Flask web applications", "repo_url": "https://github.com/example/flask-template", - "icon_url": "mdi:flask", "visibility": "public" } } @@ -34,11 +37,14 @@ class TemplateCreate(BaseModel): class TemplateUpdate(BaseModel): - """Schema for updating a template.""" + """Schema for updating a template. + + Wie ``TemplateCreate`` — kein Icon-Feld mehr. Bild-Änderungen laufen + über den dedizierten Upload-Endpoint. + """ name: Optional[str] = Field(None, description="Name of the template", max_length=255) description: Optional[str] = Field(None, description="Template description") repo_url: Optional[str] = Field(None, description="Git repository URL", max_length=500) - icon_url: Optional[str] = Field(None, description="Icon URL or identifier (mdi:server, fa:server, 🚀, /icons/template.svg)", max_length=500) visibility: Optional[str] = Field(None, description="Template visibility (private/public) - Only admins can change this") model_config = ConfigDict( @@ -46,7 +52,6 @@ class TemplateUpdate(BaseModel): "example": { "name": "Updated Template Name", "description": "Updated description", - "icon_url": "mdi:server" } } ) @@ -59,7 +64,6 @@ class TemplateResponse(BaseModel): description: Optional[str] = Field(None, description="Template description") owner_id: str = Field(..., description="Owner user ID") repo_url: str = Field(..., description="Git repository URL") - icon_url: Optional[str] = Field(None, description="Icon URL or identifier") visibility: str = Field(..., description="Template visibility") publish_requested: bool = Field( default=False, @@ -83,7 +87,8 @@ class TemplateResponse(BaseModel): # Internes Feld für die ``effective_icon``-Berechnung. Wird von SQLAlchemy # via ``from_attributes=True`` gefüllt, aus der Response aber - # ausgeblendet — Clients bekommen nur ``effective_icon``. + # ausgeblendet — Clients bekommen nur ``effective_icon`` / + # ``has_uploaded_icon``. icon: Any = Field(default=None, exclude=True, repr=False) @computed_field # type: ignore[prop-decorator] @@ -115,25 +120,24 @@ def owner_username(self) -> Optional[str]: def has_uploaded_icon(self) -> bool: """True wenn ein Icon-Bild via ``POST /templates/{id}/icon`` hochgeladen wurde. - Wird aus der ``TemplateIcon``-Relation abgeleitet und dient dem - Frontend als billiges Signal, ob ``effective_icon`` auf den - Serve-Endpoint verweist oder auf ``icon_url``. + Wird aus der ``TemplateIcon``-Relation abgeleitet und ist dasselbe + Signal, das ``effective_icon`` intern nutzt — praktisch fürs + Frontend, um „Icon entfernen"-Buttons konditional zu rendern. """ return self.icon is not None @computed_field # type: ignore[prop-decorator] @property def effective_icon(self) -> Optional[str]: - """Bevorzugter Icon-Wert für das Frontend. + """Icon-URL für das Frontend, falls ein Bild hochgeladen wurde. - Wenn ein Icon-Bild hochgeladen wurde → ``/api/v1/templates/{id}/icon``. - Andernfalls Fallback auf ``icon_url`` (``mdi:*``, externe URL, …). - Ist beides leer, ist der Wert ``None`` — der Client rendert dann - einen Default-Placeholder. + Wenn ein Icon existiert → ``/api/v1/templates/{id}/icon``. + Sonst ``None`` — der Client rendert dann einen Default-Placeholder. + Externe URLs oder ``mdi:*``-Identifier gibt es nicht mehr. """ if self.icon is not None: return f"/api/v1/templates/{self.id}/icon" - return self.icon_url + return None model_config = ConfigDict( from_attributes=True, @@ -147,8 +151,9 @@ def effective_icon(self) -> Optional[str]: "owner_email": "berg@dhbw.de", "owner_username": "bberg", "repo_url": "https://github.com/example/flask-template", - "icon_url": "mdi:flask", "visibility": "public", + "has_uploaded_icon": False, + "effective_icon": None, "versions": [], "created_at": "2024-11-27T10:00:00Z", "updated_at": "2024-11-27T10:00:00Z" @@ -164,10 +169,12 @@ class GithubImportNewTemplate(BaseModel): no approval flow). Pass ``visibility="public"`` to make it marketplace- visible — the first version then enters the standard approval flow (``pending`` unless the caller is an admin). + + Icons werden nach dem Import optional via ``POST /templates/{id}/icon`` + hochgeladen — kein Icon-Feld mehr auf dem Import-Body. """ name: str = Field(..., max_length=255) description: Optional[str] = None - icon_url: Optional[str] = Field(None, max_length=500) github_url: str = Field(..., description=GITHUB_URL_DESCRIPTION, max_length=1000) app_yaml_path: Optional[str] = Field( default=None, @@ -231,4 +238,3 @@ class GithubImportNewVersion(BaseModel): } } ) - diff --git a/src/services/github_import_service.py b/src/services/github_import_service.py index fba7094..cfe4fb5 100644 --- a/src/services/github_import_service.py +++ b/src/services/github_import_service.py @@ -334,7 +334,6 @@ def import_to_new_template( app_yaml_path: Optional[str], name: str, description: Optional[str], - icon_url: Optional[str], owner_user_id: str, owner_user_roles: list[str], visibility: TemplateVisibility = TemplateVisibility.PRIVATE, @@ -368,7 +367,6 @@ def import_to_new_template( description=description, owner_id=owner_user_id, repo_url=github_url, - icon_url=icon_url, visibility=effective_visibility, publish_requested=wants_public, ) diff --git a/src/services/template_service.py b/src/services/template_service.py index c0086a7..e2da30d 100644 --- a/src/services/template_service.py +++ b/src/services/template_service.py @@ -88,7 +88,6 @@ def create_template( name=template_data.name, description=template_data.description, repo_url=template_data.repo_url, - icon_url=template_data.icon_url, visibility=TemplateVisibility.PRIVATE, owner_id=owner_id, ) diff --git a/tests/api/test_template_icon_routes.py b/tests/api/test_template_icon_routes.py index fbab466..879ad4c 100644 --- a/tests/api/test_template_icon_routes.py +++ b/tests/api/test_template_icon_routes.py @@ -89,7 +89,6 @@ def sample_template(db_session, owner): owner_id=owner.id, repo_url="https://github.com/example/icon-template", visibility=TemplateVisibility.PUBLIC, - icon_url="mdi:server", ) db_session.add(template) db_session.commit() @@ -172,8 +171,8 @@ def test_upload_updates_effective_icon_in_template_response( body = get_resp.json()["data"] assert body["has_uploaded_icon"] is True assert body["effective_icon"] == f"/api/v1/templates/{sample_template.id}/icon" - # icon_url bleibt als Rohfeld erhalten - assert body["icon_url"] == "mdi:server" + # icon_url gibt es nicht mehr — nur noch effective_icon / has_uploaded_icon + assert "icon_url" not in body def test_upload_svg_rejected_415(self, owner_client, sample_template): response = owner_client.post( @@ -272,11 +271,11 @@ def test_owner_can_delete_icon(self, owner_client, sample_template): # Icon ist danach weg → GET liefert 404. get_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}/icon") assert get_resp.status_code == status.HTTP_404_NOT_FOUND - # ``effective_icon`` fällt wieder auf ``icon_url`` zurück. + # ``effective_icon`` ist ohne Upload ``None`` — Frontend rendert Placeholder. tpl_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}") body = tpl_resp.json()["data"] assert body["has_uploaded_icon"] is False - assert body["effective_icon"] == "mdi:server" + assert body["effective_icon"] is None def test_delete_is_idempotent(self, owner_client, sample_template): """Auch ohne vorher hochgeladenes Icon liefert DELETE 204.""" diff --git a/tests/unit/test_template_effective_icon_schema.py b/tests/unit/test_template_effective_icon_schema.py index 2a66599..88449a5 100644 --- a/tests/unit/test_template_effective_icon_schema.py +++ b/tests/unit/test_template_effective_icon_schema.py @@ -1,8 +1,8 @@ -"""Tests für die ``effective_icon``-Aggregation auf TemplateResponse. +"""Tests für ``effective_icon`` und ``has_uploaded_icon`` auf TemplateResponse. -Frontend soll nur ein Feld rendern müssen: hochgeladenes Bild → Serve-URL, -sonst Fallback auf ``icon_url``, sonst ``None``. Die rohe Icon-Relation -wird bewusst ausgeblendet. +Nach dem Umbau kennt das Backend nur noch hochgeladene Icon-Bilder; +``mdi:*``/URL-Strings gibt es nicht mehr. Frontend rendert entweder +``effective_icon`` als ```` oder einen Placeholder. """ from datetime import datetime, timezone from types import SimpleNamespace @@ -18,7 +18,6 @@ def _orm_template(**overrides): description=None, owner_id="user-1", repo_url="https://github.com/example/test", - icon_url=None, visibility="private", versions=None, owner=None, @@ -31,33 +30,15 @@ def _orm_template(**overrides): class TestEffectiveIcon: - def test_uploaded_icon_wins_over_icon_url(self): - """Wenn beide gesetzt sind, wird die Serve-URL des Uploads zurückgegeben.""" + def test_uploaded_icon_returns_serve_url(self): icon = SimpleNamespace(id="icon-42") - response = TemplateResponse.model_validate( - _orm_template(icon_url="mdi:server", icon=icon) - ) + response = TemplateResponse.model_validate(_orm_template(icon=icon)) assert response.effective_icon == "/api/v1/templates/tmpl-1/icon" assert response.has_uploaded_icon is True - def test_icon_url_only_returned_when_no_upload(self): - response = TemplateResponse.model_validate( - _orm_template(icon_url="mdi:server", icon=None) - ) - assert response.effective_icon == "mdi:server" - assert response.has_uploaded_icon is False - - def test_external_url_returned_when_no_upload(self): - response = TemplateResponse.model_validate( - _orm_template(icon_url="https://cdn.example.com/logo.png", icon=None) - ) - assert response.effective_icon == "https://cdn.example.com/logo.png" - assert response.has_uploaded_icon is False - - def test_none_when_neither_set(self): - response = TemplateResponse.model_validate( - _orm_template(icon_url=None, icon=None) - ) + def test_no_icon_returns_none(self): + """Ohne Upload ist ``effective_icon`` ``None`` — kein Fallback.""" + response = TemplateResponse.model_validate(_orm_template(icon=None)) assert response.effective_icon is None assert response.has_uploaded_icon is False @@ -68,21 +49,18 @@ def test_raw_icon_object_not_leaked_into_json(self): Clients bekommen nur ``effective_icon`` + ``has_uploaded_icon``.""" icon = SimpleNamespace(id="icon-42", content_type="image/png") payload = TemplateResponse.model_validate( - _orm_template(icon_url="mdi:server", icon=icon) + _orm_template(icon=icon) ).model_dump(mode="json") assert "icon" not in payload + assert "icon_url" not in payload # Feld existiert nicht mehr assert payload["effective_icon"] == "/api/v1/templates/tmpl-1/icon" assert payload["has_uploaded_icon"] is True - # icon_url bleibt als Rohfeld sichtbar, damit Bearbeitungs-UIs - # den ursprünglichen Wert weiter im Formular haben. - assert payload["icon_url"] == "mdi:server" - def test_json_payload_when_only_icon_url_set(self): + def test_json_payload_when_no_upload(self): payload = TemplateResponse.model_validate( - _orm_template(icon_url="mdi:server", icon=None) + _orm_template(icon=None) ).model_dump(mode="json") - assert payload["effective_icon"] == "mdi:server" + assert payload["effective_icon"] is None assert payload["has_uploaded_icon"] is False - assert payload["icon_url"] == "mdi:server" diff --git a/tests/unit/test_template_icon_service.py b/tests/unit/test_template_icon_service.py index 822c23d..399a489 100644 --- a/tests/unit/test_template_icon_service.py +++ b/tests/unit/test_template_icon_service.py @@ -29,7 +29,6 @@ def _tpl(owner_id: str = "owner-1") -> Template: t.description = None t.owner_id = owner_id t.repo_url = "https://example.com" - t.icon_url = None t.visibility = TemplateVisibility.PRIVATE t.publish_requested = False t.versions = [] diff --git a/tests/unit/test_template_response_schema.py b/tests/unit/test_template_response_schema.py index 164b180..9b21249 100644 --- a/tests/unit/test_template_response_schema.py +++ b/tests/unit/test_template_response_schema.py @@ -15,10 +15,10 @@ def _orm_template(owner=None, **overrides): description="A test template", owner_id="user-1", repo_url="https://github.com/example/test", - icon_url=None, visibility="private", versions=None, owner=owner, + icon=None, created_at=datetime(2026, 1, 1, tzinfo=timezone.utc), updated_at=datetime(2026, 1, 1, tzinfo=timezone.utc), ) From 87c37d66fb7d43f2e212ea9c82c641e0f335d6d3 Mon Sep 17 00:00:00 2001 From: nicowre Date: Wed, 1 Jul 2026 11:59:20 +0200 Subject: [PATCH 3/3] =?UTF-8?q?refactor(templates):=20rename=20effective?= =?UTF-8?q?=5Ficon=20=E2=86=92=20icon=5Fpath,=20drop=20has=5Fuploaded=5Fic?= =?UTF-8?q?on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nach dem Wegfall der icon_url-Alternative gibt es nur noch eine Quelle für ein Template-Icon: den Upload. Der Name ``effective_icon`` machte nur Sinn, solange das Feld zwischen zwei Kandidaten (Upload vs. mdi/URL-String) wählte — jetzt ist es einfach der Pfad zum Icon-Bild oder null. Zusätzlich: streng genommen ist der Wert kein URL, sondern ein relativer Pfad (kein Scheme, kein Host). Deshalb ``icon_path`` statt ``icon_url`` — der Client muss ihn ohnehin gegen seine API-Base-URL auflösen. ``has_uploaded_icon`` fällt weg — das Flag trägt exakt dieselbe Info wie ``icon_path !== null`` und ist reines Rauschen im Response-Body. Response-Feld im POST-Upload-Endpoint ebenfalls von ``url`` auf ``icon_path`` umbenannt für Konsistenz. Tests: 573 passed. Ruff + mypy grün. --- src/api/templates.py | 10 +++--- src/schemas/template.py | 31 +++++++------------ tests/api/test_template_icon_routes.py | 16 +++++----- ...a.py => test_template_icon_path_schema.py} | 29 +++++++++-------- 4 files changed, 37 insertions(+), 49 deletions(-) rename tests/unit/{test_template_effective_icon_schema.py => test_template_icon_path_schema.py} (64%) diff --git a/src/api/templates.py b/src/api/templates.py index 20c7e62..94cbc73 100644 --- a/src/api/templates.py +++ b/src/api/templates.py @@ -321,9 +321,9 @@ async def upload_template_icon( ``settings.max_icon_size_bytes``). Der Endpoint speichert die Bytes in der Tabelle ``template_icons`` und - setzt in der Template-Response ``effective_icon`` auf + setzt in der Template-Response ``icon_path`` auf ``/api/v1/templates/{id}/icon``. Templates ohne hochgeladenes Bild - haben ``effective_icon = null`` — das Frontend zeigt dann einen + haben ``icon_path = null`` — das Frontend zeigt dann einen Placeholder. """ is_admin = UserRole.ADMIN.value in current_user.get("roles", []) @@ -344,7 +344,7 @@ async def upload_template_icon( "content_type": icon.content_type, "file_name": icon.file_name, "size_bytes": icon.size_bytes, - "url": f"/api/v1/templates/{icon.template_id}/icon", + "icon_path": f"/api/v1/templates/{icon.template_id}/icon", }, message="Template icon uploaded successfully", request_id=request_id, @@ -399,8 +399,8 @@ async def delete_template_icon( Owner-or-admin-only. Idempotent: wenn kein Icon existiert, ist die Antwort trotzdem 204 (Client muss nicht wissen, ob vorher eins da war). - Danach fällt ``effective_icon`` auf ``null`` zurück — Frontend - rendert einen Placeholder. + Danach fällt ``icon_path`` auf ``null`` zurück — Frontend rendert + einen Placeholder. """ is_admin = UserRole.ADMIN.value in current_user.get("roles", []) service = TemplateIconService(db) diff --git a/src/schemas/template.py b/src/schemas/template.py index 73b74f0..896ec0c 100644 --- a/src/schemas/template.py +++ b/src/schemas/template.py @@ -85,10 +85,9 @@ class TemplateResponse(BaseModel): # `owner_username` are exposed to clients. owner: Any = Field(default=None, exclude=True, repr=False) - # Internes Feld für die ``effective_icon``-Berechnung. Wird von SQLAlchemy + # Internes Feld für die ``icon_path``-Berechnung. Wird von SQLAlchemy # via ``from_attributes=True`` gefüllt, aus der Response aber - # ausgeblendet — Clients bekommen nur ``effective_icon`` / - # ``has_uploaded_icon``. + # ausgeblendet — Clients bekommen nur ``icon_path``. icon: Any = Field(default=None, exclude=True, repr=False) @computed_field # type: ignore[prop-decorator] @@ -117,23 +116,16 @@ def owner_username(self) -> Optional[str]: @computed_field # type: ignore[prop-decorator] @property - def has_uploaded_icon(self) -> bool: - """True wenn ein Icon-Bild via ``POST /templates/{id}/icon`` hochgeladen wurde. + def icon_path(self) -> Optional[str]: + """Relativer API-Pfad zum Icon-Bild, oder ``None``. - Wird aus der ``TemplateIcon``-Relation abgeleitet und ist dasselbe - Signal, das ``effective_icon`` intern nutzt — praktisch fürs - Frontend, um „Icon entfernen"-Buttons konditional zu rendern. - """ - return self.icon is not None - - @computed_field # type: ignore[prop-decorator] - @property - def effective_icon(self) -> Optional[str]: - """Icon-URL für das Frontend, falls ein Bild hochgeladen wurde. + Wenn ein Icon-Bild via ``POST /templates/{id}/icon`` hochgeladen + wurde → ``/api/v1/templates/{id}/icon``. Sonst ``None`` — der + Client rendert dann einen Default-Placeholder. - Wenn ein Icon existiert → ``/api/v1/templates/{id}/icon``. - Sonst ``None`` — der Client rendert dann einen Default-Placeholder. - Externe URLs oder ``mdi:*``-Identifier gibt es nicht mehr. + Bewusst *path*, nicht *url*: der Wert enthält keinen Origin und + muss vom Client gegen die API-Base-URL aufgelöst werden (dieselbe + Base-URL, gegen die auch alle anderen ``/api/v1/*``-Calls laufen). """ if self.icon is not None: return f"/api/v1/templates/{self.id}/icon" @@ -152,8 +144,7 @@ def effective_icon(self) -> Optional[str]: "owner_username": "bberg", "repo_url": "https://github.com/example/flask-template", "visibility": "public", - "has_uploaded_icon": False, - "effective_icon": None, + "icon_path": None, "versions": [], "created_at": "2024-11-27T10:00:00Z", "updated_at": "2024-11-27T10:00:00Z" diff --git a/tests/api/test_template_icon_routes.py b/tests/api/test_template_icon_routes.py index 879ad4c..4689fdf 100644 --- a/tests/api/test_template_icon_routes.py +++ b/tests/api/test_template_icon_routes.py @@ -2,7 +2,7 @@ Deckt POST/GET/DELETE ab, inkl. Content-Type-Whitelist, Größenlimit, Owner/Admin-Gate, sowie das Zusammenspiel mit der TemplateResponse -(``effective_icon`` schaltet nach dem Upload auf die Serve-URL um). +(``icon_path`` zeigt nach dem Upload auf den Serve-Endpoint, sonst null). """ import pytest from fastapi import status @@ -157,9 +157,9 @@ def test_owner_can_upload_png(self, owner_client, sample_template): assert data["template_id"] == sample_template.id assert data["content_type"] == "image/png" assert data["size_bytes"] == len(PNG_1x1) - assert data["url"] == f"/api/v1/templates/{sample_template.id}/icon" + assert data["icon_path"] == f"/api/v1/templates/{sample_template.id}/icon" - def test_upload_updates_effective_icon_in_template_response( + def test_upload_populates_icon_path_in_template_response( self, owner_client, sample_template ): owner_client.post( @@ -169,9 +169,8 @@ def test_upload_updates_effective_icon_in_template_response( get_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}") assert get_resp.status_code == status.HTTP_200_OK body = get_resp.json()["data"] - assert body["has_uploaded_icon"] is True - assert body["effective_icon"] == f"/api/v1/templates/{sample_template.id}/icon" - # icon_url gibt es nicht mehr — nur noch effective_icon / has_uploaded_icon + assert body["icon_path"] == f"/api/v1/templates/{sample_template.id}/icon" + # icon_url gibt es nicht mehr — nur noch icon_path assert "icon_url" not in body def test_upload_svg_rejected_415(self, owner_client, sample_template): @@ -271,11 +270,10 @@ def test_owner_can_delete_icon(self, owner_client, sample_template): # Icon ist danach weg → GET liefert 404. get_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}/icon") assert get_resp.status_code == status.HTTP_404_NOT_FOUND - # ``effective_icon`` ist ohne Upload ``None`` — Frontend rendert Placeholder. + # ``icon_path`` ist ohne Upload ``None`` — Frontend rendert Placeholder. tpl_resp = owner_client.get(f"/api/v1/templates/{sample_template.id}") body = tpl_resp.json()["data"] - assert body["has_uploaded_icon"] is False - assert body["effective_icon"] is None + assert body["icon_path"] is None def test_delete_is_idempotent(self, owner_client, sample_template): """Auch ohne vorher hochgeladenes Icon liefert DELETE 204.""" diff --git a/tests/unit/test_template_effective_icon_schema.py b/tests/unit/test_template_icon_path_schema.py similarity index 64% rename from tests/unit/test_template_effective_icon_schema.py rename to tests/unit/test_template_icon_path_schema.py index 88449a5..a54283f 100644 --- a/tests/unit/test_template_effective_icon_schema.py +++ b/tests/unit/test_template_icon_path_schema.py @@ -1,8 +1,9 @@ -"""Tests für ``effective_icon`` und ``has_uploaded_icon`` auf TemplateResponse. +"""Tests für ``icon_path`` auf TemplateResponse. Nach dem Umbau kennt das Backend nur noch hochgeladene Icon-Bilder; ``mdi:*``/URL-Strings gibt es nicht mehr. Frontend rendert entweder -``effective_icon`` als ```` oder einen Placeholder. +``icon_path`` als ```` (gegen die API-Base-URL aufgelöst) +oder einen Placeholder. """ from datetime import datetime, timezone from types import SimpleNamespace @@ -29,38 +30,36 @@ def _orm_template(**overrides): return SimpleNamespace(**defaults) -class TestEffectiveIcon: - def test_uploaded_icon_returns_serve_url(self): +class TestIconPath: + def test_uploaded_icon_returns_serve_path(self): icon = SimpleNamespace(id="icon-42") response = TemplateResponse.model_validate(_orm_template(icon=icon)) - assert response.effective_icon == "/api/v1/templates/tmpl-1/icon" - assert response.has_uploaded_icon is True + assert response.icon_path == "/api/v1/templates/tmpl-1/icon" def test_no_icon_returns_none(self): - """Ohne Upload ist ``effective_icon`` ``None`` — kein Fallback.""" + """Ohne Upload ist ``icon_path`` ``None`` — kein Fallback.""" response = TemplateResponse.model_validate(_orm_template(icon=None)) - assert response.effective_icon is None - assert response.has_uploaded_icon is False + assert response.icon_path is None class TestSerializedPayloadShape: def test_raw_icon_object_not_leaked_into_json(self): """Die ORM-Icon-Relation darf nicht in die Response wandern — - Clients bekommen nur ``effective_icon`` + ``has_uploaded_icon``.""" + Clients bekommen nur ``icon_path``.""" icon = SimpleNamespace(id="icon-42", content_type="image/png") payload = TemplateResponse.model_validate( _orm_template(icon=icon) ).model_dump(mode="json") assert "icon" not in payload - assert "icon_url" not in payload # Feld existiert nicht mehr - assert payload["effective_icon"] == "/api/v1/templates/tmpl-1/icon" - assert payload["has_uploaded_icon"] is True + assert "icon_url" not in payload # Altes Feld existiert nicht mehr + assert "effective_icon" not in payload # Zwischenname war effective_icon + assert "has_uploaded_icon" not in payload # ebenfalls entfernt + assert payload["icon_path"] == "/api/v1/templates/tmpl-1/icon" def test_json_payload_when_no_upload(self): payload = TemplateResponse.model_validate( _orm_template(icon=None) ).model_dump(mode="json") - assert payload["effective_icon"] is None - assert payload["has_uploaded_icon"] is False + assert payload["icon_path"] is None