Skip to content

A repair step's descriptor version is a hand-maintained duplicate, and three of nine have drifted #3452

Description

@rubenvdlinde

Found while fixing the migration/version gate (#3451). Same shape, different layer.

What is true today

Eight Repair steps import a register descriptor through
ConfigurationService::importFromApp(..., version: ...), whose gate is
version_compare with force: false. The version each one passes is a
hand-written PHP constant, not the descriptor's own info.version:

private const REGISTER_PATH    = '/lib/Settings/credential_broker_register.json';
private const REGISTER_VERSION = '1.0.0';

Two numbers describing one artefact, kept in step by hand. Measured on
development at 229a04d, three of the nine (descriptor, version) pairs have
already drifted:

repair step constant descriptor info.version
ImportCredentialBrokerRegister 1.0.0 1.4.0
ImportDsarRegistersdata_subject_request_register.json 1.2.0 1.0.0
ImportDsarRegistersdsar_policy_pack_register.json 1.1.0 1.0.0
the other six match match

Note they drift in both directions, which is what says this is a maintenance
gap rather than one forgotten edit.

Why it matters

  • The constant is what gates the import. The descriptor's info.version is
    decorative on that path — so editing the JSON and bumping the version inside it
    changes nothing about whether the change reaches an existing instance.
  • But info.version is not decorative everywhere:
    RegisterDescriptorService::inventory() reads descriptor versions to decide
    current vs behind for the admin panel. The panel and the importer can
    therefore disagree about which version an instance is on, in either direction.
  • The whole-configuration skip no longer depends on this (it compares a content
    hash since Added property close-after-click to every NcActionButton #426), which removes the worst case. The per-register and
    per-schema
    gates inside ImportHandler (L844, L998, L1873, L2872, L3491)
    still compare versions with no content-diff escape — the dossiq#1786 shape:
    the code moves, the version does not, the change never lands.

What this needs, and why it is not in #3451

A ruling first, then a mechanism. Three candidate numbers exist per descriptor
(the PHP constant, info.version, and each register's/schema's own version),
and deciding which is canonical is a design decision, not a gate:

  1. Derive — drop the constant and pass $data['info']['version']. One
    number. Note this changes behaviour for the credential broker: its gate value
    would move 1.0.01.4.0, which re-imports the descriptor on the next
    upgrade. That is the correct direction (the content did change), but it is a
    behaviour change and wants its own verification.
  2. Assert equality — keep both and add a diff-free unit test that every
    repair step's version equals its descriptor's info.version. Reds today on
    the three rows above; the fix is three one-line edits.
  3. Content hash, as Added property close-after-click to every NcActionButton #426 did one layer up — the strongest answer, and the
    largest change.

Option 2 is the cheapest thing that stops the drift growing; option 1 or 3 is the
thing that ends it.

Reproduce the measurement

for f in lib/Repair/*Register*.php; do
  p=$(grep -oP "(?<=REGISTER_PATH = ')[^']+" $f | head -1)
  v=$(grep -oP "(?<=REGISTER_VERSION = ')[^']+" $f | head -1)
  [ -n "$p" ] && printf "%-45s const=%-8s json=%s\n" "$(basename $f)" "$v" \
    "$(python3 -c "import json;print(json.load(open('.$p'))['info']['version'])")"
done

ImportDsarRegisters holds its pairs in a REGISTERS const map rather than two
constants, so it needs reading by hand.

Refs #3451

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

triageAwaiting triage

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions