---
title: "ADR-025: Secret entity is a readonly value object"
manual: "nr-vault"
version: "1.0"
permalink: "https://docs.typo3.org/permalink/netresearch/nr-vault:adr-025-secret-entity-readonly@1.0"
source: "Developer/Adr/ADR-025-SecretEntityReadonly.rst"
rendered: "2026-09-18T07:37:50+00:00"
---

# ADR-025: Secret entity is a readonly value object {#adr-025-secret-entity-is-a-readonly-value-object}

**Table of contents**

-   [Status](https://docs.typo3.org/permalink/netresearch/nr-vault:status@1.0)
-   [Date](https://docs.typo3.org/permalink/netresearch/nr-vault:date@1.0)
-   [Context](https://docs.typo3.org/permalink/netresearch/nr-vault:context@1.0)
-   [Decision](https://docs.typo3.org/permalink/netresearch/nr-vault:decision@1.0)
-   [Consequences](https://docs.typo3.org/permalink/netresearch/nr-vault:consequences@1.0)
-   [Verified](https://docs.typo3.org/permalink/netresearch/nr-vault:verified@1.0)
-   [References](https://docs.typo3.org/permalink/netresearch/nr-vault:references@1.0)

## Status {#status}

Accepted

## Date {#date}

2026-05-22

## Context {#context}

The original `Domain\Model\Secret` was a classic "anaemic + mutable"
entity: 28 fields, 28 `set*()` mutators, a `Secret::create()`
named factory, and a `setUid()` 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:

1.  **Aliased mutation is hard to reason about.** A controller hands
    a `Secret` to 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.
1.  **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.
1.  **Reviewer-found bugs were direct consequences.** A subsequent
    PR (#142 review) surfaced that `SecretCreatedEvent` would
    receive `uid = null` because 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 {#decision}

Convert `Secret` to a fully readonly value object:

-   Constructor promotion with `readonly` on all 28 fields.
-   All `set*()` mutators removed.
-   All `$secret->getX()` accessor methods retained as a
    compatibility shim; new code should use direct property access
    (`$secret->encryptedValue`).
-   Validation moves into the constructor (envelope-encryption triple
    must be all-set-or-all-empty); `Secret::create()` named factory
    removed.

Four named lifecycle transitions remain as `with*()` withers, each
returning a new instance:

-   `withUid(?int)` — repository attaches the post-INSERT UID
-   `withValueRotation(EncryptedData, int $rotatedAt)` — bundles
    the seven fields that change during a value rotation
-   `withReEncryptedDek(string, string)` — master-key rotation
    updates only the DEK envelope
-   `withMetadata(array)` — adapter merge

Each wither delegates to a private `cloneWith(array $changes)`
helper that uses `get_object_vars($this)` spread to avoid the
N×28-arg duplication that would otherwise dominate the file.

### Repository contract change {#repository-contract-change}

`SecretRepositoryInterface::save(Secret $secret): 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 `VaultAdapterInterface::store(Secret $secret): Secret`
so events fired after `adapter->store()` see the populated UID.

Both later gained a `bool $persistGroupRelations = true` 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 {#consequences}

### Positive {#positive}

-   Aliased mutation no longer possible. Defensive cloning becomes
    unnecessary throughout the codebase.
-   Two real bugs uncovered + fixed during the conversion:

    -   `SecretCreatedEvent` UID-null regression that this design
        makes structurally impossible.
    -   Pre-existing missing `cruser_id` column from
        `toDatabaseRow()` (silently `cruser_id = 0` since the
        entity was first written, surfaced via PR #142 review).
-   Tests significantly smaller and clearer:  30 tautological
    "setX/getX round-trip" tests removed; constructor coverage
    subsumes them.

### Negative {#negative}

-   Breaking API change for downstream extensions constructing
    `Secret` directly or calling `set*()` mutators. Migration is
    mechanical (named-args ctor + `with*()`).
-   `$repo->save($secret); $secret->getUid()` no longer works —
    must capture the return value. CHANGELOG entry mandatory.

## Verified {#verified}

-   1711 unit tests pass after conversion (down from 1745 because
    tautological tests deleted; `+369` assertions net because
    surviving tests cover more behaviour per case).
-   Functional master-key rotation test exercises the full
    `withReEncryptedDek → save` round-trip end-to-end.

## References {#references}

-   Pull request: [#142](https://github.com/netresearch/t3x-nr-vault/pull/142)
-   Follow-up: [#143](https://github.com/netresearch/t3x-nr-vault/pull/143)
    (related table-routing audit triggered by this PR's review cycle)