Conversation
McHunkyTrunk
requested changes
Aug 10, 2026
McHunkyTrunk
left a comment
McHunkyTrunk
left a comment
Contributor
There was a problem hiding this comment.
Von der Beschreibung / Architektur her klingt alles sehr plausibel. Opus-5-Review im Anhang.
hermann-p
reviewed
Aug 11, 2026
smnhgn
force-pushed
the
GRUD_DEV-1199/link-attributes
branch
from
August 17, 2026 11:47
55ea0c1 to
1dd1c9c
Compare
smnhgn
force-pushed
the
GRUD_DEV-1199/link-attributes
branch
from
August 17, 2026 12:51
1143e53 to
87816a8
Compare
## Critical
- **Validate `formatPattern` when only `linkAttributes` changes.** The check ran only on
`formatPattern.isDefined`, so renaming or clearing an attribute left a stored pattern pointing at a
`{{attributes.<name>}}` token that no longer existed. Both directions are now validated against the
effective pair after the change. Not covered by decision: `linkAttributes` live on the link (shared
with the backlink column) while `formatPattern` lives on the column, so a change from one side is
still not validated against a pattern on the other — documented in code and swagger.
- **Reject multilanguage link attributes on tables without langtags (422).** `langtags: []` is a
supported configuration, but with an empty langtag list the reshape replaced every stored value
with `{}` or `null` and committed successfully. Rejected on create and change; the reshape asserts
the invariant too, because the failure mode was silent data loss.
- **Validate langtag keys on write, and collapse langtag-agnostically.** Unknown keys are rejected
against the table's langtags. The multilanguage → language-neutral collapse now walks the stored
object via `jsonb_each` (ordered by `array_position`) instead of the table's langtag list, so a
value under a since-removed langtag survives instead of being dropped silently.
## Important
- Re-assert `maxCount` and the multilanguage invariant in `updateLinkAttributesDefinition` — the
migrations only ever touch position 0, so a second entry would be persisted and never migrated.
- Persist and return a link attribute's `description`; it was parsed but dropped on serialization,
and that JSON is what gets stored, so it was lost for good.
- Skip the link-attribute history query for columns without definitions (a guaranteed-empty round
trip per link column per change for the entire existing stock).
- Add `WHERE attributes IS NOT NULL` to the attribute-wipe UPDATE.
- Invalidate dependent column caches only on structure changes, gated on the existing
`isAtLeastOneStructureProperty` flag.
- Normalize `date`/`datetime`: writes via Joda, kind migrations via the same `TO_CHAR` formats the row
projections use. Two spellings of one instant could previously coexist, and a migration silently
changed a value's format.
## Minor
- Reduce `parseArrayElements` to `map(extractLinkValue)` (it re-matched, and rejected `Long` where
the `Seq[_]` path accepted it).
- Replace `asInstanceOf` in the value validator with explicit type expectations, so clients no longer
get a raw `ClassCastException` message.
- Treat an empty `attributes` array as no value instead of storing `[]`.
- Guard `updateLinkAttributesDefinition` against non-link columns (was an NPE on a NULL `link_id`).
- Move link-attribute structure tests into `ChangeLinkAttributesStructureTest`; `ChangeStructureTest`
is byte-identical to master again.
- Harden the `schema_v43` backfill: filter `table_type = 'BASE TABLE'` and resolve via
`format('%I.%I', ...)` instead of a `search_path`-dependent unqualified cast.
- Carry only `attributes` over when rebuilding a concat-target link row, instead of every projection
field — the previous version newly surfaced `final`/`archived` there, and inconsistently.
## Tests
20 new tests, one per fix. `dateTimeValueIsNormalizedIdenticallyByWriteAndMigration` writes the same
instant directly and via a kind migration and compares them — it is the only thing keeping the Joda
format in `LinkAttributeValueValidator` and the `TO_CHAR` format in `ModelHelper` in sync.
## Critical - **Restore the read-back-writable invariant.** The write-side per-key langtag validation is gone. A value belongs to the link, which two tables with differing langtag sets share, and a langtag can be removed from a table after a value was stored under it — so a stored key is not necessarily one the addressed side would accept today. Rejecting it made a value the API hands out unwritable and broke `duplicateRow` (it re-writes the values it just read) with a 400, reachable without anyone reconfiguring anything. Keys nobody can address anymore stay handled where it is safe: the multilanguage collapse walks the stored object instead of the langtag list. - **Evaluate the langtag guards against the link, not the addressed column.** `linkAttributes` live in `system_link_table` and are shared by both sides, so `checkMultilanguageAllowed` and the multilanguage flip now use the union of both linked tables' langtags (`retrieveLinkLangtags`). Before, the answer depended on which side the request came through, which locked the backlink side out of editing a valid definition — a pure rename was rejected with 422 whenever the other table had no langtags of its own. The flip had to move to the same set: a narrower one would push a value into langtags one side cannot read, or trip the assertion on a definition just accepted. ## Important - Correct the cache-invalidation comment: `structureProperties` contains `columnName` and `ordering`, so a rename does still invalidate. The gating only spares display-only changes. - Correct swagger: 422 fires only when *neither* linked table has langtags, and langtag keys of a stored value are deliberately unrestricted. ## Incidental - Drop the now-dead `checkValidValue` wrapper, `LinkColumn.tableLangtags`, and `normalize`'s `allowedLangtags` parameter whose default silently disabled validation. - Move `reshapeLinkAttributeValues` from `table: Table` to `langtags: Seq[String]`. - Resolve the link's langtags in `change` *before* `connection.begin()` — `retrieveLinkInformation` reads on its own connection, which must not happen while holding a transaction. - Run both definition guards inside a `Future` instead of `_ =`. The caller applies `recoverWith(t.rollbackAndFail())` to the returned Future, so a synchronously escaping throw would have skipped the rollback and left the transaction open. ## Tests Removed the two tests that pinned the wrong semantics (`rejectUnknownLangtagInAttributeValue`, `rejectUnknownLangtagOnPutAttributesEndpoint`). Added six that fail without these fixes: - `attributeValueUnderForeignLangtagIsAccepted` - `attributeValueUnderRemovedLangtagStaysWritable` — read, write back verbatim, then `duplicateRow` - `attributeValueSurvivesDuplicateOnSideWithNarrowerLangtags` — two tables with different langtag sets, nothing reconfigured - `createMultilanguageLinkAttributeSucceedsWhenOnlyOneSideHasLangtags` - `changeLinkAttributesFromBacklinkSideWithoutOwnLangtagsSucceeds` - `multilanguageFlipFromBacklinkSideUsesTheLinksLangtags` Not covered: that a display-only change does *not* trigger the dependency walk — "was not invalidated" is not observable over HTTP without inspecting cache state directly.
14 tasks
Contributor
|
@smnhgn Opus5-Update. |
McHunkyTrunk
self-requested a review
September 2, 2026 12:25
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submit a pull request
Related Ticket: GRUD_DEV-1199
Please make sure the following is true
Other information/comments (e.g. reasons why points are not checked from above)
Link-Attribute (
linkAttributes)Eine Link-Verbindung (Kante zwischen zwei Zeilen) kann jetzt eigene Werte tragen — z. B. einen
Prozentanteil, ein Datum oder einen Freitext pro Verknüpfung.
linkAttributes) und ist damit für Hin- undBacklink-Spalte identisch.
attributesim Zellwert) und sind von beidenSeiten lesbar und editierbar.
Ein Link-Attribut ist bewusst keine echte Spalte: keine
id, keineordering, kein Eintrag insystem_columns— es wird ausschließlich über seinennamereferenziert.API
Struktur
linkAttributeskann beim Anlegen einer Link-Spalte mitgegeben und überPOSTauf die Spaltenachträglich geändert werden. Feld weglassen = unverändert, leeres Array = Definition und alle
gespeicherten Werte löschen.
kinds:text,numeric,integer,boolean,date,datetime. Namen müssen\w+entsprechen und innerhalb der Spalte eindeutig sein.formatPatterngibt es jetzt auch für Link-Spalten, mit den Platzhaltern{{value}}und{{attributes.<name>}}. Pattern und Definitionen werden in beide Richtungen gegeneinandervalidiert; damit ein Pattern gelöscht werden kann (Voraussetzung fürs Leeren der Definitionen),
wird
formatPattern: nullals „löschen" akzeptiert.Kind-Wechsel castet,
multilanguage-Wechsel reshaped, Hinzufügen/Entfernen von Definitionenpasst die Länge der positionsbasierten Wert-Arrays an. Ein nicht castbarer Wert lässt die
gesamte Änderung fehlschlagen — analog zum Kind-Wechsel einer normalen Spalte.
Werte
PUT /tables/{tableId}/columns/{columnId}/rows/{rowId}/link/{linkId}/attributesstatt einer nackten ID ein Objekt
{ "id": …, "attributes": [...] }sein, ebenso{ "to": …, "attributes": [...] }.attributes[i]↔linkAttributes[i]), gegen dieDefinition validiert, und
date/datetimewerden normalisiert (datetime nach UTC).nullist für jeden Kind erlaubt und bedeutet „geleert"; das unterscheidet sich von einemkomplett fehlenden
attributes(„nichts gespeichert").Weiteres Verhalten
Attributes-Endpunkt schreibt Historie.
invalidiert (Backlink-Seite, Concat, Group-Spalten) — vorher konnten dort veraltete Zellwerte
weiterausgeliefert werden.
linkAttributesverhalten sich exakt wie bisher;weder im Column- noch im Cell-Response taucht ein neues Feld auf. Alle bisherigen
Request-Formen für Link-Werte bleiben unverändert gültig.
in der Zieltabelle.
Persistenz
schema_v43:system_link_table.attributes(Definition) sowie eineattributes-Spalte auf jederlink_table_<linkId>(Werte). Bestehende Link-Tabellen werden per Migration nachgezogen, neuebekommen die Spalte direkt beim Anlegen.
Rollout-Grenzen (absichtlich, vorläufig)
Zwei Grenzen sind Rollout-Entscheidungen, keine strukturellen Limits — der Code dahinter existiert
und funktioniert, das Frontend ist nur noch nicht bereit dafür:
400,error.json.linkAttributes).multilanguage: truewird abgelehnt (400,error.json.linkAttributes).Beide sind über einen Test-Seam abschaltbar, damit die N-Definitions- und Multilanguage-Pfade nicht
zu unerreichbarem Code werden; über HTTP verhalten sie sich wie Konstanten. Tests decken sowohl das
Verhalten mit gesetzten Grenzen als auch die Pfade dahinter ab.
Dokumentation