fix: resolve cross-entity slug references during configuration import - #727
Conversation
Imported configurations that reference entities created earlier in the same run would silently drop those references because the in-memory slug/ID maps were only populated from pre-existing DB rows. ConfigurationService gains registerImportedEntityMapping() to update the maps after each entity import, withComponentSlug() to normalise missing slug fields, and reconcileImportedReferences() for a targeted second pass over endpoints and synchronizations. RuleHandler adds 'synchronization' to its entity-type lists so synchronization references in rule configs are correctly serialised and deserialised. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iliation - Remove dead method_exists(getId) guard — getId() is on the base Entity class and is always present; only getSlug() warrants the check. - Drop resetMappings() before reconcileImportedReferences(): the in-memory maps built by registerImportedEntityMapping during the first pass are already complete, so a full DB reload is unnecessary and caused every endpoint/synchronization to be written twice. - Add rules to the reconciliation pass — rules are imported at step 3, before synchronizations (step 5), so synchronization references in rule configs could not be resolved in the first pass. - Remove synchronizations from the reconciliation pass — synchronizations depend only on sources, mappings and endpoints, all of which are imported before them, so they never have unresolvable forward references. - Remove redundant registerImportedEntityMapping calls inside reconcileImportedReferences; the in-memory maps are already authoritative by that point. - Rewrite reconcileImportedReferences docblock to explain the dependency reasoning instead of re-stating the implementation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| $ruleData = $this->withComponentSlug($ruleData, $ruleSlug); | ||
| $rule = $this->handlers['rule']->import($ruleData, $this->mappings); | ||
| $result['rules'][$ruleSlug] = $rule; | ||
| $this->registerImportedEntityMapping('rule', $rule); |
There was a problem hiding this comment.
🟡 Concern — registerImportedEntityMapping calls for 'rule' and 'job' are dead code
$this->mappings only contains buckets initialised by resetMappings() (source, mapping, endpoint, synchronization, and similar mapper-backed types). There is no 'rule' or 'job' bucket, so both calls hit the early-return guard (isset($this->mappings[$entityType]) === false) and silently no-op. The guard prevents corruption, but the calls imply rule and job slugs participate in cross-entity slug resolution — which they currently do not.
Same issue applies to the registerImportedEntityMapping('job', $job) call a few lines below.
Either remove both dead calls, or — if rule/job slugs are intended to be resolvable in the future — add the corresponding buckets to resetMappings().
| if (isset($components['endpoints']) && is_array($components['endpoints'])) { | ||
| foreach ($components['endpoints'] as $endpointSlug => $endpointData) { | ||
| $endpointData = $this->withComponentSlug($endpointData, $endpointSlug); | ||
| $result['endpoints'][$endpointSlug] = $this->handlers['endpoint']->import($endpointData, $this->mappings); |
There was a problem hiding this comment.
🟡 Concern — reconcileImportedReferences doesn't re-register updated endpoint mappings after the second-pass import
After re-importing endpoints here, registerImportedEntityMapping is not called on the updated entity. If the upsert produces a freshly-assigned ID (e.g. the first-pass import failed to fully resolve the entity and the second pass creates it), $this->mappings['endpoint'] will be stale.
No current downstream step reads from the endpoint mapping after the reconcile pass runs, so there is no observable bug today. However, if reconcileImportedReferences is extended to include additional entity types that reference endpoints, the stale mapping will cause hard-to-trace failures.
Consider calling $this->registerImportedEntityMapping('endpoint', $result['endpoints'][$endpointSlug]) after this line.
WilcoLouwerse
left a comment
WilcoLouwerse
left a comment
There was a problem hiding this comment.
The cross-entity slug reconciliation logic is sound and the ordering rationale is well-documented; two non-blocking concerns around dead registerImportedEntityMapping calls and a stale endpoint mapping after the second pass are worth addressing but do not block merge.
Quality Report — ConductionNL/openconnector @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 148/148 | |||
| npm | ✅ | ✅ 674/674 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-19 04:04 UTC
Download the full PDF report from the workflow artifacts.
Resolves 30-file conflict between i18n's Tier-4 refactor (OR-adoption + PHPCS docblock harmonisation + manifest v2 schema URL flip) and the 9 commits dev accumulated independently (#823 LogIndex wrapper, #842 .php-cs-fixer cleanup, #849 root-config sync + phpmd cleanup, #727 cross-entity slug refs, #752 PDOK adapter, #762 brand cobalt, #767 specter spec, #703 .gitignore harmonise, #679 openspec sync workflows). Resolution strategy: - 17 DU conflicts (Db classes + ExportService) — confirmed i18n's deletions (Tier-4 OR-adoption: data moved off bespoke Db/ classes to OR-backed objects). - l10n/en.json + l10n/nl.json — took HEAD's union (translation work was done on i18n). - composer.lock — took HEAD's (i18n had it regenerated for new deps). - src/manifest.json — took HEAD (v2 schema URL + 2-space indent + the typed-primitive page shapes; whitespace-only conflict otherwise). - 8 UU conflicts on PHP controllers/services + routes.php + registry.js — took HEAD (i18n). The systematic pattern: i18n calls the new OR API (->getObject()) while dev still references the now-deleted Db classes (->jsonSerialize()). Dev's references would break at runtime against i18n's structural state; HEAD is the only internally-consistent resolution. All conflict-resolved files: 0 markers remaining, PHP syntactically valid. Manifest still validates clean against v2 schema 2.7.0.
3 participants
Summary
registerImportedEntityMapping()to update in-memory slug/ID maps after each entity is imported, so entities created earlier in the same run are immediately available as references for later ones. AddedwithComponentSlug()to normalize missing slug fields from the component key. AddedreconcileImportedReferences()for a targeted second pass over endpoints and synchronizations, which commonly depend on entities imported just before them.synchronizationto the entity-type lists used during slug↔ID conversion, so synchronization references in rule configs are correctly serialized and deserialized.Test plan
🤖 Generated with Claude Code