Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor annotation root comparisons #1431

Merged
merged 2 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Analysers/AttributeAnnotationFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function build(\Reflector $reflector, Context $context): array
// merge backwards into parents...
$isParent = function (OA\AbstractAnnotation $annotation, OA\AbstractAnnotation $possibleParent): bool {
// regular annotation hierarchy
$explicitParent = null !== $possibleParent::matchNested(get_class($annotation)) && !$annotation instanceof OA\Attachable;
$explicitParent = null !== $possibleParent->matchNested($annotation) && !$annotation instanceof OA\Attachable;

$isParentAllowed = false;
// support Attachable subclasses
Expand All @@ -126,7 +126,7 @@ public function build(\Reflector $reflector, Context $context): array
}

// Property can be nested...
return get_class($annotation) != get_class($possibleParent)
return $annotation->getRoot() != $possibleParent->getRoot()
&& ($explicitParent || ($isAttachable && $isParentAllowed));
};

Expand Down
52 changes: 28 additions & 24 deletions src/Annotations/AbstractAnnotation.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public function merge(array $annotations, bool $ignore = false): array

foreach ($annotations as $annotation) {
$mapped = false;
if ($details = static::matchNested(get_class($annotation))) {
if ($details = $this->matchNested($annotation)) {
$property = $details->value;
if (is_array($property)) {
$property = $property[0];
Expand Down Expand Up @@ -416,7 +416,7 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',

/** @var class-string<AbstractAnnotation> $class */
$class = get_class($annotation);
if ($details = static::matchNested($class)) {
if ($details = $this->matchNested($annotation)) {
$property = $details->value;
if (is_array($property)) {
$this->_context->logger->warning('Only one ' . Util::shorten(get_class($annotation)) . '() allowed for ' . $this->identity() . ' multiple found, skipped: ' . $annotation->_context);
Expand Down Expand Up @@ -574,46 +574,50 @@ public function identity(): string
}

/**
* Find matching nested details.
* Check if `$other` can be nested and if so return details about where/how.
*
* @param class-string $class the class to match
* @param AbstractAnnotation $other the other annotation
*
* @return null|object key/value object or `null`
*/
public static function matchNested(string $class)
public function matchNested($other)
{
if (array_key_exists($class, static::$_nested)) {
return (object) ['key' => $class, 'value' => static::$_nested[$class]];
}

$parent = $class;
// only consider the immediate OpenApi parent
while (0 !== strpos($parent, 'OpenApi\\Annotations\\') && $parent = get_parent_class($parent)) {
if ($kvp = static::matchNested($parent)) {
return $kvp;
}
if ($other instanceof AbstractAnnotation && array_key_exists($root = $other->getRoot(), static::$_nested)) {
return (object) ['key' => $root, 'value' => static::$_nested[$root]];
}

return null;
}

/**
* Match the annotation root.
* Get the root annotation.
*
* @param class-string $rootClass the root class to match
* This is used for resolving type equality and nesting rules to allow those rules to also work for custom,
* derived annotation classes.
*
* @return class-string the root annotation class in the `OpenApi\\Annotations` namespace
*/
public function isRoot(string $rootClass): bool
public function getRoot(): string
{
$parent = get_class($this);
$class = get_class($this);

// only consider the immediate OpenApi parent
do {
if ($parent == $rootClass) {
return true;
if (0 === strpos($class, 'OpenApi\\Annotations\\')) {
break;
}
} while (0 !== strpos($parent, 'OpenApi\\Annotations\\') && $parent = get_parent_class($parent));
} while ($class = get_parent_class($class));

return false;
return $class;
}

/**
* Match the annotation root.
*
* @param class-string $rootClass the root class to match
*/
public function isRoot(string $rootClass): bool
{
return $this->getRoot() == $rootClass;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/Processors/ExpandClasses.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Attributes as OAT;
use OpenApi\Generator;

/**
Expand All @@ -25,7 +24,7 @@ class ExpandClasses implements ProcessorInterface
public function __invoke(Analysis $analysis)
{
/** @var OA\Schema[] $schemas */
$schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
$schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);

foreach ($schemas as $schema) {
if ($schema->_context->is('class')) {
Expand Down
10 changes: 2 additions & 8 deletions src/Processors/ExpandEnums.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Attributes as OAT;
use OpenApi\Generator;

/**
Expand All @@ -33,7 +32,7 @@ public function __invoke(Analysis $analysis)
protected function expandContextEnum(Analysis $analysis): void
{
/** @var OA\Schema[] $schemas */
$schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
$schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);

foreach ($schemas as $schema) {
if ($schema->_context->is('enum')) {
Expand Down Expand Up @@ -63,12 +62,7 @@ protected function expandContextEnum(Analysis $analysis): void
protected function expandSchemaEnum(Analysis $analysis): void
{
/** @var OA\Schema[] $schemas */
$schemas = $analysis->getAnnotationsOfType([
OA\Schema::class,
OAT\Schema::class,
OA\ServerVariable::class,
OAT\ServerVariable::class,
]);
$schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OA\ServerVariable::class]);

foreach ($schemas as $schema) {
if (Generator::isDefault($schema->enum)) {
Expand Down
3 changes: 1 addition & 2 deletions src/Processors/ExpandInterfaces.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Attributes as OAT;
use OpenApi\Generator;

/**
Expand All @@ -23,7 +22,7 @@ class ExpandInterfaces
public function __invoke(Analysis $analysis)
{
/** @var OA\Schema[] $schemas */
$schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
$schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);

foreach ($schemas as $schema) {
if ($schema->_context->is('class')) {
Expand Down
3 changes: 1 addition & 2 deletions src/Processors/ExpandTraits.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Attributes as OAT;
use OpenApi\Generator;

/**
Expand All @@ -23,7 +22,7 @@ class ExpandTraits implements ProcessorInterface
public function __invoke(Analysis $analysis)
{
/** @var OA\Schema[] $schemas */
$schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
$schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);

// do regular trait inheritance / merge
foreach ($schemas as $schema) {
Expand Down
5 changes: 4 additions & 1 deletion src/Processors/MergeIntoComponents.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ public function __invoke(Analysis $analysis)
$components = new OA\Components(['_context' => new Context(['generated' => true], $analysis->context)]);
}

/** @var OA\AbstractAnnotation $annotation */
foreach ($analysis->annotations as $annotation) {
if (OA\Components::matchNested(get_class($annotation)) && $annotation->_context->is('nested') === false) {
if ($annotation instanceof OA\AbstractAnnotation
&& in_array(OA\Components::class, $annotation::$_parents)
&& false === $annotation->_context->is('nested')) {
// A top level annotation.
$components->merge([$annotation], true);
$analysis->openapi->components = $components;
Expand Down
6 changes: 5 additions & 1 deletion src/Processors/MergeIntoOpenApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ public function __invoke(Analysis $analysis)
$openapi->paths[] = $path;
}
}
} elseif (OA\OpenApi::matchNested(get_class($annotation)) && property_exists($annotation, '_context') && $annotation->_context->is('nested') === false) {
} elseif (
$annotation instanceof OA\AbstractAnnotation
&& in_array(OA\OpenApi::class, $annotation::$_parents)
&& property_exists($annotation, '_context')
&& false === $annotation->_context->is('nested')) {
// A top level annotation.
$merge[] = $annotation;
}
Expand Down
3 changes: 1 addition & 2 deletions tests/Analysers/ReflectionAnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use OpenApi\Analysers\TokenAnalyser;
use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Attributes as OAT;
use OpenApi\Context;
use OpenApi\Generator;
use OpenApi\Processors\CleanUnusedComponents;
Expand Down Expand Up @@ -136,7 +135,7 @@ public function testApiAttributesBasic(AnalyserInterface $analyser): void

// check CustomAttachable is only attached to @OA\Get
/** @var OA\Get[] $gets */
$gets = $analysis->getAnnotationsOfType(OAT\Get::class, true);
$gets = $analysis->getAnnotationsOfType(OA\Get::class, true);
$this->assertCount(2, $gets);
$this->assertTrue(is_array($gets[0]->attachables), 'Attachables not set');
$this->assertCount(1, $gets[0]->attachables);
Expand Down
3 changes: 1 addition & 2 deletions tests/Annotations/AbstractAnnotationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public function nestedMatches(): iterable
$parameterMatch = (object) ['key' => OA\Parameter::class, 'value' => ['parameters']];

return [
'unknown' => [self::class, null],
'simple-match' => [OA\Parameter::class, $parameterMatch],
'invalid-annotation' => [OA\Schema::class, null],
'sub-annotation' => [SubParameter::class, $parameterMatch],
Expand All @@ -127,7 +126,7 @@ public function nestedMatches(): iterable
*/
public function testMatchNested(string $class, $expected): void
{
$this->assertEquals($expected, OA\Get::matchNested($class));
$this->assertEquals($expected, (new OA\Get([]))->matchNested(new $class([])));
}

public function testDuplicateOperationIdValidation(): void
Expand Down
Loading