diff --git a/scripts/api-sync.php b/scripts/api-sync.php index 63680ba..9e1c549 100644 --- a/scripts/api-sync.php +++ b/scripts/api-sync.php @@ -725,6 +725,14 @@ function reconcileEnums(array $map, array $newSpec, array $reachable, array $cla { $applicable = []; $needsHuman = []; + // Same idempotency concern as reconcileTypes()' $scheduledInsertions: more than one + // spec-map.json enums entry can legitimately point at the same SDK enum class (e.g. the same + // LimitIncreaseRequestSupportingDocumentType class is mapped from both + // CustomerLimitIncreaseIn.supporting_document_type and + // GetCustomerLimitIncreaseOut.supporting_document_type). If a new spec value is missing from + // that class, both entries independently report it -- dedupe per (class, value) so --apply + // inserts the case at most once per run. + $scheduledCases = []; foreach ($map['enums'] as $entry) { $s = $entry['spec']; @@ -752,6 +760,11 @@ function reconcileEnums(array $map, array $newSpec, array $reachable, array $cla if (isset($divergenceEnumIndex["{$className}|{$value}"])) { continue; // recorded known divergence, not pending drift } + $caseKey = "{$className}|{$value}"; + if (isset($scheduledCases[$caseKey])) { + continue; // same class+value already scheduled by an earlier enums entry this run + } + $scheduledCases[$caseKey] = true; $applicable[] = [ 'kind' => 'enum-member-added', 'enum' => $className, @@ -960,6 +973,16 @@ function reconcileTypes(array $map, array $newSpec, array $reachable, array $cla { $applicable = []; $needsHuman = []; + // Insertion is per (class, field), not per (schema, class, field): a multi-schema entry + // (e.g. spec: ["PayinOut", "CreatePayinOut"]) or two separate entries mapping the same SDK + // class (e.g. TrackingTransaction reused across the payin and payout tracking_transaction + // paths) can independently find the SAME missing field on the SAME class -- once per schema + // that lacks it. Without this dedupe, --apply would insert that promoted property/fromArray + // line/toArray line more than once into one file in a single run, corrupting the source + // (duplicate constructor parameters). This index makes insertion idempotent per run: the + // first schema to report a given (class, field) as missing wins, later reports of the exact + // same gap are dropped as already-scheduled, not re-emitted. + $scheduledInsertions = []; foreach ($map['types'] as $entry) { $schemas = typeSpecSchemas($entry); @@ -1045,6 +1068,11 @@ function reconcileTypes(array $map, array $newSpec, array $reachable, array $cla // is unambiguous; for a fan-out with multiple sdk sites we apply to ALL of them, since // any of them might legitimately accept/return the new field. foreach ($entry['sdk'] as $site) { + $insertionKey = "{$site['class']}|{$field}"; + if (isset($scheduledInsertions[$insertionKey])) { + continue; // same class+field already scheduled by an earlier schema/entry this run + } + $scheduledInsertions[$insertionKey] = true; $applicable[] = [ 'kind' => 'field-added', 'schema' => $schemaName, diff --git a/tests/ApiSync/ApiSyncTest.php b/tests/ApiSync/ApiSyncTest.php index dfdf9ae..98625b3 100644 --- a/tests/ApiSync/ApiSyncTest.php +++ b/tests/ApiSync/ApiSyncTest.php @@ -396,6 +396,142 @@ public function applying_a_field_twice_only_inserts_it_once(): void $this->assertSame(3, substr_count($source, 'description')); } + #[Test] + public function a_multi_schema_entry_missing_the_same_field_on_both_schemas_is_only_scheduled_once(): void + { + // Reproduces the exact double-apply bug: a single spec-map.json entry with TWO schemas + // (e.g. spec: ["PayinOut", "CreatePayinOut"]) sharing one SDK class. If both schemas are + // independently missing the same field, reconcileTypes() used to emit one 'field-added' + // per schema -- two entries for the same (class, field) -- and --apply inserted the + // promoted property/fromArray line/toArray line twice into one file in a single run. + $map = $this->widgetMap(); + $map['types'][0]['spec'] = ['WidgetOut', 'WidgetOutAlias']; + + $classIndex = scanAllClasses($this->fixtureRoot); + $newSpec = $this->baseSpec([ + 'components' => ['schemas' => [ + 'WidgetOut' => ['properties' => ['description' => ['type' => ['string', 'null']]]], + 'WidgetOutAlias' => [ + 'type' => 'object', + 'properties' => [ + 'id' => ['type' => 'string'], + 'name' => ['type' => 'string'], + 'color' => ['type' => 'string', 'enum' => ['red', 'blue']], + 'description' => ['type' => ['string', 'null']], + ], + ], + ]], + ]); + // WidgetOutAlias is only reachable if something references it -- webhooks is a + // convenient root for a schema with no operation of its own. + $newSpec['webhooks'] = ['widget.alias' => ['post' => ['requestBody' => ['content' => ['application/json' => ['schema' => ['$ref' => '#/components/schemas/WidgetOutAlias']]]]]]]; + $reachable = computeReachable($newSpec); + + [$applicable, $needsHuman] = reconcileTypes($map, $newSpec, $reachable, $classIndex, []); + + $this->assertEmpty($needsHuman); + $this->assertCount(1, $applicable, 'the same (class, field) gap reported by two schemas in one entry must be scheduled only once'); + $this->assertSame('description', $applicable[0]['field']); + $this->assertSame('WidgetResponse', $applicable[0]['class']); + + foreach ($applicable as $change) { + applyFieldInsertion($this->fixtureRoot, $change['file'], $change['class'], $change['field'], $change['propSchema']); + } + + $source = file_get_contents("{$this->fixtureRoot}/{$applicable[0]['file']}"); + $this->assertSame(3, substr_count($source, 'description'), 'exactly one constructor param + one fromArray line + one toArray line, never two of each'); + + // a second --apply-equivalent pass over the now-patched source finds nothing left to do. + $classIndex = scanAllClasses($this->fixtureRoot); + [$applicableAgain, $needsHumanAgain] = reconcileTypes($map, $newSpec, $reachable, $classIndex, []); + $this->assertEmpty($applicableAgain); + $this->assertEmpty($needsHumanAgain); + } + + #[Test] + public function two_separate_entries_mapping_the_same_class_missing_the_same_field_are_only_scheduled_once(): void + { + // Same bug, different trigger shape: TWO separate spec-map.json `types` entries (not one + // multi-schema entry) independently point at the same SDK class -- this is exactly how + // TrackingTransaction is reused across the payin and payout tracking_transaction paths. + $map = $this->widgetMap(); + $map['types'][] = [ + 'spec' => 'WidgetOutAlias', + 'sdk' => [['file' => 'src/Resources/Widgets/Widgets.php', 'class' => 'WidgetResponse']], + ]; + + $classIndex = scanAllClasses($this->fixtureRoot); + $newSpec = $this->baseSpec([ + 'components' => ['schemas' => [ + 'WidgetOut' => ['properties' => ['description' => ['type' => ['string', 'null']]]], + 'WidgetOutAlias' => [ + 'type' => 'object', + 'properties' => [ + 'id' => ['type' => 'string'], + 'name' => ['type' => 'string'], + 'color' => ['type' => 'string', 'enum' => ['red', 'blue']], + 'description' => ['type' => ['string', 'null']], + ], + ], + ]], + ]); + $newSpec['webhooks'] = ['widget.alias' => ['post' => ['requestBody' => ['content' => ['application/json' => ['schema' => ['$ref' => '#/components/schemas/WidgetOutAlias']]]]]]]; + $reachable = computeReachable($newSpec); + + [$applicable, $needsHuman] = reconcileTypes($map, $newSpec, $reachable, $classIndex, []); + + $this->assertEmpty($needsHuman); + $this->assertCount(1, $applicable, 'two entries mapping the same class must not double-schedule the same missing field'); + + foreach ($applicable as $change) { + applyFieldInsertion($this->fixtureRoot, $change['file'], $change['class'], $change['field'], $change['propSchema']); + } + + $source = file_get_contents("{$this->fixtureRoot}/{$applicable[0]['file']}"); + $this->assertSame(3, substr_count($source, 'description')); + } + + #[Test] + public function two_enum_entries_mapping_the_same_class_missing_the_same_value_are_only_scheduled_once(): void + { + // Mirrors the field-insertion bug on the enum side: two spec-map.json `enums` entries + // (e.g. the same LimitIncreaseRequestSupportingDocumentType class mapped from both + // CustomerLimitIncreaseIn and GetCustomerLimitIncreaseOut) independently report the same + // missing case, which used to insert it twice into the enum file in one --apply run. + $map = $this->widgetMap(); + $map['enums'][] = [ + 'spec' => ['schema' => 'WidgetOutAlias', 'property' => 'color'], + 'sdk' => ['file' => 'src/Types/WidgetColor.php', 'class' => 'WidgetColor'], + ]; + + $classIndex = scanAllClasses($this->fixtureRoot); + $newSpec = $this->baseSpec([ + 'components' => ['schemas' => [ + 'WidgetOut' => ['properties' => ['color' => ['enum' => ['red', 'blue', 'green']]]], + 'WidgetOutAlias' => [ + 'type' => 'object', + 'properties' => [ + 'color' => ['type' => 'string', 'enum' => ['red', 'blue', 'green']], + ], + ], + ]], + ]); + $newSpec['webhooks'] = ['widget.alias' => ['post' => ['requestBody' => ['content' => ['application/json' => ['schema' => ['$ref' => '#/components/schemas/WidgetOutAlias']]]]]]]; + $reachable = computeReachable($newSpec); + + [$applicable, $needsHuman] = reconcileEnums($map, $newSpec, $reachable, $classIndex, []); + + $this->assertEmpty($needsHuman); + $this->assertCount(1, $applicable, 'two enum entries reporting the same missing case on the same class must not double-schedule it'); + $this->assertSame('green', $applicable[0]['value']); + + $specValuesByEnum = ['WidgetColor' => ['red', 'blue', 'green']]; + applyEnumCaseInsertion($this->fixtureRoot, $applicable[0]['file'], 'WidgetColor', 'green', $newSpec, $specValuesByEnum); + + $source = file_get_contents("{$this->fixtureRoot}/src/Types/WidgetColor.php"); + $this->assertSame(1, substr_count($source, "case GREEN = 'green';")); + } + // ---- NEEDS_HUMAN: removals ---- #[Test]