From 7bc5102c0ce1204a6b8f10c3cb0d3570d9725ec5 Mon Sep 17 00:00:00 2001 From: bbrands02 Date: Fri, 24 Apr 2026 11:36:51 +0000 Subject: [PATCH 1/2] fix: resolve cross-entity slug references during configuration import 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 --- .../ConfigurationHandlers/RuleHandler.php | 4 +- lib/Service/ConfigurationService.php | 103 ++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/lib/Service/ConfigurationHandlers/RuleHandler.php b/lib/Service/ConfigurationHandlers/RuleHandler.php index 15abd9ac0..70b22a5af 100644 --- a/lib/Service/ConfigurationHandlers/RuleHandler.php +++ b/lib/Service/ConfigurationHandlers/RuleHandler.php @@ -66,7 +66,7 @@ public function export(Entity $entity, array $mappings, array &$mappingIds = []) */ private function convertIdsToSlugs(array $config, array $mappings, array &$mappingIds = []): array { - $entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema']; + $entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema', 'synchronization']; foreach ($config as $key => $value) { if (is_array($value)) { @@ -134,7 +134,7 @@ public function import(array $data, array $mappings): Entity */ private function convertSlugsToIds(array $config, array $mappings): array { - $entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema']; + $entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema', 'synchronization']; foreach ($config as $key => $value) { if (is_array($value)) { diff --git a/lib/Service/ConfigurationService.php b/lib/Service/ConfigurationService.php index 6dbdbb8da..d44a76927 100644 --- a/lib/Service/ConfigurationService.php +++ b/lib/Service/ConfigurationService.php @@ -22,6 +22,7 @@ use OCA\OpenConnector\Service\ConfigurationHandlers\JobHandler; use OCA\OpenConnector\Service\ConfigurationHandlers\SourceHandler; use OCA\OpenConnector\Service\ConfigurationHandlers\RuleHandler; +use OCP\AppFramework\Db\Entity; /** * Class ConfigurationService @@ -218,6 +219,93 @@ private function resetMappings(): void $this->mappings['schema']['slugToId'] = $this->schemaMapper->getSlugToIdMap(); } + /** + * Register a freshly imported entity in the in-memory slug/ID maps. + * + * This keeps references created earlier in the same import run available + * to later entities without requiring a second import pass. + * + * @param string $entityType Mapping bucket name + * @param Entity $entity Imported entity + * + * @return void + */ + private function registerImportedEntityMapping(string $entityType, Entity $entity): void + { + if (isset($this->mappings[$entityType]) === false) { + return; + } + + if (method_exists($entity, 'getId') === false || method_exists($entity, 'getSlug') === false) { + return; + } + + $id = $entity->getId(); + $slug = $entity->getSlug(); + + if ($id === null || $slug === null || $slug === '') { + return; + } + + $id = (string) $id; + + $this->mappings[$entityType]['idToSlug'][$id] = $slug; + $this->mappings[$entityType]['slugToId'][$slug] = $id; + } + + /** + * Ensure imported component payloads always carry their component-key slug. + * + * Some exports reference related entities by component key even when the nested + * payload is missing or inconsistent about its own slug field. Normalizing the + * slug up front keeps first-pass imports referentially stable. + * + * @param array $data Component payload + * @param string $slug Component key + * + * @return array + */ + private function withComponentSlug(array $data, string $slug): array + { + if (($data['slug'] ?? null) === null || $data['slug'] === '') { + $data['slug'] = $slug; + } + + return $data; + } + + /** + * Re-run import handlers after all mappings are known to resolve late references. + * + * This is intentionally limited to entity types that commonly depend on other + * imported slugs created earlier in the same run. + * + * @param array $components Original imported components + * @param array> $result Imported entity result map + * + * @return void + */ + private function reconcileImportedReferences(array $components, array &$result): void + { + if (isset($components['endpoints']) && is_array($components['endpoints'])) { + foreach ($components['endpoints'] as $endpointSlug => $endpointData) { + $endpointData = $this->withComponentSlug($endpointData, $endpointSlug); + $endpoint = $this->handlers['endpoint']->import($endpointData, $this->mappings); + $result['endpoints'][$endpointSlug] = $endpoint; + $this->registerImportedEntityMapping('endpoint', $endpoint); + } + } + + if (isset($components['synchronizations']) && is_array($components['synchronizations'])) { + foreach ($components['synchronizations'] as $syncSlug => $syncData) { + $syncData = $this->withComponentSlug($syncData, $syncSlug); + $synchronization = $this->handlers['synchronization']->import($syncData, $this->mappings); + $result['synchronizations'][$syncSlug] = $synchronization; + $this->registerImportedEntityMapping('synchronization', $synchronization); + } + } + } + /** * Get all entities associated with a specific configuration ID, indexed by their slug. * @@ -753,51 +841,66 @@ public function importConfiguration(array $oas): array // 1. Import sources first (no dependencies). if (isset($components['sources'])) { foreach ($components['sources'] as $sourceSlug => $sourceData) { + $sourceData = $this->withComponentSlug($sourceData, $sourceSlug); $source = $this->handlers['source']->import($sourceData, $this->mappings); $result['sources'][$sourceSlug] = $source; + $this->registerImportedEntityMapping('source', $source); } } // 2. Import mappings (depends on sources). if (isset($components['mappings'])) { foreach ($components['mappings'] as $mappingSlug => $mappingData) { + $mappingData = $this->withComponentSlug($mappingData, $mappingSlug); $mapping = $this->handlers['mapping']->import($mappingData, $this->mappings); $result['mappings'][$mappingSlug] = $mapping; + $this->registerImportedEntityMapping('mapping', $mapping); } } // 3. Import rules (depends on sources). if (isset($components['rules'])) { foreach ($components['rules'] as $ruleSlug => $ruleData) { + $ruleData = $this->withComponentSlug($ruleData, $ruleSlug); $rule = $this->handlers['rule']->import($ruleData, $this->mappings); $result['rules'][$ruleSlug] = $rule; + $this->registerImportedEntityMapping('rule', $rule); } } // 4. Import endpoints (depends on sources and mappings). if (isset($components['endpoints'])) { foreach ($components['endpoints'] as $endpointSlug => $endpointData) { + $endpointData = $this->withComponentSlug($endpointData, $endpointSlug); $endpoint = $this->handlers['endpoint']->import($endpointData, $this->mappings); $result['endpoints'][$endpointSlug] = $endpoint; + $this->registerImportedEntityMapping('endpoint', $endpoint); } } // 5. Import synchronizations (depends on sources, mappings, and endpoints). if (isset($components['synchronizations'])) { foreach ($components['synchronizations'] as $syncSlug => $syncData) { + $syncData = $this->withComponentSlug($syncData, $syncSlug); $synchronization = $this->handlers['synchronization']->import($syncData, $this->mappings); $result['synchronizations'][$syncSlug] = $synchronization; + $this->registerImportedEntityMapping('synchronization', $synchronization); } } // 6. Import jobs (depends on synchronizations, endpoints, and sources). if (isset($components['jobs'])) { foreach ($components['jobs'] as $jobSlug => $jobData) { + $jobData = $this->withComponentSlug($jobData, $jobSlug); $job = $this->handlers['job']->import($jobData, $this->mappings); $result['jobs'][$jobSlug] = $job; + $this->registerImportedEntityMapping('job', $job); } } + $this->resetMappings(); + $this->reconcileImportedReferences($components, $result); + return $result; } } From 5b818baa3386395875fe6ad80061d4d1b1cce005 Mon Sep 17 00:00:00 2001 From: bbrands02 Date: Fri, 24 Apr 2026 11:44:35 +0000 Subject: [PATCH 2/2] fix: address code review issues in ConfigurationService import reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- lib/Service/ConfigurationService.php | 33 ++++++++++++++-------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/Service/ConfigurationService.php b/lib/Service/ConfigurationService.php index d44a76927..252541957 100644 --- a/lib/Service/ConfigurationService.php +++ b/lib/Service/ConfigurationService.php @@ -236,7 +236,7 @@ private function registerImportedEntityMapping(string $entityType, Entity $entit return; } - if (method_exists($entity, 'getId') === false || method_exists($entity, 'getSlug') === false) { + if (method_exists($entity, 'getSlug') === false) { return; } @@ -275,10 +275,14 @@ private function withComponentSlug(array $data, string $slug): array } /** - * Re-run import handlers after all mappings are known to resolve late references. + * Re-run import handlers for entity types whose dependencies are imported later + * in the ordered pass, so those references are resolved with the complete mapping set. * - * This is intentionally limited to entity types that commonly depend on other - * imported slugs created earlier in the same run. + * Rules (step 3) and endpoints (step 4) are imported before synchronizations (step 5), + * so any references they hold to synchronizations cannot be resolved in the first pass. + * Synchronizations and jobs are imported last and have no such forward dependencies. + * + * Each handler is idempotent (upsert by slug), so re-running it is safe. * * @param array $components Original imported components * @param array> $result Imported entity result map @@ -287,21 +291,17 @@ private function withComponentSlug(array $data, string $slug): array */ private function reconcileImportedReferences(array $components, array &$result): void { - if (isset($components['endpoints']) && is_array($components['endpoints'])) { - foreach ($components['endpoints'] as $endpointSlug => $endpointData) { - $endpointData = $this->withComponentSlug($endpointData, $endpointSlug); - $endpoint = $this->handlers['endpoint']->import($endpointData, $this->mappings); - $result['endpoints'][$endpointSlug] = $endpoint; - $this->registerImportedEntityMapping('endpoint', $endpoint); + if (isset($components['rules']) && is_array($components['rules'])) { + foreach ($components['rules'] as $ruleSlug => $ruleData) { + $ruleData = $this->withComponentSlug($ruleData, $ruleSlug); + $result['rules'][$ruleSlug] = $this->handlers['rule']->import($ruleData, $this->mappings); } } - if (isset($components['synchronizations']) && is_array($components['synchronizations'])) { - foreach ($components['synchronizations'] as $syncSlug => $syncData) { - $syncData = $this->withComponentSlug($syncData, $syncSlug); - $synchronization = $this->handlers['synchronization']->import($syncData, $this->mappings); - $result['synchronizations'][$syncSlug] = $synchronization; - $this->registerImportedEntityMapping('synchronization', $synchronization); + 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); } } } @@ -898,7 +898,6 @@ public function importConfiguration(array $oas): array } } - $this->resetMappings(); $this->reconcileImportedReferences($components, $result); return $result;