mirror of
https://github.com/Donkie/Spoolman.git
synced 2026-10-22 01:52:17 -05:00
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.
This commit is contained in:
1 parent
57f2b30a4c
commit
7e4d2ee419
3 files changed
+137
-2
No files matched your search
@@ -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.
|
||||
@@ -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
|
||||
),
|
||||
|
||||
@@ -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"
|
||||
Reference in new issue
Block a user