Skip to content

Commit 0c0e113

Browse files
bbrands02clauderubenvdlinde
authored
fix: resolve cross-entity slug references during configuration import (#727)
* 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 <noreply@anthropic.com> * fix: address code review issues in ConfigurationService import reconciliation - 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> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Ruben van der Linde <ruben@conduction.nl>
1 parent f1f4cce commit 0c0e113

2 files changed

Lines changed: 104 additions & 2 deletions

File tree

‎lib/Service/ConfigurationHandlers/RuleHandler.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function export(Entity $entity, array $mappings, array &$mappingIds=[]):
6666
*/
6767
private function convertIdsToSlugs(array $config, array $mappings, array &$mappingIds=[]): array
6868
{
69-
$entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema'];
69+
$entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema', 'synchronization'];
7070

7171
foreach ($config as $key => $value) {
7272
if (is_array($value)) {
@@ -138,7 +138,7 @@ public function import(array $data, array $mappings): Entity
138138
*/
139139
private function convertSlugsToIds(array $config, array $mappings): array
140140
{
141-
$entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema'];
141+
$entityTypes = ['source', 'job', 'endpoint', 'mapping', 'register', 'schema', 'synchronization'];
142142

143143
foreach ($config as $key => $value) {
144144
if (is_array($value)) {

‎lib/Service/ConfigurationService.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCA\OpenConnector\Service\ConfigurationHandlers\JobHandler;
2323
use OCA\OpenConnector\Service\ConfigurationHandlers\SourceHandler;
2424
use OCA\OpenConnector\Service\ConfigurationHandlers\RuleHandler;
25+
use OCP\AppFramework\Db\Entity;
2526

2627
/**
2728
* Class ConfigurationService
@@ -219,6 +220,93 @@ private function resetMappings(): void
219220
$this->mappings['schema']['slugToId'] = $this->schemaMapper->getSlugToIdMap();
220221
}//end resetMappings()
221222

223+
/**
224+
* Register a freshly imported entity in the in-memory slug/ID maps.
225+
*
226+
* This keeps references created earlier in the same import run available
227+
* to later entities without requiring a second import pass.
228+
*
229+
* @param string $entityType Mapping bucket name
230+
* @param Entity $entity Imported entity
231+
*
232+
* @return void
233+
*/
234+
private function registerImportedEntityMapping(string $entityType, Entity $entity): void
235+
{
236+
if (isset($this->mappings[$entityType]) === false) {
237+
return;
238+
}
239+
240+
if (method_exists($entity, 'getSlug') === false) {
241+
return;
242+
}
243+
244+
$id = $entity->getId();
245+
$slug = $entity->getSlug();
246+
247+
if ($id === null || $slug === null || $slug === '') {
248+
return;
249+
}
250+
251+
$id = (string) $id;
252+
253+
$this->mappings[$entityType]['idToSlug'][$id] = $slug;
254+
$this->mappings[$entityType]['slugToId'][$slug] = $id;
255+
}
256+
257+
/**
258+
* Ensure imported component payloads always carry their component-key slug.
259+
*
260+
* Some exports reference related entities by component key even when the nested
261+
* payload is missing or inconsistent about its own slug field. Normalizing the
262+
* slug up front keeps first-pass imports referentially stable.
263+
*
264+
* @param array $data Component payload
265+
* @param string $slug Component key
266+
*
267+
* @return array
268+
*/
269+
private function withComponentSlug(array $data, string $slug): array
270+
{
271+
if (($data['slug'] ?? null) === null || $data['slug'] === '') {
272+
$data['slug'] = $slug;
273+
}
274+
275+
return $data;
276+
}
277+
278+
/**
279+
* Re-run import handlers for entity types whose dependencies are imported later
280+
* in the ordered pass, so those references are resolved with the complete mapping set.
281+
*
282+
* Rules (step 3) and endpoints (step 4) are imported before synchronizations (step 5),
283+
* so any references they hold to synchronizations cannot be resolved in the first pass.
284+
* Synchronizations and jobs are imported last and have no such forward dependencies.
285+
*
286+
* Each handler is idempotent (upsert by slug), so re-running it is safe.
287+
*
288+
* @param array<string,mixed> $components Original imported components
289+
* @param array<string,array<string,Entity>> $result Imported entity result map
290+
*
291+
* @return void
292+
*/
293+
private function reconcileImportedReferences(array $components, array &$result): void
294+
{
295+
if (isset($components['rules']) && is_array($components['rules'])) {
296+
foreach ($components['rules'] as $ruleSlug => $ruleData) {
297+
$ruleData = $this->withComponentSlug($ruleData, $ruleSlug);
298+
$result['rules'][$ruleSlug] = $this->handlers['rule']->import($ruleData, $this->mappings);
299+
}
300+
}
301+
302+
if (isset($components['endpoints']) && is_array($components['endpoints'])) {
303+
foreach ($components['endpoints'] as $endpointSlug => $endpointData) {
304+
$endpointData = $this->withComponentSlug($endpointData, $endpointSlug);
305+
$result['endpoints'][$endpointSlug] = $this->handlers['endpoint']->import($endpointData, $this->mappings);
306+
}
307+
}
308+
}
309+
222310
/**
223311
* Get all entities associated with a specific configuration ID, indexed by their slug.
224312
*
@@ -785,51 +873,65 @@ public function importConfiguration(array $oas): array
785873
// 1. Import sources first (no dependencies).
786874
if (isset($components['sources'])) {
787875
foreach ($components['sources'] as $sourceSlug => $sourceData) {
876+
$sourceData = $this->withComponentSlug($sourceData, $sourceSlug);
788877
$source = $this->handlers['source']->import($sourceData, $this->mappings);
789878
$result['sources'][$sourceSlug] = $source;
879+
$this->registerImportedEntityMapping('source', $source);
790880
}
791881
}
792882

793883
// 2. Import mappings (depends on sources).
794884
if (isset($components['mappings'])) {
795885
foreach ($components['mappings'] as $mappingSlug => $mappingData) {
886+
$mappingData = $this->withComponentSlug($mappingData, $mappingSlug);
796887
$mapping = $this->handlers['mapping']->import($mappingData, $this->mappings);
797888
$result['mappings'][$mappingSlug] = $mapping;
889+
$this->registerImportedEntityMapping('mapping', $mapping);
798890
}
799891
}
800892

801893
// 3. Import rules (depends on sources).
802894
if (isset($components['rules'])) {
803895
foreach ($components['rules'] as $ruleSlug => $ruleData) {
896+
$ruleData = $this->withComponentSlug($ruleData, $ruleSlug);
804897
$rule = $this->handlers['rule']->import($ruleData, $this->mappings);
805898
$result['rules'][$ruleSlug] = $rule;
899+
$this->registerImportedEntityMapping('rule', $rule);
806900
}
807901
}
808902

809903
// 4. Import endpoints (depends on sources and mappings).
810904
if (isset($components['endpoints'])) {
811905
foreach ($components['endpoints'] as $endpointSlug => $endpointData) {
906+
$endpointData = $this->withComponentSlug($endpointData, $endpointSlug);
812907
$endpoint = $this->handlers['endpoint']->import($endpointData, $this->mappings);
813908
$result['endpoints'][$endpointSlug] = $endpoint;
909+
$this->registerImportedEntityMapping('endpoint', $endpoint);
814910
}
815911
}
816912

817913
// 5. Import synchronizations (depends on sources, mappings, and endpoints).
818914
if (isset($components['synchronizations'])) {
819915
foreach ($components['synchronizations'] as $syncSlug => $syncData) {
916+
$syncData = $this->withComponentSlug($syncData, $syncSlug);
820917
$synchronization = $this->handlers['synchronization']->import($syncData, $this->mappings);
821918
$result['synchronizations'][$syncSlug] = $synchronization;
919+
$this->registerImportedEntityMapping('synchronization', $synchronization);
822920
}
823921
}
824922

825923
// 6. Import jobs (depends on synchronizations, endpoints, and sources).
826924
if (isset($components['jobs'])) {
827925
foreach ($components['jobs'] as $jobSlug => $jobData) {
926+
$jobData = $this->withComponentSlug($jobData, $jobSlug);
828927
$job = $this->handlers['job']->import($jobData, $this->mappings);
829928
$result['jobs'][$jobSlug] = $job;
929+
$this->registerImportedEntityMapping('job', $job);
830930
}
831931
}
832932

933+
$this->reconcileImportedReferences($components, $result);
934+
833935
return $result;
834936
}//end importConfiguration()
835937
}//end class

0 commit comments

Comments
 (0)