ADR-025: Secret entity is a readonly value object
Table of contents
Status
Accepted
Date
2026-05-22
Context
The original Domain\ was a classic "anaemic + mutable"
entity: 28 fields, 28 set* mutators, a Secret::
named factory, and a set mutator that the repository called
after INSERT to write back the auto-assigned UID into the
caller's reference.
The multi-axis review (H-5) flagged this for three reasons:
- Aliased mutation is hard to reason about. A controller hands
a
Secretto a service, the service mutates it, the repository mutates it again, an event listener mutates it once more — anyone holding the reference observes silent state changes. - The pattern leaks into wider TYPO3 code. Downstream extensions consuming the entity inherit the mutable contract: they need to defensively clone or risk shared-state bugs.
- Reviewer-found bugs were direct consequences. A subsequent
PR (#142 review) surfaced that
Secretwould receiveCreated Event uid = nullbecause the event dispatch happened against the pre-save instance — a regression that would have been impossible if the repository's write-back returned a new instance instead of mutating the input.
Decision
Convert Secret to a fully readonly value object:
- Constructor promotion with
readonlyon all 28 fields. - All
set*mutators removed.() - All
$secret->getaccessor methods retained as a compatibility shim; new code should use direct property access (X () $secret->encrypted).Value - Validation moves into the constructor (envelope-encryption triple
must be all-set-or-all-empty);
Secret::named factory removed.create ()
Four named lifecycle transitions remain as with* withers, each
returning a new instance:
with— repository attaches the post-INSERT UIDUid (?int) with— bundles the seven fields that change during a value rotationValue Rotation (Encrypted Data, int $rotated At) with— master-key rotation updates only the DEK envelopeRe Encrypted Dek (string, string) with— adapter mergeMetadata (array)
Each wither delegates to a private clone
helper that uses get_ spread to avoid the
N×28-arg duplication that would otherwise dominate the file.
Repository contract change
Secret (was void).
On INSERT the returned instance carries the freshly-assigned UID;
on UPDATE the original is returned. Callers MUST capture the
return value if they need the UID — the input is readonly.
Same shape applied to Vault
so events fired after adapter->store see the populated UID.
Both later gained a bool $persist parameter for the
two-tier MM handling. Passing false leaves the record's group tiers
untouched — MM rows and count columns alike — which the FormEngine completion
path needs so it does not overwrite ACL relations DataHandler has already
written. The decision recorded here, returning a new instance rather than
mutating a readonly one, is unaffected.
Consequences
Positive
- Aliased mutation no longer possible. Defensive cloning becomes unnecessary throughout the codebase.
-
Two real bugs uncovered + fixed during the conversion:
SecretUID-null regression that this design makes structurally impossible.Created Event - Pre-existing missing
cruser_column fromid to(silentlyDatabase Row () cruser_since the entity was first written, surfaced via PR #142 review).id = 0
- Tests significantly smaller and clearer: 30 tautological "setX/getX round-trip" tests removed; constructor coverage subsumes them.
Negative
- Breaking API change for downstream extensions constructing
Secretdirectly or callingset*mutators. Migration is mechanical (named-args ctor +() with*).() $repo->saveno longer works — must capture the return value. CHANGELOG entry mandatory.($secret); $secret->get Uid ()
Verified
- 1711 unit tests pass after conversion (down from 1745 because
tautological tests deleted;
+369assertions net because surviving tests cover more behaviour per case). - Functional master-key rotation test exercises the full
withround-trip end-to-end.Re Encrypted Dek → save