fix(import): skip on content-hash equality alone, not version ordering - #2325
Merged
Merged
Conversation
computeDefinitionHash() already covers the fully-merged configuration — monolith plus every register.d fragment — and is computed UNCONDITIONALLY before the gate is consulted. So an identical hash means importing would write exactly what is stored, and there is nothing to do whatever the version says. The gate additionally required version_compare($version, $stored, '<='). Three apps (opencatalogi, procest, softwarecatalog) append a digest to that version per ADR-037 — `1.2.3+frag.a1b2c3d4` — so a fragment change would force a re-import. But version_compare does not treat `+…` as semver build metadata; it compares it as further version parts, LEXICALLY. The gate therefore fired based on how two md5 hashes happened to sort: incoming 1.0.0+frag.abc12345 vs stored 1.0.0+frag.def67890 -> re-import the same pair reversed -> skip Unchanged content re-imported roughly half the time. A digest has no order, so version_compare was the wrong instrument for it. Correctness is unchanged: a changed fragment changes the merged data, changes the hash, fails this equality, and proceeds to the per-entity gates exactly as before (#426). A never-stored hash also fails it, so older installs heal on their next run. Measured on the dev instance: 4 apps now skip on unchanged content that previously re-imported. NOT sufficient on its own. When an import IS needed it remains pathologically slow, because seeding config objects dispatches the object lifecycle and every installed app's listeners run — 100% CPU, ~950 MB RSS, zero queries. That is a separate fix and `SystemOperationContext::isActive()` is the unused lever for it.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 713/713 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-04 07:38 UTC
Download the full PDF report from the workflow artifacts.
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
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.
computeDefinitionHash()already covers the fully-merged configuration — monolith plus everyregister.dfragment — and is computed unconditionally before the gate is consulted (ImportHandler ~1972). So an identical hash means importing would write exactly what is already stored.The gate additionally required
version_compare($version, $stored, '<='). Three apps (opencatalogi, procest, softwarecatalog) append a digest to that version per ADR-037 —1.2.3+frag.a1b2c3d4— so that a fragment change forces a re-import. Butversion_comparedoes not treat+…as semver build metadata; it compares it as further version parts, lexically:So the decision hinged on how two md5 hashes happened to sort. Unchanged content re-imported about half the time; a genuine change was skipped the other half, caught only by the content-differs fallback. A digest has no order.
Correctness is unchanged. A changed fragment changes the merged data → changes the hash → fails this equality → proceeds to the per-entity gates exactly as before (#426). A never-stored hash also fails it, so older installs heal on their next run.
Measured: 4 apps now skip on unchanged content that previously re-imported.
This is not sufficient on its own
When an import IS needed it remains pathologically slow. OpenCatalogi's payload is 1 register, 10 schemas, 12 objects, 59 KB — and it runs 12+ minutes at 100% CPU, ~950 MB RSS, zero database queries. Sampled from the log mid-import:
Seeding config objects dispatches the object lifecycle, so every installed app's listeners run — document extraction and compliance checks for objects that are merely being seeded.
importFromApp()already wraps its work inSystemOperationContext::run(), andSystemOperationContext::isActive()is public. No listener in any app checks it. That is the lever for the follow-up.Companion PRs dropping the now-pointless digest suffix: opencatalogi and procest
fix/drop-frag-digest-from-version.