diff --git a/docs/guide/extension-points.md b/docs/guide/extension-points.md index 9e0d6f908..6c7e7aa78 100644 --- a/docs/guide/extension-points.md +++ b/docs/guide/extension-points.md @@ -72,6 +72,11 @@ An augmenter can also `add()` attributes, but it runs after resolution, so a `$r what it adds stays unresolved. Use an augmenter to enrich what is there, and this to put something there. +Once added, a contribution looks like any scanned attribute. If a later step needs to tell +yours apart — a [merger](#mergers) deciding precedence, an augmenter that should leave them +alone — mark them as you add them with `setMeta()` under a key you own, and read it back with +`getMeta()` there. + ## Resolvers Seeding from reflectors means the specification can name a class that was never a source: a @@ -116,6 +121,71 @@ registration order. The enum is `OpenApi\Augmenter\Group`. The [Augmenters reference](/reference/augmenters) lists the built-in pipeline and what each phase is for. +## Mergers + +Two attributes can claim one key. Two operations on the same path and method, two schemas named +`Pet` — a scan finds one, a `withSpecification()` hook contributes the other, an inheritance +clone makes a third. Something has to decide which of them the document holds, and until it +does the compiler decides by accident: it writes each into a PHP array and keeps whichever it +wrote last. + +`Augmenter\Merge` decides instead, through a chain of mergers. A merger says what makes two +attributes the same one and what the survivor is — below, an operation the scan already +described keeps the key against one a hook contributed and marked as its own: + +```php +use OpenApi\Contracts\AttributeInterface; +use OpenApi\Contracts\MergerInterface; +use OpenApi\Spec as OA; + +final class MyOperationMerger implements MergerInterface +{ + public function supports(string $class): bool + { + return is_a($class, OA\Operation::class, true); + } + + public function identity(AttributeInterface $attribute): ?string + { + return $attribute->path !== null && $attribute->method !== null + ? $attribute->method . ' ' . $attribute->path + : null; + } + + public function merge(AttributeInterface $earlier, AttributeInterface $later): AttributeInterface + { + return $later->getMeta(self::class, false) ? $earlier : $later; + } +} +``` + +`Builder::withMergers()` registers it. They are tried in order and the first to claim a type +handles it, so `Merge\LastWins` — which claims everything, keeps the later entry and warns with +both locations — ships last: + +```php +use OpenApi\Merge; +use OpenApi\Utils\TypedList; + +$builder->withMergers(fn (TypedList $mergers) => $mergers->insert( + new MyOperationMerger(), + Merge\LastWins::class, +)); +``` + +`identity()` returning `null` means the attribute never merges and passes through: that is how +servers and security requirements stay as they are, being positional rather than keyed. + +`$earlier` and `$later` are in producer order, which is the only thing the pipeline guarantees — +`return $later` is last-wins. Precedence beyond that order is a policy the core does not hold. A +package that needs to recognise its own attributes marks them as it creates them — +`$operation->setMeta(MyOperationMerger::class, true)` — and reads that back in `merge()`. `meta` +is keyed by whoever writes to it; nothing in swagger-php writes or reads it. + +The pass runs over the `Specification`'s own collections, where the halves come from different +places. A duplicate key *inside* one attribute — two `200` responses in one operation — is two +entries one author wrote in one place, and the compiler is left to keep the last of them. + ## Compilers `Builder::setCompiler()` replaces the compiler that turns the `Specification` into a @@ -147,7 +217,9 @@ alternative. **Property types are not widened for downstream convenience.** The strong typing is what makes the DTOs worth having. Metadata that only means something to one integration belongs -in an `Attachable`, not in a widened `$ref: string|object`. +in an `Attachable` when it is declared in source next to the attribute it describes, and in +`setMeta()` when code attaches it along the way — not in a widened `$ref: string|object`. +Neither reaches the generated document. **There is no framework-specific code, and no plans for any.** Translators, contributions, augmenters and attachables are the contract; anything a framework needs can be built from them, outside diff --git a/docs/reference/augmenters.md b/docs/reference/augmenters.md index f1a031a13..9fa973b65 100644 --- a/docs/reference/augmenters.md +++ b/docs/reference/augmenters.md @@ -114,6 +114,27 @@ Resolves FQCN-based $ref values to JSON Reference paths. Builds a map of class names to their component paths and rewrites any $ref that looks like a FQCN into the proper #/components/... path. +### [Merge](https://github.com/zircote/swagger-php/tree/master/src/Augmenter/Merge.php) + +Reduces every root collection to one entry per key, through the registered mergers. + +Two attributes can claim one key — two operations on the same path and method, two schemas +named `Pet`. The merger that claims the type says what identifies it and what the survivor is, +and one entry per key reaches the compiler. + +Only the root collections, because that is where the halves come from different places — a +scan, a `withSpecification()` hook, the resolver, an inheritance clone — and something has to +decide between them. A collision *inside* one attribute is two entries the same author wrote +in one place; the compiler reports it and keeps its own last-write-wins. + +Registered twice, and both are this class. The first run is the first pipe of the **reduce** +phase, which is the earliest point every identity exists — `Augmenter\PathItems` resolves an +operation's path and `Augmenter\Names` infers component keys, both in **resolve** — and it is +before `Cleanup` and everything downstream that should see the survivor rather than both +halves. The second is the last pipe of all, so what a late augmenter adds is reduced too; +it is a grouping over lists and costs nothing when there is nothing to do. An augmenter +registered after it runs after it; `insert()` is how to land ahead. + ### [PathFilter](https://github.com/zircote/swagger-php/tree/master/src/Augmenter/PathFilter.php) Filters operations by tag and/or path patterns. diff --git a/docs/reference/builder.md b/docs/reference/builder.md index 43e36465e..6c1786742 100644 --- a/docs/reference/builder.md +++ b/docs/reference/builder.md @@ -193,6 +193,35 @@ $builder->withResolver(function (Resolver $resolver) { Resolvers implement `OpenApi\Contracts\ResolverInterface` and receive the FQCN and the `Assembler` in use. The first one to return `true` claims the FQCN. See the [Resolver section](/reference/architecture#resolver) in the architecture docs for details, including how to reorder or clear the chain. +### Merger configuration (spec/hybrid mode) {#mergers} + +Mergers decide what makes two attributes the same one and which of them the document holds. +`Augmenter\Merge` applies them to the `Specification`'s collections, first in the **reduce** +phase and again as the last pipe, so a key claimed twice reaches the compiler once. + +`Merge\LastWins` is registered by default: it claims every type, keys each collection the way +the document does — component key, path and method, webhook and method, tag name — and on a +collision keeps the later entry and warns with both locations. Positional lists, `servers` and +`security`, have no key and are left alone. + +Use `withMergers()` to add your own. The hook runs when called; repeated calls configure the +same list: + +```php +use OpenApi\Merge; +use OpenApi\Utils\TypedList; + +$builder->withMergers(fn (TypedList $mergers) => $mergers->insert( + new MyOperationMerger(), + Merge\LastWins::class, +)); +``` + +Mergers implement `OpenApi\Contracts\MergerInterface` and are tried in registration order; the +first to `supports()` a type claims it, so a merger for one type goes ahead of the catch-all. +`insert()` places it there. See the [Mergers section](/guide/extension-points#mergers) in the +extension points guide for what a merger looks like and how it recognises its own attributes. + ### Attribute factory configuration (spec mode) {#attribute-factory} Use `withAttributeFactory()` to add custom attribute translators. The hook runs when called; diff --git a/docs/reference/extension-points.md b/docs/reference/extension-points.md index dd5edc91d..f58245fa0 100644 --- a/docs/reference/extension-points.md +++ b/docs/reference/extension-points.md @@ -37,3 +37,18 @@ is enough as long as everything it references (directly or transitively) carries A FQCN is considered resolved if the specification knows it once collected. Classes without any spec attributes are left to the next resolver in the chain. + +## Default Mergers + +### [LastWins](https://github.com/zircote/swagger-php/tree/master/src/Merge/LastWins.php) + +The catch-all merger: one entry per key survives, the later one, and the author is told. + +It reduces rather than folds: two entries in, one out, so the compiler never sees a collision +and its own accidental rules stop deciding anything. Combining fields from both halves is a +type-specific merger's job, registered ahead of this one. + +Identity by collection: the component key for anything in a `components` bucket, path and +method for an operation, webhook and method for a webhook operation, path for a path-bound +path item, name for a tag. Servers, security requirements and external documentation are +positional — their entries have no identity, duplicates are legal, and they pass through. diff --git a/src/Augmenter/Merge.php b/src/Augmenter/Merge.php new file mode 100644 index 000000000..5bcf64671 --- /dev/null +++ b/src/Augmenter/Merge.php @@ -0,0 +1,126 @@ + + */ +class Merge implements PipeInterface, LoggerAwareInterface +{ + use LoggerAwareTrait; + + /** + * @param TypedList $mergers + */ + public function __construct( + protected TypedList $mergers, + protected Group $group = Group::Reduce, + ) { + } + + public function __invoke(mixed $payload): mixed + { + if ($this->logger instanceof LoggerInterface) { + foreach ($this->mergers as $merger) { + if ($merger instanceof LoggerAwareInterface) { + $merger->setLogger($this->logger); + } + } + } + + // every root collection, discovered rather than listed: a new bucket on the + // `Specification` is reduced without this pass being told about it + foreach (get_object_vars($payload) as $property => $value) { + if (is_array($value)) { + $payload->{$property} = $this->reduce($value); + } + } + + return null; + } + + public function group(): string|\BackedEnum + { + return $this->group; + } + + /** + * @param array $attributes + * @return list + */ + protected function reduce(array $attributes): array + { + if (count($attributes) < 2) { + return array_values($attributes); + } + + $reduced = []; + $slots = []; + + foreach ($attributes as $attribute) { + $merger = $attribute instanceof AttributeInterface ? $this->mergerFor($attribute) : null; + $identity = $merger?->identity($attribute); + + if (!$merger instanceof MergerInterface || $identity === null) { + $reduced[] = $attribute; + continue; + } + + // two mergers never fold into each other: whoever claimed the type decides + $key = $merger::class . "\0" . $identity; + + if (!array_key_exists($key, $slots)) { + $slots[$key] = count($reduced); + $reduced[] = $attribute; + continue; + } + + $reduced[$slots[$key]] = $merger->merge($reduced[$slots[$key]], $attribute); + } + + return $reduced; + } + + protected function mergerFor(AttributeInterface $attribute): ?MergerInterface + { + foreach ($this->mergers as $merger) { + if ($merger->supports($attribute::class)) { + return $merger; + } + } + + return null; + } +} diff --git a/src/Builder.php b/src/Builder.php index 174d53f7f..f0647fe22 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -9,6 +9,7 @@ use OpenApi\Builder\Mode; use OpenApi\Builder\Result; use OpenApi\Contracts\CompilerInterface; +use OpenApi\Contracts\MergerInterface; use OpenApi\Loggers\CollectingLogger; use OpenApi\Utils\AttributeFactory; use OpenApi\Utils\ClassReflector; @@ -58,6 +59,11 @@ class Builder */ protected ?Utils\Pipeline $augmenters = null; + /** + * @var Utils\TypedList|null + */ + protected ?Utils\TypedList $mergers = null; + /** * @var callable|null */ @@ -168,6 +174,35 @@ public function withAugmenters(callable $hook): static return $this; } + /** + * @return Utils\TypedList + */ + public function getMergers(): Utils\TypedList + { + $this->mergers ??= new Utils\TypedList($this->getDefaultMergers()); + + return $this->mergers; + } + + /** + * Configure the merger chain via callable. + * + * Mergers decide what makes two attributes the same one and what the survivor is. They are + * tried in registration order and the first to claim a type handles it, so a merger for one + * type goes ahead of `Merge\LastWins`, which claims everything and ships last. + * + * Runs when called, against the list the builder holds; repeated calls configure the same + * list. + * + * @param callable(Utils\TypedList): (Utils\TypedList|void) $hook + */ + public function withMergers(callable $hook): static + { + $hook($this->getMergers()); + + return $this; + } + public function getAttributeFactory(): AttributeFactory { $this->attributeFactory ??= new AttributeFactory(); @@ -266,6 +301,8 @@ protected function doBuildClassic(): Result protected function doBuildSpec(bool $hybrid = false): Result { + $collecting = new CollectingLogger($this->getLogger()); + $attributeFactory = $this->getAttributeFactory(); $assembler = new Assembler(attributeFactory: $attributeFactory); @@ -316,6 +353,7 @@ protected function doBuildSpec(bool $hybrid = false): Result $this->getAugmenters()->get(Augmenter\Inheritance::class) ?->setAttributeFactory($attributeFactory); + $this->getAugmenters()->setLogger($collecting); $this->getAugmenters()->process($specification); $version = $this->version ?? $specification->openapi->version ?? '3.1.0'; @@ -325,7 +363,12 @@ protected function doBuildSpec(bool $hybrid = false): Result $diagnostics = $compiler->validate($specification); $output = $compiler->compile($specification); - return Result::fromSpec($sourceScanner->getFiles(), $specification, $output, $diagnostics); + return Result::fromSpec( + $sourceScanner->getFiles(), + $specification, + $output, + [...$collecting->entries(), ...$diagnostics], + ); } /** @@ -388,6 +431,9 @@ protected function resolveCompiler(string $version): CompilerInterface */ protected function getDefaultAugmenters(): array { + // both runs share one registry, so `withMergers()` reaches them both + $mergers = $this->getMergers(); + return [ new Augmenter\Inheritance(), new Augmenter\Names(), @@ -396,6 +442,7 @@ protected function getDefaultAugmenters(): array new Augmenter\PathItems(), new Augmenter\Types(), new Augmenter\Refs(), + new Augmenter\Merge($mergers), new Augmenter\PathFilter(), new Augmenter\Cleanup(), new Augmenter\MediaTypes(), @@ -403,6 +450,19 @@ protected function getDefaultAugmenters(): array new Augmenter\OperationIds(), new Augmenter\Tags(), new Augmenter\EnumDescriptions(), + // the same pass again, as the last pipe of all: what a late augmenter added is + // reduced too, so nothing downstream has to be trusted to keep keys unique + new Augmenter\Merge($mergers, Augmenter\Group::Augment), + ]; + } + + /** + * @return list + */ + protected function getDefaultMergers(): array + { + return [ + new Merge\LastWins(), ]; } } diff --git a/src/Compiler/OpenApi31Compiler.php b/src/Compiler/OpenApi31Compiler.php index 9ec128564..ad8087c84 100644 --- a/src/Compiler/OpenApi31Compiler.php +++ b/src/Compiler/OpenApi31Compiler.php @@ -222,6 +222,12 @@ protected function compileExternalDocs(OA\ExternalDocumentation $docs): array } /** + * Operations are keyed by path and method, path items by path, and one path holds both. + * + * The union puts a path item's own fields beside the method entries already written for that + * path. Which of two path items for one path survives is `Augmenter\Merge`'s to decide, before + * the compiler sees either. + * * @param list $operations * @param list $pathItems * @return array> @@ -795,6 +801,9 @@ protected function compileExample(OA\Example $example): array * be derived from a method or a non-constructor parameter, so an attribute declared there * has to carry its own name — the same requirement classic enforces. `Augmenter\Names` * fills the key in for anything declared on a class, so what reaches here is unnameable. + * + * A key claimed twice is `Augmenter\Merge`'s to reduce and report, and it runs first, so a + * repeat seen here means a `Specification` that was compiled without the pipeline. */ protected function validateNames(Specification $specification): void { diff --git a/src/Contracts/AttributeInterface.php b/src/Contracts/AttributeInterface.php index 4373a24da..a21aa98f3 100644 --- a/src/Contracts/AttributeInterface.php +++ b/src/Contracts/AttributeInterface.php @@ -79,5 +79,15 @@ public function getClassName(): ?string; public function getShortClassName(): ?string; + /** + * Extra data for whoever needs to carry it on an attribute, keyed by whoever writes it. + * + * Nothing in swagger-php writes or reads it. A clone copies the entries; an object stored as a + * value is shared between the clone and the original. + */ + public function getMeta(string $key, mixed $default = null): mixed; + + public function setMeta(string $key, mixed $value): static; + public function getSourceLocation(): SourceLocation; } diff --git a/src/Contracts/MergerInterface.php b/src/Contracts/MergerInterface.php new file mode 100644 index 000000000..4818d5ce1 --- /dev/null +++ b/src/Contracts/MergerInterface.php @@ -0,0 +1,53 @@ +method === null) { + return null; + } + + return match (true) { + $attribute->path !== null => 'operation:' . $attribute->method . ' ' . $attribute->path, + $attribute->webhook !== null => 'webhook:' . $attribute->method . ' ' . $attribute->webhook, + default => null, + }; + } + + if ($attribute instanceof OA\Tag) { + return $attribute->name !== null ? 'tag:' . $attribute->name : null; + } + + if (ComponentName::isComponentType($attribute)) { + $component = ComponentName::of($attribute); + if ($component !== null) { + return 'component:' . $component; + } + + // a path item without a component key is path-bound, and keyed by its path + return $attribute instanceof OA\PathItem && $attribute->path !== null + ? 'path:' . $attribute->path + : null; + } + + return null; + } + + public function merge(AttributeInterface $earlier, AttributeInterface $later): AttributeInterface + { + $this->logger?->warning($this->collision($earlier, $later)); + + return $later; + } + + /** + * Both halves are named, because either could be the one the author did not mean to write. + * Two contributed halves both report `unknown`, which is stated once. + */ + protected function collision(AttributeInterface $earlier, AttributeInterface $later): string + { + $earlierAt = (string) $earlier->getSourceLocation(); + $laterAt = (string) $later->getSourceLocation(); + + return sprintf( + '%s "%s" is declared more than once, keeping the last in %s', + (new \ReflectionClass($later))->getShortName(), + $this->label($later), + $earlierAt === $laterAt ? $laterAt : $laterAt . ' and ' . $earlierAt, + ); + } + + /** + * What the key reads as in a message. The identity string is a grouping key and carries a + * space prefix nobody needs to see. + */ + protected function label(AttributeInterface $attribute): string + { + if ($attribute instanceof OA\Operation) { + return trim(($attribute->method ?? '') . ' ' . ($attribute->path ?? $attribute->webhook ?? '')); + } + + if ($attribute instanceof OA\Tag) { + return (string) $attribute->name; + } + + return (string) (ComponentName::of($attribute) ?? ($attribute instanceof OA\PathItem ? $attribute->path : null)); + } +} diff --git a/src/Spec/AbstractAttribute.php b/src/Spec/AbstractAttribute.php index 63a0e36ee..143f16192 100644 --- a/src/Spec/AbstractAttribute.php +++ b/src/Spec/AbstractAttribute.php @@ -15,6 +15,11 @@ abstract class AbstractAttribute implements AttributeInterface protected ?SourceLocation $sourceLocation = null; + /** + * @var array + */ + protected array $meta = []; + /** * @param array|null $x * @param list|null $attachables Reusable custom attachable attributes @@ -90,6 +95,18 @@ public function setReflector(?\Reflector $reflector): static return $this; } + public function getMeta(string $key, mixed $default = null): mixed + { + return array_key_exists($key, $this->meta) ? $this->meta[$key] : $default; + } + + public function setMeta(string $key, mixed $value): static + { + $this->meta[$key] = $value; + + return $this; + } + public function getSourceLocation(): SourceLocation { if (!$this->sourceLocation instanceof SourceLocation) { diff --git a/src/Utils/Pipeline.php b/src/Utils/Pipeline.php index cecc28097..eadbf9993 100644 --- a/src/Utils/Pipeline.php +++ b/src/Utils/Pipeline.php @@ -8,6 +8,7 @@ use OpenApi\OpenApiException; use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -16,8 +17,15 @@ * * @extends TypedList */ -class Pipeline extends TypedList +class Pipeline extends TypedList implements LoggerAwareInterface { + /** + * The pipeline hands this to every pipe that asks for one, so setting it here reaches all + * of them. The builder sets it once the build's collecting logger exists, which is after + * the pipeline was built. + */ + use LoggerAwareTrait; + /** * @var list|null ordered group keys; null means no grouping (insertion order only) */ @@ -25,8 +33,6 @@ class Pipeline extends TypedList protected ?string $defaultGroup = null; - protected LoggerInterface $logger; - /** * @param list $pipes * @param list|null $groups Ordered group names/enums. When set, process() executes pipes in group order. @@ -71,7 +77,7 @@ public function walk(callable $walker): static public function process(mixed $payload) { foreach ($this->ordered() as $pipe) { - if ($pipe instanceof LoggerAwareInterface) { + if ($pipe instanceof LoggerAwareInterface && $this->logger instanceof LoggerInterface) { $pipe->setLogger($this->logger); } $payload = $pipe($payload) ?? $payload; @@ -149,7 +155,7 @@ public function configure(array $config): void if (method_exists($pipe, $setter)) { $pipe->{$setter}($value); } else { - $this->logger->warning("Unknown config option '{$pipeKey}.{$name}'"); + $this->logger?->warning("Unknown config option '{$pipeKey}.{$name}'"); } } }; @@ -158,7 +164,7 @@ public function configure(array $config): void foreach (array_keys($config) as $pipeKey) { if (!isset($applied[$pipeKey])) { - $this->logger->warning("Unknown config key '{$pipeKey}'; no matching pipe in this pipeline"); + $this->logger?->warning("Unknown config key '{$pipeKey}'; no matching pipe in this pipeline"); } } } diff --git a/tests/Augmenter/MergeTest.php b/tests/Augmenter/MergeTest.php new file mode 100644 index 000000000..a4f2b2947 --- /dev/null +++ b/tests/Augmenter/MergeTest.php @@ -0,0 +1,295 @@ +build(function (Specification $specification): void { + $specification->add( + new OA\Operation\Get(path: '/pets', summary: 'FIRST', responses: [new OA\Response(response: 200, description: 'ok')]), + new OA\Operation\Get(path: '/pets', summary: 'SECOND', responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }); + + $this->assertSame('SECOND', $result->toArray()['paths']['/pets']['get']['summary']); + $this->assertCount(1, $result->specification()->operations); + $this->assertMatchesWarning('Get "get /pets" is declared more than once', $result); + } + + /** + * Without the pass, the `+` union in `compilePaths()` keeps the *first* path item for a path + * while the *last* operation wins. + */ + public function testDuplicatePathItemsNowKeepTheLast(): void + { + $result = $this->build(function (Specification $specification): void { + $specification->add(new OA\Operation\Get(path: '/pets', responses: [new OA\Response(response: 200, description: 'ok')])); + + foreach (['FIRST', 'SECOND'] as $summary) { + $pathItem = new OA\PathItem(summary: $summary); + $pathItem->path = '/pets'; + $specification->add($pathItem); + } + }); + + $path = $result->toArray()['paths']['/pets']; + + $this->assertSame('SECOND', $path['summary']); + $this->assertArrayHasKey('get', $path, 'the path item is folded into the operations already written for the path'); + $this->assertMatchesWarning('PathItem "/pets" is declared more than once', $result); + } + + public function testDuplicateWebhookOperationsAreReported(): void + { + $result = $this->build(function (Specification $specification): void { + $specification->add( + new OA\Operation\Post(webhook: 'petCreated', summary: 'FIRST', responses: [new OA\Response(response: 200, description: 'ok')]), + new OA\Operation\Post(webhook: 'petCreated', summary: 'SECOND', responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }); + + $this->assertSame('SECOND', $result->toArray()['webhooks']['petCreated']['post']['summary']); + $this->assertMatchesWarning('Post "post petCreated" is declared more than once', $result); + } + + public function testDuplicateTagsKeepTheLast(): void + { + $result = $this->build(function (Specification $specification): void { + $specification->add( + new OA\Tag(name: 'pets', description: 'FIRST'), + new OA\Tag(name: 'pets', description: 'SECOND'), + new OA\Operation\Get(path: '/pets', tags: ['pets'], responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }); + + $tags = $result->toArray()['tags']; + + $this->assertCount(1, $tags, 'the specification requires each tag name in the list to be unique'); + $this->assertSame('SECOND', $tags[0]['description']); + $this->assertMatchesWarning('Tag "pets" is declared more than once', $result); + } + + public function testDuplicateComponentsKeepTheLast(): void + { + $result = $this->build(function (Specification $specification): void { + $specification->add( + new OA\Operation\Get(path: '/pets', responses: [ + new OA\Response(response: 200, description: 'ok', content: [ + new OA\MediaType\Json(schema: new OA\Schema(ref: '#/components/schemas/Pet')), + ]), + ]), + new OA\Schema(description: 'FIRST', component: 'Pet'), + new OA\Schema(description: 'SECOND', component: 'Pet'), + ); + }); + + $schemas = $result->toArray()['components']['schemas']; + + $this->assertCount(1, $schemas); + $this->assertSame('SECOND', $schemas['Pet']['description']); + $this->assertMatchesWarning('Schema "Pet" is declared more than once', $result); + } + + /** + * Servers and security requirements are positional: their entries have no identity, two + * identical ones are legal, and nothing is reduced or reported. + */ + public function testPositionalListsAreLeftAlone(): void + { + $result = $this->build(function (Specification $specification): void { + $specification->add( + new OA\Server(url: 'https://example.com'), + new OA\Server(url: 'https://example.com'), + new OA\Operation\Get(path: '/pets', responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }); + + $this->assertCount(2, $result->toArray()['servers']); + $this->assertSame([], $result->warnings()); + } + + /** + * The policy a contributing package registers: what the scan described keeps the key, and + * the package recognises its own half by what it put in `meta`. + */ + public function testAMergerCanRecogniseWhatItsProducerMarked(): void + { + $result = (new Builder()) + ->setMode(Mode::SPEC) + ->withMergers(fn (TypedList $mergers): TypedList => $mergers->insert(new ContributionsYield(), Merge\LastWins::class)) + ->withSpecification(function (Specification $specification): void { + $specification->add( + new OA\Info(title: 'T', version: '1.0'), + new OA\Operation\Get(path: '/pets', summary: 'DESCRIBED', responses: [new OA\Response(response: 200, description: 'ok')]), + (new OA\Operation\Get(path: '/pets', summary: 'CONTRIBUTED', responses: [new OA\Response(response: 200, description: 'ok')])) + ->setMeta(ContributionsYield::class, true), + ); + }) + ->build(); + + $this->assertSame('DESCRIBED', $result->toArray()['paths']['/pets']['get']['summary']); + $this->assertSame([], $result->warnings()); + } + + public function testTheMergerThatClaimsTheTypeFirstDecides(): void + { + $result = (new Builder()) + ->setMode(Mode::SPEC) + ->withMergers(fn (TypedList $mergers): TypedList => $mergers->insert(new FirstWinsForOperations(), Merge\LastWins::class)) + ->withSpecification(function (Specification $specification): void { + $specification->add( + new OA\Info(title: 'T', version: '1.0'), + new OA\Operation\Get(path: '/pets', summary: 'FIRST', tags: ['pets'], responses: [new OA\Response(response: 200, description: 'ok')]), + new OA\Operation\Get(path: '/pets', summary: 'SECOND', tags: ['pets'], responses: [new OA\Response(response: 200, description: 'ok')]), + new OA\Tag(name: 'pets', description: 'FIRST'), + new OA\Tag(name: 'pets', description: 'SECOND'), + ); + }) + ->build(); + + $this->assertSame('FIRST', $result->toArray()['paths']['/pets']['get']['summary'], 'the registered merger claims operations'); + $this->assertSame('SECOND', $result->toArray()['tags'][0]['description'], 'everything else still falls to the catch-all'); + $this->assertCount(1, $result->warnings(), 'a merger that folds silently reports nothing'); + } + + /** + * The pass is registered twice, and the second run is the last of the default pipes, so + * what an augmenter adds is reduced too. + */ + public function testTheLastRunReducesWhatAnAugmenterAdded(): void + { + $result = (new Builder()) + ->setMode(Mode::SPEC) + ->withAugmenters(fn ($pipeline) => $pipeline->insert( + function (Specification $specification): void { + $specification->add(new OA\Operation\Get(path: '/pets', summary: 'LATE', responses: [new OA\Response(response: 200, description: 'ok')])); + }, + Augmenter\Merge::class, + )) + ->withSpecification(function (Specification $specification): void { + $specification->add( + new OA\Info(title: 'T', version: '1.0'), + new OA\Operation\Get(path: '/pets', summary: 'FIRST', responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }) + ->build(); + + $this->assertSame('LATE', $result->toArray()['paths']['/pets']['get']['summary']); + $this->assertMatchesWarning('Get "get /pets" is declared more than once', $result); + } + + public function testRemovingThePassRestoresTheUndiagnosedLastWins(): void + { + $result = (new Builder()) + ->setMode(Mode::SPEC) + ->withAugmenters(fn ($pipeline) => $pipeline->remove(Augmenter\Merge::class)) + ->withSpecification(function (Specification $specification): void { + $specification->add( + new OA\Info(title: 'T', version: '1.0'), + new OA\Operation\Get(path: '/pets', summary: 'FIRST', responses: [new OA\Response(response: 200, description: 'ok')]), + new OA\Operation\Get(path: '/pets', summary: 'SECOND', responses: [new OA\Response(response: 200, description: 'ok')]), + ); + }) + ->build(); + + $this->assertCount(2, $result->specification()->operations, 'both runs go, so nothing is reduced'); + $this->assertSame('SECOND', $result->toArray()['paths']['/pets']['get']['summary']); + $this->assertStringStartsWith( + 'operationId must be unique', + $result->warnings()[0], + 'the only signal left is the accidental one, and it guards a different key', + ); + $this->assertCount(1, $result->warnings()); + } + + protected function build(callable $contribution): Builder\Result + { + return (new Builder()) + ->setMode(Mode::SPEC) + ->withSpecification(function (Specification $specification) use ($contribution): void { + $specification->add(new OA\Info(title: 'T', version: '1.0')); + $contribution($specification); + }) + ->build(); + } + + protected function assertMatchesWarning(string $expected, Builder\Result $result): void + { + $matching = array_filter($result->warnings(), fn (string $warning): bool => str_contains($warning, $expected)); + + $this->assertCount(1, $matching, "expected exactly one warning containing '{$expected}', got: " . implode(' | ', $result->warnings())); + } +} + +/** + * A consumer's precedence policy: an operation already described stays as it is, and folding is + * not worth reporting. `supports()` claims one type, so everything else falls through to the + * catch-all registered behind it. + */ +final class FirstWinsForOperations implements MergerInterface +{ + public function supports(string $class): bool + { + return is_a($class, OA\Operation::class, true); + } + + public function identity(AttributeInterface $attribute): ?string + { + return $attribute instanceof OA\Operation && $attribute->path !== null && $attribute->method !== null + ? $attribute->method . ' ' . $attribute->path + : null; + } + + public function merge(AttributeInterface $earlier, AttributeInterface $later): AttributeInterface + { + return $earlier; + } +} + +/** + * An operation marked as contributed gives way to one that is not. + */ +final class ContributionsYield implements MergerInterface +{ + public function supports(string $class): bool + { + return is_a($class, OA\Operation::class, true); + } + + public function identity(AttributeInterface $attribute): ?string + { + return $attribute instanceof OA\Operation && $attribute->path !== null && $attribute->method !== null + ? $attribute->method . ' ' . $attribute->path + : null; + } + + public function merge(AttributeInterface $earlier, AttributeInterface $later): AttributeInterface + { + return $later->getMeta(self::class, false) ? $earlier : $later; + } +} diff --git a/tests/Spec/MetaTest.php b/tests/Spec/MetaTest.php new file mode 100644 index 000000000..865a92d10 --- /dev/null +++ b/tests/Spec/MetaTest.php @@ -0,0 +1,67 @@ +assertNull($schema->getMeta('adapter')); + $this->assertSame('none', $schema->getMeta('adapter', 'none')); + } + + public function testANullValueIsStillSet(): void + { + $schema = (new OA\Schema())->setMeta('adapter', null); + + $this->assertNull($schema->getMeta('adapter', 'none')); + } + + /** + * `Augmenter\PathItems` and `Augmenter\Inheritance` clone attributes; an entry set on the + * clone must not appear on the original. + */ + public function testACloneKeepsItsOwnEntries(): void + { + $schema = (new OA\Schema())->setMeta('adapter', 'symfony'); + + $clone = (clone $schema)->setMeta('adapter', 'laravel'); + + $this->assertSame('symfony', $schema->getMeta('adapter')); + $this->assertSame('laravel', $clone->getMeta('adapter')); + } + + /** + * Classic's `Context` started as a place to carry information and became something the + * processors branch on, one read at a time. `meta` is for code outside swagger-php, so a + * reader or writer appearing in `src/` is a decision this test makes someone take on purpose. + */ + public function testNothingInTheCoreUsesMeta(): void + { + $users = []; + + /** @var \SplFileInfo $file */ + foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator(__DIR__ . '/../../src')) as $file) { + if ($file->getExtension() !== 'php') { + continue; + } + + if (preg_match('/->(get|set)Meta\(|->meta\b/', (string) file_get_contents($file->getPathname()))) { + $users[] = $file->getBasename(); + } + } + + sort($users); + + $this->assertSame(['AbstractAttribute.php'], $users, 'meta belongs to whoever extends swagger-php; core code using it is new behaviour'); + } +} diff --git a/tests/Utils/PipelineTest.php b/tests/Utils/PipelineTest.php index f7b6865d2..a9156e882 100644 --- a/tests/Utils/PipelineTest.php +++ b/tests/Utils/PipelineTest.php @@ -14,6 +14,8 @@ use OpenApi\Utils\Pipeline; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; final class PipelineTest extends TestCase { @@ -24,6 +26,31 @@ public function testProcess(): void $this->assertSame('x', $pipeline->process('')); } + /** + * The pipeline hands its logger to every pipe that asks, and the builder only has the build's + * logger once the pipeline has been built. + */ + public function testALoggerSetAfterConstructionReachesThePipes(): void + { + $pipe = new class () implements LoggerAwareInterface { + use LoggerAwareTrait; + + public function __invoke(mixed $payload): mixed + { + $this->logger?->warning('from the pipe'); + + return $payload; + } + }; + + $logger = new CollectingLogger(); + $pipeline = new Pipeline([$pipe]); + $pipeline->setLogger($logger); + $pipeline->process(''); + + $this->assertSame([['level' => 'warning', 'message' => 'from the pipe']], $logger->entries()); + } + public static function configCases(): \Iterator { yield 'default' => [[], true]; diff --git a/tools/src/Docs/Reference/AugmenterGenerator.php b/tools/src/Docs/Reference/AugmenterGenerator.php index 64bde20b9..2baacba34 100644 --- a/tools/src/Docs/Reference/AugmenterGenerator.php +++ b/tools/src/Docs/Reference/AugmenterGenerator.php @@ -65,12 +65,13 @@ protected function collectAugmenterDetails(): array $augmenters = []; $builder = new Builder(); + // a pipe registered more than once — `Augmenter\Merge` runs twice — is one entry here $builder->getAugmenters()->walk(function ($augmenter) use (&$augmenters): void { $rc = new \ReflectionClass($augmenter); - $augmenters[] = $this->collectAugmenterData($rc); + $augmenters[$rc->getName()] ??= $this->collectAugmenterData($rc); }); - return $augmenters; + return array_values($augmenters); } /** diff --git a/tools/src/Docs/Reference/ExtensionPointGenerator.php b/tools/src/Docs/Reference/ExtensionPointGenerator.php index 0c831e77e..dbd542d2d 100644 --- a/tools/src/Docs/Reference/ExtensionPointGenerator.php +++ b/tools/src/Docs/Reference/ExtensionPointGenerator.php @@ -6,7 +6,9 @@ namespace OpenApi\Tools\Docs\Reference; +use OpenApi\Builder; use OpenApi\Contracts\AttributeTranslatorInterface; +use OpenApi\Contracts\MergerInterface; use OpenApi\Contracts\ResolverInterface; use OpenApi\Resolver; use OpenApi\Tools\Docs\DocGenerator; @@ -44,6 +46,12 @@ public function generate(): array $content .= $this->renderSections($data); } + $content .= "\n" . $this->renderer->sectionHeader('Default Mergers'); + foreach ($this->collect($this->mergers()) as $data) { + $content .= "\n" . $this->renderer->classHeader($data['name'], 'Merge'); + $content .= $this->renderSections($data); + } + return ['extension-points' => $content]; } @@ -71,6 +79,14 @@ function (TypedList $list) use (&$resolvers): void { return $resolvers; } + /** + * @return list + */ + protected function mergers(): array + { + return array_values(iterator_to_array((new Builder())->getMergers())); + } + /** * @param list $instances * @return list, see: list}>