diff --git a/design/nfc-phase-1-tag-identity.md b/design/nfc-phase-1-tag-identity.md index 40ceefda..c1b7d0de 100644 --- a/design/nfc-phase-1-tag-identity.md +++ b/design/nfc-phase-1-tag-identity.md @@ -66,13 +66,18 @@ two physical tags with two different UIDs. In `spoolman/database/models.py`, alongside `SpoolField` (`models.py:125`): ```python -class SpoolTag(Base): - __tablename__ = "spool_tag" +class Tag(Base): + __tablename__ = "tag" id: Mapped[int] = mapped_column(primary_key=True, index=True) - spool_id: Mapped[int] = mapped_column(ForeignKey("spool.id"), index=True) - spool: Mapped["Spool"] = relationship(back_populates="tags") uid: Mapped[str] = mapped_column(String(64), unique=True, index=True) + target_type: Mapped[str] = mapped_column(String(16)) + spool_id: Mapped[int | None] = mapped_column(ForeignKey("spool.id"), index=True) + spool: Mapped["Spool | None"] = relationship(back_populates="tags") + filament_id: Mapped[int | None] = mapped_column( + ForeignKey("filament.id", ondelete="CASCADE"), index=True + ) + target_value: Mapped[str | None] = mapped_column(String(64)) format: Mapped[str | None] = mapped_column(String(32)) added: Mapped[datetime] = mapped_column() ``` @@ -84,12 +89,42 @@ class SpoolTag(Base): - `format` — nullable free-ish string (`openprinttag`, `opentag3d`, `ntag`, `bambu`, `tigertag`, `qidi`). Informational in Phase 1; Phase 2 uses it. Not an enum in the DB — new tag types appear faster than migrations should. -- Unique index on `uid` is the point of the whole table: one tag, one spool, enforced. +- Unique index on `uid` is the point of the whole table: one physical tag identifies exactly one + thing, enforced. + +### Why the table is wider than "spool tags" + +Only spools are tagged in Phase 1, and only `POST /spool/{id}/tag` exists. The *table* is general +because the expensive half of this decision is the one that cannot be revisited cheaply: renaming a +table, dropping a NOT NULL and adding a discriminator all mean altering a populated table on four +databases, whereas adding an endpoint later is additive and free. The shape was settled before +release, while the migration had shipped to nobody. + +Two kinds of target, and the difference is the whole reason for the shape: + +- **Targets that are rows** — a spool, later a filament — get real foreign keys, so the database + keeps referential integrity and cascades deletes. A tag row orphaned by a deleted spool is exactly + the class of silent rot this project cannot afford. +- **Targets that are not rows** — a *location* is a `String(64)` on `Spool`, not a table, so a tag + meaning "show me Shelf A" can only ever carry the value. `target_value` holds it, and a new kind of + this sort (a saved search, say) costs **no migration at all**: only a new `target_type` string. + +`target_type` names which is in force rather than leaving readers to infer it from whichever column +is non-null — a kind that populates neither would otherwise be unreadable. + +**No CHECK constraint** enforces "exactly one target". No migration in this tree uses one, and MySQL +below 8.0.16 parses and silently ignores them, so a CHECK here would be real on three databases out +of four. `spoolman/database/tag.py` is the single write path instead — the same argument, and the +same module, as `normalize_uid`. + +**`filament_id` has no ORM relationship yet.** Nothing writes filament tags, and a `selectin` +collection on `Filament` would add a query to every filament listing for rows that cannot exist. +`ON DELETE CASCADE` holds integrity meanwhile; the relationship arrives with the feature. On `Spool`, add the relationship following the `extra` convention at `models.py:94`: ```python -tags: Mapped[list["SpoolTag"]] = relationship( +tags: Mapped[list["Tag"]] = relationship( back_populates="spool", cascade="save-update, merge, delete, delete-orphan", lazy="selectin", @@ -519,7 +554,7 @@ data model, endpoints and protocol above are the spec; this is the file layout a | `spoolman/database/tag.py` | `link()`, `unlink()`, `find_spool_by_uid()` | Follows `database/spool.py` conventions: `AsyncSession` first, raising `ItemNotFoundError` / `ItemCreateError` from `exceptions.py`. Kept out of `spool.py`, which is already ~800 lines and the largest module in the tree. | | `spoolman/scanrelay.py` | Debounce cache and reader registry, as a class | Relay state is not database state and not HTTP state. Isolating it means the debounce window and registry eviction are testable without a socket or a session. | | `spoolman/api/v1/tag.py` | `/tag/scan` POST, `/tag/reader` GET, the two scan websockets | Mirrors the one-router-per-resource layout of `spool.py` / `vendor.py` / `field.py`. Thin: it validates, delegates, and broadcasts. | -| `migrations/versions/_spool_tags.py` | CREATE TABLE + indexes | — | +| `migrations/versions/_tags.py` | CREATE TABLE + indexes | — | `database/tag.py` imports `spool.spool_changed` to emit the update event. One direction only, so no import cycle; the reverse (spool.py knowing about tags) is not needed because the relationship is @@ -529,7 +564,7 @@ declared on the ORM model. | File | Change | |---|---| -| `spoolman/database/models.py` | `SpoolTag` ORM class next to `SpoolField` (`models.py:125`); `Spool.tags` relationship with `lazy="selectin"`. | +| `spoolman/database/models.py` | `Tag` ORM class next to `SpoolField` (`models.py:125`); `Spool.tags` relationship with `lazy="selectin"`. | | `spoolman/api/v1/models.py` | `SpoolTag` response model; `tags` field on `Spool` (`models.py:285`); `SCANNED` on `EventType` (`models.py:604`); `TagScanEvent(Event)` with `resource: Literal["tag_scan"]`. | | `spoolman/database/spool.py` | `find()` gains a `tag: str \| None` kwarg, applied inside `_apply_spool_filters` (`spool.py:259`). | | `spoolman/api/v1/spool.py` | `tag` query param wired into the `find` handler; `POST /spool/{id}/tag` and `DELETE /spool/{id}/tag/{uid}` on the existing router. | @@ -546,7 +581,7 @@ differently-shaped UID. **`?tag=` is a join, not a subquery.** Added in `_apply_spool_filters` so the list query and the count query stay in sync automatically — they share that builder. Because `uid` is unique, at most -one `spool_tag` row can match, so a plain join cannot multiply result rows and the existing +one `tag` row can match, so a plain join cannot multiply result rows and the existing `contains_eager` chain for filament/vendor is unaffected. **Link and unlink emit `SpoolEvent`, not a new event type.** They mutate a spool as far as any client diff --git a/migrations/versions/2026_08_13_1000-fe4970567bb3_spool_tags.py b/migrations/versions/2026_08_13_1000-fe4970567bb3_spool_tags.py deleted file mode 100644 index f67bf486..00000000 --- a/migrations/versions/2026_08_13_1000-fe4970567bb3_spool_tags.py +++ /dev/null @@ -1,50 +0,0 @@ -"""spool tags. - -Revision ID: fe4970567bb3 -Revises: 9c1d5f2a7b31 -Create Date: 2026-08-13 10:00:00.000000 -""" - -import sqlalchemy as sa -from alembic import op - -# revision identifiers, used by Alembic. -revision = "fe4970567bb3" -down_revision = "9c1d5f2a7b31" -branch_labels = None -depends_on = None - - -def upgrade() -> None: - """Create the spool_tag table linking physical NFC/RFID tags to spools.""" - # Purely additive: one new table, no existing column touched, so lossless by - # construction. CREATE TABLE and its indexes ride the same revision, which is the - # pattern the extra-fields tables already established (b8881bdb716c) and which the - # 4-DB suite covers; the DDL/DML split cockroachdb needs (see 304a32906234) does not - # apply here because there is no data migration. - op.create_table( - "spool_tag", - sa.Column("id", sa.Integer(), nullable=False), - sa.Column("spool_id", sa.Integer(), nullable=False), - sa.Column("uid", sa.String(length=64), nullable=False), - sa.Column("format", sa.String(length=32), nullable=True), - sa.Column("added", sa.DateTime(), nullable=False), - sa.ForeignKeyConstraint( - ["spool_id"], - ["spool.id"], - ), - sa.PrimaryKeyConstraint("id"), - ) - op.create_index(op.f("ix_spool_tag_id"), "spool_tag", ["id"], unique=False) - op.create_index(op.f("ix_spool_tag_spool_id"), "spool_tag", ["spool_id"], unique=False) - # The whole point of the table: one tag, one spool, enforced by the database rather - # than by whichever client happened to write last. - op.create_index(op.f("ix_spool_tag_uid"), "spool_tag", ["uid"], unique=True) - - -def downgrade() -> None: - """Perform the downgrade.""" - op.drop_index(op.f("ix_spool_tag_uid"), table_name="spool_tag") - op.drop_index(op.f("ix_spool_tag_spool_id"), table_name="spool_tag") - op.drop_index(op.f("ix_spool_tag_id"), table_name="spool_tag") - op.drop_table("spool_tag") diff --git a/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py b/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py new file mode 100644 index 00000000..2c369be1 --- /dev/null +++ b/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py @@ -0,0 +1,70 @@ +"""tags. + +Revision ID: fe4970567bb3 +Revises: 9c1d5f2a7b31 +Create Date: 2026-08-13 10:00:00.000000 +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "fe4970567bb3" +down_revision = "9c1d5f2a7b31" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """Create the tag table mapping physical NFC/RFID tag UIDs to what they identify.""" + # Purely additive: one new table, no existing column touched, so lossless by + # construction. CREATE TABLE and its indexes ride the same revision, which is the + # pattern the extra-fields tables already established (b8881bdb716c) and which the + # 4-DB suite covers; the DDL/DML split cockroachdb needs (see 304a32906234) does not + # apply here because there is no data migration. + # + # Only spools are tagged today, and the table is deliberately wider than that. A tag + # is a mapping from a UID to "the thing to bring up", and not every such thing is a + # row: a location in Spoolman is a string on a spool rather than a table, so a tag + # meaning "show me Shelf A" can only ever carry the value. Getting that wrong here + # would mean altering a populated table's nullability on four databases later, so + # the shape is settled now, while nothing has shipped. See models.Tag for the full + # reasoning. + op.create_table( + "tag", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("uid", sa.String(length=64), nullable=False), + sa.Column("target_type", sa.String(length=16), nullable=False), + sa.Column("spool_id", sa.Integer(), nullable=True), + sa.Column("filament_id", sa.Integer(), nullable=True), + sa.Column("target_value", sa.String(length=64), nullable=True), + sa.Column("format", sa.String(length=32), nullable=True), + sa.Column("added", sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint( + ["spool_id"], + ["spool.id"], + ), + # Filament tags have no ORM relationship yet (nothing writes them), so the + # database is what stops a deleted filament leaving tag rows behind. + sa.ForeignKeyConstraint( + ["filament_id"], + ["filament.id"], + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index(op.f("ix_tag_id"), "tag", ["id"], unique=False) + op.create_index(op.f("ix_tag_spool_id"), "tag", ["spool_id"], unique=False) + op.create_index(op.f("ix_tag_filament_id"), "tag", ["filament_id"], unique=False) + # The whole point of the table: one physical tag means exactly one thing, enforced by + # the database rather than by whichever client happened to write last. + op.create_index(op.f("ix_tag_uid"), "tag", ["uid"], unique=True) + + +def downgrade() -> None: + """Perform the downgrade.""" + op.drop_index(op.f("ix_tag_uid"), table_name="tag") + op.drop_index(op.f("ix_tag_filament_id"), table_name="tag") + op.drop_index(op.f("ix_tag_spool_id"), table_name="tag") + op.drop_index(op.f("ix_tag_id"), table_name="tag") + op.drop_table("tag") diff --git a/spoolman/api/v1/models.py b/spoolman/api/v1/models.py index 727a2405..c07d6739 100644 --- a/spoolman/api/v1/models.py +++ b/spoolman/api/v1/models.py @@ -88,14 +88,23 @@ class Message(BaseModel): class TagConflictMessage(Message): - """A tag UID is already linked to a different spool. + """A tag UID is already linked to something else. Subclasses Message so the `message` key is where it is in every other error body, and adds the conflicting spool's ID so a client can offer to move the tag there instead of making the user go and find it. + + `spool_id` is optional because a tag identifies one thing and that thing is not always + a spool -- see `models.Tag`. It is absent when the UID is held by something else, in + which case `message` still says what; a client that cannot offer "move it here" without + an id should fall back to reporting the message. """ - spool_id: int = Field(description="The spool the tag is already linked to.", examples=[42]) + spool_id: int | None = Field( + None, + description="The spool the tag is already linked to, if it is a spool that holds it.", + examples=[42], + ) class SettingResponse(BaseModel): diff --git a/spoolman/database/models.py b/spoolman/database/models.py index f645d9d8..0212f404 100644 --- a/spoolman/database/models.py +++ b/spoolman/database/models.py @@ -97,7 +97,7 @@ class Spool(Base): # selectin for the same reason as `extra` above: a spool listing already joins # filament and vendor, and a joined collection here would multiply against the # extra-field rows. - tags: Mapped[list["SpoolTag"]] = relationship( + tags: Mapped[list["Tag"]] = relationship( back_populates="spool", cascade="save-update, merge, delete, delete-orphan", lazy="selectin", @@ -139,24 +139,61 @@ class SpoolField(Base): value: Mapped[str] = mapped_column(Text()) -class SpoolTag(Base): - """A physical NFC/RFID tag linked to a spool. +class Tag(Base): + """A physical NFC/RFID tag, and the thing tapping it should bring up. - Many tags per spool is a real case rather than a hypothetical: copying a Prusa NFC-V + Many tags per target is a real case rather than a hypothetical: copying a Prusa NFC-V tag's payload onto an NTAG215 so a PN532 can read it leaves one spool carrying two physical tags with two different UIDs (#776). - The unique index on `uid` is the point of the table -- one tag, one spool, enforced. - It only means that because every write goes through `spoolman.tags.normalize_uid`; - see the module docstring there. + The unique index on `uid` is the point of the table -- one physical tag means exactly + one thing, enforced. It only means that because every write goes through + `spoolman.tags.normalize_uid`; see the module docstring there. + + ## Why the target is not just `spool_id` + + Only spools are tagged today. The columns are wider than that because the shapes a + tag might point at are not all rows, and finding that out after release would mean + altering a populated table's nullability on four databases: + + * `spool_id` / `filament_id` -- targets that ARE rows, so they are real foreign keys + and the database keeps them honest. + * `target_value` -- targets that are not rows at all. A location in Spoolman is a + string on a spool (`Spool.location`), not a table, so a tag that means "show me + Shelf A" can only carry the value. Adding such a kind later needs no migration: + only a new `target_type`. + + `target_type` says which of those is in force rather than leaving readers to infer it + from whichever column is non-null, so a future kind that populates nothing (or + something new) stays legible. + + Exactly-one-target is enforced in `spoolman.database.tag`, not by a CHECK constraint: + no migration in this tree uses one, and MySQL below 8.0.16 accepts and silently + ignores them, so a constraint here would be real on three databases out of four -- + worse than a single enforced path. Same reasoning as `normalize_uid`. """ - __tablename__ = "spool_tag" + __tablename__ = "tag" id: Mapped[int] = mapped_column(primary_key=True, index=True) - spool_id: Mapped[int] = mapped_column(ForeignKey("spool.id"), index=True) - spool: Mapped["Spool"] = relationship(back_populates="tags") uid: Mapped[str] = mapped_column(String(64), unique=True, index=True) + # Which kind of thing this tag points at; see the class docstring. Values live in + # spoolman.tags. A string rather than an enum for the same reason `format` is one: + # new kinds should not need a schema migration on four databases. + target_type: Mapped[str] = mapped_column(String(16)) + spool_id: Mapped[int | None] = mapped_column(ForeignKey("spool.id"), index=True) + spool: Mapped["Spool | None"] = relationship(back_populates="tags") + # No ORM relationship yet, deliberately. Nothing writes filament tags, and a + # `selectin` collection on Filament would add a query to every filament listing for + # rows that cannot exist. ON DELETE CASCADE keeps the database honest in the + # meantime; the relationship arrives with the feature that needs it. + filament_id: Mapped[int | None] = mapped_column( + ForeignKey("filament.id", ondelete="CASCADE"), + index=True, + ) + # For target kinds addressed by value rather than by row -- a location name. Sized to + # match Spool.location, which is what it would hold. + target_value: Mapped[str | None] = mapped_column(String(64)) # Free-ish string (openprinttag, ntag, bambu, ...) rather than an enum: informational # in phase 1, and new tag types appear faster than migrations should. format: Mapped[str | None] = mapped_column(String(32)) diff --git a/spoolman/database/spool.py b/spoolman/database/spool.py index 909a9e03..0c5494e8 100644 --- a/spoolman/database/spool.py +++ b/spoolman/database/spool.py @@ -293,10 +293,14 @@ def _apply_spool_filters( if tag is not None: # An inner join rather than a subquery, so the list query and the count query stay # in sync automatically: they share this builder. `uid` is unique, so at most one - # spool_tag row can match and the join cannot multiply result rows -- which is why - # the contains_eager chain for filament/vendor is unaffected. - stmt = stmt.join(models.SpoolTag, models.SpoolTag.spool_id == models.Spool.id).where( - models.SpoolTag.uid == normalize_uid(tag), + # tag row can match and the join cannot multiply result rows -- which is why the + # contains_eager chain for filament/vendor is unaffected. + # + # The join condition also does the filtering: a tag that points at something other + # than a spool has a null `spool_id`, which matches no spool, so such a UID finds + # nothing here rather than finding the wrong thing. + stmt = stmt.join(models.Tag, models.Tag.spool_id == models.Spool.id).where( + models.Tag.uid == normalize_uid(tag), ) stmt = add_where_clause_int(stmt, models.Spool.filament_id, filament_id) stmt = add_where_clause_int_opt(stmt, models.Filament.vendor_id, vendor_id) diff --git a/spoolman/database/tag.py b/spoolman/database/tag.py index ae8dc4f2..40caeaf9 100644 --- a/spoolman/database/tag.py +++ b/spoolman/database/tag.py @@ -1,4 +1,4 @@ -"""Helper functions for interacting with spool tag database objects. +"""Helper functions for interacting with tag database objects. Kept out of `spool.py`, which is already the largest module in the tree, but following its conventions exactly: `AsyncSession` first, exceptions from `spoolman.exceptions`, @@ -8,6 +8,14 @@ This module is the ONE place a UID gets normalized, on the way in and on every l Doing it in a Pydantic validator would read better but would only cover callers that arrive over HTTP -- and the unique constraint is worthless if any other path (an importer, a migration backfill, a future tag codec) can write a differently-shaped UID. + +It is also the one place that decides what a tag points at. `models.Tag` can address +things that are not spools -- and things that are not rows at all, such as a location -- +but only spool tags are written today, so `link` sets `target_type` and every read here +is explicit about wanting a spool rather than assuming the tag it found is one. The +database has no CHECK enforcing "exactly one target": no migration in this tree uses one +and MySQL below 8.0.16 silently ignores them, so a single enforced write path is worth +more than a constraint that is real on three databases out of four. """ import logging @@ -20,24 +28,39 @@ from sqlalchemy.ext.asyncio import AsyncSession from spoolman.api.v1.models import EventType from spoolman.database import models, spool from spoolman.exceptions import ItemNotFoundError, TagConflictError -from spoolman.tags import normalize_format, normalize_uid +from spoolman.tags import TARGET_SPOOL, normalize_format, normalize_uid logger = logging.getLogger(__name__) -async def _get_tag_by_uid(db: AsyncSession, uid: str) -> models.SpoolTag | None: +async def _get_tag_by_uid(db: AsyncSession, uid: str) -> models.Tag | None: """Get the tag with this exact normalized UID, if any. Hits the unique index.""" - stmt = sqlalchemy.select(models.SpoolTag).where(models.SpoolTag.uid == uid) + stmt = sqlalchemy.select(models.Tag).where(models.Tag.uid == uid) return (await db.execute(stmt)).scalar_one_or_none() +def _conflict(uid: str, existing: models.Tag) -> TagConflictError: + """Describe a UID that is already spoken for. + + Only spool tags are written today, so the second branch is unreachable in practice -- + but the table is deliberately able to hold other kinds (see `models.Tag`), and a + conflict that reported a spool id of `None` would be worse than one that says plainly + what holds the tag. The id is included only when there genuinely is one, which is what + lets a client offer "move it here" and fall back to reporting the message otherwise. + """ + if existing.spool_id is not None: + return TagConflictError(f"Tag {uid} is already linked to spool {existing.spool_id}.", existing.spool_id) + target = existing.target_value if existing.target_value is not None else existing.filament_id + return TagConflictError(f"Tag {uid} is already linked to {existing.target_type} {target}.") + + async def link( *, db: AsyncSession, spool_id: int, uid: str, tag_format: str | None = None, -) -> models.SpoolTag: +) -> models.Tag: """Link a physical tag to a spool. Re-linking a UID to the spool that already holds it is idempotent; if the request @@ -51,7 +74,7 @@ async def link( tag_format: Optional tag format name, e.g. "ntag". Returns: - models.SpoolTag: The linked tag. + models.Tag: The linked tag. Raises: ItemNotFoundError: If no spool with that ID exists. @@ -67,18 +90,16 @@ async def link( existing = await _get_tag_by_uid(db, uid) if existing is not None: if existing.spool_id != spool_id: - raise TagConflictError( - f"Tag {uid} is already linked to spool {existing.spool_id}.", - existing.spool_id, - ) + raise _conflict(uid, existing) if tag_format is not None and existing.format != tag_format: existing.format = tag_format await db.commit() await spool.spool_changed(db_spool, EventType.UPDATED) return existing - tag = models.SpoolTag( + tag = models.Tag( uid=uid, + target_type=TARGET_SPOOL, format=tag_format, added=datetime.utcnow().replace(microsecond=0), ) @@ -93,10 +114,7 @@ async def link( winner = await _get_tag_by_uid(db, uid) if winner is None: raise - raise TagConflictError( - f"Tag {uid} is already linked to spool {winner.spool_id}.", - winner.spool_id, - ) from None + raise _conflict(uid, winner) from None await spool.spool_changed(db_spool, EventType.UPDATED) return tag @@ -147,6 +165,9 @@ async def find_spool_by_uid(db: AsyncSession, uid: str) -> models.Spool | None: """ tag = await _get_tag_by_uid(db, normalize_uid(uid)) - if tag is None: + # A known tag that points at something other than a spool is not a spool match, and + # answering with one would be a lie. `spool_id` carries that on its own: it is null + # for every other kind of target. + if tag is None or tag.spool_id is None: return None return await spool.get_by_id(db, tag.spool_id) diff --git a/spoolman/exceptions.py b/spoolman/exceptions.py index 60df8772..536b577b 100644 --- a/spoolman/exceptions.py +++ b/spoolman/exceptions.py @@ -18,14 +18,19 @@ class SpoolMeasureError(Exception): class TagConflictError(Exception): - """A tag UID is already linked to a different spool. + """A tag UID is already linked to something else. Carries the conflicting spool's ID rather than only a message, so the API can hand it to the client and the client can offer "move the tag to this spool instead" without a second lookup. + + `spool_id` is None when the UID is held by something that is not a spool -- a tag + identifies exactly one thing, and not every such thing is a spool (see + `database.models.Tag`). The message says what in that case, and a client with nothing + to offer beyond it simply reports it. """ - def __init__(self, message: str, spool_id: int) -> None: + def __init__(self, message: str, spool_id: int | None = None) -> None: """Initialize with the message and the ID of the spool already holding the tag.""" super().__init__(message) self.spool_id = spool_id diff --git a/spoolman/tags.py b/spoolman/tags.py index ffbec3cb..b5ba0e3e 100644 --- a/spoolman/tags.py +++ b/spoolman/tags.py @@ -8,21 +8,32 @@ colon-separated lowercase, ESPHome emits ``04-a2-b3``, nfcpy hands you raw bytes most scripts ``.hex()`` into unseparated lowercase. ``normalize_uid`` collapses all of those to one canonical spelling so that the unique -index on ``spool_tag.uid`` actually means "one tag, one spool". Without it the same -physical tag could be linked to three spools under three spellings and no constraint -would notice. +index on ``tag.uid`` actually means "one physical tag identifies one thing". Without it +the same physical tag could be linked to three spools under three spellings and no +constraint would notice. Pure module: no database, no FastAPI. Same shape as ``colors.py`` and ``math.py``. """ import re -# Mirrors the SpoolTag.uid column width, so an over-long UID is a 400 and not a 500. +# Mirrors the Tag.uid column width, so an over-long UID is a 400 and not a 500. UID_MAX_LENGTH = 64 -# Mirrors the SpoolTag.format column width. +# Mirrors the Tag.format column width. FORMAT_MAX_LENGTH = 32 +# What a tag points at, stored in `Tag.target_type`. Only TARGET_SPOOL is ever written +# today; the other two are named here because the column exists for them, and a reserved +# name is cheaper to honour than one invented later by two people differently. +# +# TARGET_SPOOL and TARGET_FILAMENT address a row, and carry a foreign key. TARGET_LOCATION +# addresses a value: a location is a string on a spool rather than a table, so such a tag +# means "the spools whose location is this" and lives in `Tag.target_value`. +TARGET_SPOOL = "spool" +TARGET_FILAMENT = "filament" +TARGET_LOCATION = "location" + # Tag formats Spoolman knows the name of. Deliberately NOT an enum and NOT enforced: # `format` is informational in phase 1, new tag types appear faster than migrations # should, and rejecting an unknown one would make Spoolman the thing standing between a diff --git a/tests/test_tag_uid.py b/tests/test_tag_uid.py index 61290166..7c83d123 100644 --- a/tests/test_tag_uid.py +++ b/tests/test_tag_uid.py @@ -1,6 +1,6 @@ """Tests for tag UID normalization. -The unique index on `spool_tag.uid` only means "one tag, one spool" because every write goes +The unique index on `tag.uid` only means "one tag, one thing" because every write goes through `normalize_uid`. These pin the shapes real readers produce, so a change that stopped collapsing one of them would show up here rather than as two spools quietly claiming the same physical tag. diff --git a/tests_integration/tests/spool/test_tags.py b/tests_integration/tests/spool/test_tags.py index a961394a..9673ac00 100644 --- a/tests_integration/tests/spool/test_tags.py +++ b/tests_integration/tests/spool/test_tags.py @@ -1,6 +1,6 @@ """Integration tests: linking physical NFC/RFID tags to spools, and looking a spool up by one. -The point of the `spool_tag` table is that one tag identifies exactly one spool, enforced by the +The point of the `tag` table is that one tag identifies exactly one thing, enforced by the database rather than by whichever client wrote last. These tests pin that from both ends: the 409 that stops a second spool claiming a tag, and the normalization that stops the same physical tag being linked twice under two spellings.