From 7e4d2ee419d44c8989ee7f2ed79ebd5ef32c8080 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Sun, 2 Aug 2026 00:22:27 -0700 Subject: [PATCH] fix(api): repair filament rows already stored with a leading # The validator fix only guards the write path. Instances that already hit #780 keep 500ing on GET /filament because the stored value exceeds the response model's max_length, and the UI can't repair what it can't list. Filament.from_db now strips the # and returns None when the stored value still isn't a valid 6/8 character code, so a single bad row no longer takes down the whole list. A migration chained from 415a8f855e14 rewrites existing rows one by one through op.get_bind() rather than with SQL string functions, so it behaves the same on sqlite, postgres, mysql and cockroachdb. downgrade() is a no-op. --- ..._0900-9c1d5f2a7b31_strip_color_hex_hash.py | 51 ++++++++++++++++ spoolman/api/v1/models.py | 29 ++++++++- tests/test_color_hex_validation.py | 59 +++++++++++++++++++ 3 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/2026_08_02_0900-9c1d5f2a7b31_strip_color_hex_hash.py diff --git a/migrations/versions/2026_08_02_0900-9c1d5f2a7b31_strip_color_hex_hash.py b/migrations/versions/2026_08_02_0900-9c1d5f2a7b31_strip_color_hex_hash.py new file mode 100644 index 00000000..89f86411 --- /dev/null +++ b/migrations/versions/2026_08_02_0900-9c1d5f2a7b31_strip_color_hex_hash.py @@ -0,0 +1,51 @@ +"""strip color hex hash. + +Revision ID: 9c1d5f2a7b31 +Revises: 415a8f855e14 +Create Date: 2026-08-02 09:00:00.000000 +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "9c1d5f2a7b31" +down_revision = "415a8f855e14" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """Strip leading # characters from stored filament color codes.""" + """Rows are rewritten one by one through the connection rather than with SQL""" + """string functions, so this behaves identically on sqlite, postgres, mysql""" + """and cockroachdb (see 304a32906234 for the cockroachdb caveat).""" + filament = sa.Table( + "filament", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("color_hex", sa.String(length=8), nullable=True), + sa.Column("multi_color_hexes", sa.String(length=128), nullable=True), + ) + + connection = op.get_bind() + rows = connection.execute( + sa.select(filament.c.id, filament.c.color_hex, filament.c.multi_color_hexes), + ).fetchall() + + for row in rows: + values = {} + + if row.color_hex is not None and "#" in row.color_hex: + values["color_hex"] = row.color_hex.replace("#", "") + + if row.multi_color_hexes is not None and "#" in row.multi_color_hexes: + values["multi_color_hexes"] = row.multi_color_hexes.replace("#", "") + + if values: + connection.execute(sa.update(filament).where(filament.c.id == row.id).values(**values)) + + +def downgrade() -> None: + """Perform the downgrade.""" + # The leading # was never valid, so there is nothing to restore. diff --git a/spoolman/api/v1/models.py b/spoolman/api/v1/models.py index ba6a0b84..b47636de 100644 --- a/spoolman/api/v1/models.py +++ b/spoolman/api/v1/models.py @@ -21,6 +21,31 @@ def datetime_to_str(dt: datetime) -> str: SpoolmanDateTime = Annotated[datetime, PlainSerializer(datetime_to_str)] +def _sanitize_color_hex(value: str | None) -> str | None: + """Normalize a color code read from the database. + + Older releases could store a value with a leading ``#`` (see #780). Strip it and + drop anything that still isn't a valid 6 or 8 character code, so that one bad row + doesn't make the whole filament list unserializable. + """ + if not value: + return None + clr = value.upper().removeprefix("#") + if len(clr) not in (6, 8) or any(c not in "0123456789ABCDEF" for c in clr): + return None + return clr + + +def _sanitize_multi_color_hexes(value: str | None) -> str | None: + """Normalize a comma-separated list of color codes read from the database.""" + if not value: + return None + colors = [c for c in (_sanitize_color_hex(part) for part in value.split(",")) if c is not None] + if len(colors) < 2: # noqa: PLR2004 + return None + return ",".join(colors) + + def _extra_fields_description(entity: str) -> str: r"""Build the description for an entity's ``extra`` field. @@ -242,8 +267,8 @@ class Filament(BaseModel): comment=item.comment, settings_extruder_temp=item.settings_extruder_temp, settings_bed_temp=item.settings_bed_temp, - color_hex=item.color_hex, - multi_color_hexes=item.multi_color_hexes, + color_hex=_sanitize_color_hex(item.color_hex), + multi_color_hexes=_sanitize_multi_color_hexes(item.multi_color_hexes), multi_color_direction=( MultiColorDirection(item.multi_color_direction) if item.multi_color_direction is not None else None ), diff --git a/tests/test_color_hex_validation.py b/tests/test_color_hex_validation.py index 8c8f3bdb..7822cf87 100644 --- a/tests/test_color_hex_validation.py +++ b/tests/test_color_hex_validation.py @@ -5,11 +5,14 @@ original string, so a ``#``-prefixed 8-digit color was stored with 9 characters fit the ``String(8)`` column, breaking every later read of that filament. """ +from datetime import datetime, timezone +from types import SimpleNamespace from typing import Any import pytest from spoolman.api.v1.filament import FilamentParameters +from spoolman.api.v1.models import Filament, _sanitize_color_hex, _sanitize_multi_color_hexes def _params(**kwargs: Any) -> FilamentParameters: # noqa: ANN401 @@ -44,3 +47,59 @@ def test_multi_color_hexes_strips_leading_hash(value: str, expected: str): def test_color_hex_fits_the_database_column(): assert len(_params(color_hex="#FF000000").color_hex) <= 8 + + +@pytest.mark.parametrize( + ("stored", "expected"), + [ + ("FF0000", "FF0000"), + ("#FF0000", "FF0000"), + ("#FF000000", "FF000000"), + ("#ff000000", "FF000000"), + ("nonsense", None), + ("#12345", None), + (None, None), + ("", None), + ], +) +def test_from_db_sanitizes_color_hex(stored: str | None, expected: str | None): + assert _sanitize_color_hex(stored) == expected + + +@pytest.mark.parametrize( + ("stored", "expected"), + [ + ("FF0000,00FF00", "FF0000,00FF00"), + ("#FF0000,#00FF00", "FF0000,00FF00"), + ("#FF0000,garbage", None), + ("#FF0000", None), + (None, None), + ], +) +def test_from_db_sanitizes_multi_color_hexes(stored: str | None, expected: str | None): + assert _sanitize_multi_color_hexes(stored) == expected + + +def test_from_db_survives_a_row_written_before_the_fix(): + item = SimpleNamespace( + id=1, + registered=datetime(2024, 1, 1, tzinfo=timezone.utc), + name="broken", + vendor=None, + material=None, + price=None, + density=1.24, + diameter=1.75, + weight=None, + spool_weight=None, + article_number=None, + comment=None, + settings_extruder_temp=None, + settings_bed_temp=None, + color_hex="#FF000000", + multi_color_hexes=None, + multi_color_direction=None, + external_id=None, + extra=[], + ) + assert Filament.from_db(item).color_hex == "FF000000"