Skip to content

[Backport #1971 to 5.x] Annotation validation refactoring #1972

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.

Sign up for GitHub

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

Closed
github-actions wants to merge 1 commit into from
+228 −141
Closed

[Backport #1971 to 5.x] Annotation validation refactoring #1972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 39 additions & 4 deletions src/Analysis.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,12 +423,47 @@ public function process($processors = null): void

public function validate(): bool
{
if ($this->openapi instanceof OA\OpenApi) {
return $this->openapi->validate();
if (!$this->openapi instanceof OA\OpenApi) {
$this->context->logger->warning('No openapi target set. Run the MergeIntoOpenApi processor before validate()');

return false;
}

$isValid = true;
$version = $this->openapi->openapi;
$context = new \stdClass();

foreach ($this->collectAnnotations($this->openapi) as $annotation) {
$isValid = $annotation->validate($this, $version, $context) && $isValid;
}

$this->context->logger->warning('No openapi target set. Run the MergeIntoOpenApi processor before validate()');
return $isValid;

}

/**
* @return array<OA\AbstractAnnotation>
*/
protected function collectAnnotations(OA\AbstractAnnotation $root): array
{
$annotations = [$root];

foreach (get_object_vars($root) as $field => $value) {
if (null === $value || Generator::isDefault($value) || is_scalar($value) || in_array($field, $root::$_blacklist)) {
continue;
}

return false;
if ($value instanceof OA\AbstractAnnotation) {
$annotations = array_merge($annotations, $this->collectAnnotations($value));
} elseif (is_array($value)) {
foreach ($value as $item) {
if ($item instanceof OA\AbstractAnnotation) {
$annotations = array_merge($annotations, $this->collectAnnotations($item));
}
}
}
}

return $annotations;
}
}
171 changes: 105 additions & 66 deletions src/Annotations/AbstractAnnotation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace OpenApi\Annotations;

use OpenApi\Analysis;
use OpenApi\Annotations as OA;
use OpenApi\Context;
use OpenApi\Generator;
Expand Down Expand Up @@ -450,28 +451,62 @@ public function jsonSerialize()
}

/**
* Validate annotation tree, and log notices & warnings.
*
* @param array $stack the path of annotations above this annotation in the tree
* @param array $skip (prevent stack overflow, when traversing an infinite dependency graph)
* @param string $ref Current ref path?
* @param object $context a free-form context contains
* Validate a given value against a `_$type` definition.
*/
public function validate(array $stack = [], array $skip = [], string $ref = '', ?object $context = null): bool
private function validateValueType(string $type, mixed $value): bool
{
if (in_array($this, $skip, true)) {
if (str_starts_with($type, '[') && str_ends_with($type, ']')) {
// $value must be an array
if (!$this->validateValueType('array', $value)) {
return false;
}

$itemType = substr($type, 1, -1);
foreach ($value as $item) {
if (!$this->validateValueType($itemType, $item)) {
return false;
}
}

return true;
}

$valid = true;
if (is_subclass_of($type, AbstractAnnotation::class)) {
$type = 'object';
}

$isValidType = fn (string $type, mixed $value): bool => match ($type) {
'string' => is_string($value),
'boolean' => is_bool($value),
'integer' => is_int($value),
'number' => is_numeric($value),
'object' => is_object($value),
'array' => is_array($value) && array_is_list($value),
'scheme' => in_array($value, ['http', 'https', 'ws', 'wss'], true),
default => throw new OpenApiException('Invalid type "' . $type . '"'),
};

foreach (explode('|', $type) as $tt) {
if ($isValidType(trim($tt), $value)) {
return true;
}
}

return false;
}

// Report orphaned annotations
public function validate(?Analysis $analysis = null, string $version = OpenApi::DEFAULT_VERSION, ?object $context = null): bool
{
$isValid = true;

// validate unmerged
foreach ($this->_unmerged as $annotation) {
if (!is_object($annotation)) {
$this->_context->logger->warning('Unexpected type: "' . gettype($annotation) . '" in ' . $this->identity() . '->_unmerged, expecting a Annotation object');
break;
}

<<<<<<< HEAD
/** @var class-string<AbstractAnnotation> $class */
$class = get_class($annotation);
if ($details = $this->matchNested($annotation)) {
Expand All @@ -485,14 +520,28 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
$message = 'Unexpected ' . $annotation->identity();
if ($class::$_parents) {
$message .= ', expected to be inside ' . implode(', ', Util::shorten($class::$_parents));
=======
if ($details = $this->matchNested($annotation)) {
$property = $details->value;
if (is_array($property)) {
$this->_context->logger->warning('Only one ' . $annotation->identity([]) . ' allowed for ' . $this->identity() . ' multiple found, skipped: ' . $annotation->_context);
} else {
$this->_context->logger->warning('Only one ' . $annotation->identity([]) . ' allowed for ' . $this->identity() . " multiple found in:\n Using: " . $this->{$property}->_context . "\n Skipped: " . $annotation->_context);
}
} elseif ($annotation instanceof AbstractAnnotation) {
$message = 'Unexpected ' . $annotation->identity();
if ($annotation::$_parents) {
$message .= ', expected to be inside ' . implode(', ', AbstractAnnotation::shorten($annotation::$_parents));
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
}
$this->_context->logger->warning($message . ' in ' . $annotation->_context);
}
$valid = false;

$isValid = false;
}

// Report conflicting key
foreach (static::$_nested as $annotationClass => $nested) {
// validate conflicting keys
foreach ($this::$_nested as $annotationClass => $nested) {
if (is_string($nested) || count($nested) === 1) {
continue;
}
Expand All @@ -502,9 +551,15 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
}
$keys = [];
$keyField = $nested[1];
/** @var AbstractAnnotation $item */
foreach ($this->{$property} as $key => $item) {
<<<<<<< HEAD
if (is_array($item) && is_numeric($key) === false) {
$this->_context->logger->warning($this->identity() . '->' . $property . ' is an object literal, use nested ' . Util::shorten($annotationClass) . '() annotation(s) in ' . $this->_context);
=======
if (is_array($item) && !is_numeric($key)) {
$this->_context->logger->warning($this->identity() . '->' . $property . ' is an object literal, use nested ' . AbstractAnnotation::shorten($annotationClass) . '() annotation(s) in ' . $this->_context);
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
$keys[$key] = $item;
} elseif (Generator::isDefault($item->{$keyField})) {
$this->_context->logger->error($item->identity() . ' is missing key-field: "' . $keyField . '" in ' . $item->_context);
Expand All @@ -516,29 +571,46 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
}
}

<<<<<<< HEAD
if (property_exists($this, 'ref') && !Generator::isDefault($this->ref) && is_string($this->ref)) {
if (substr($this->ref, 0, 2) === '#/' && $stack !== [] && $stack[0] instanceof OpenApi) {
// Internal reference
=======
// validate refs
if ($analysis?->openapi && property_exists($this, 'ref') && !Generator::isDefault($this->ref) && is_string($this->ref)) {
if (str_starts_with($this->ref, '#/')) {
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
try {
$stack[0]->ref($this->ref);
$analysis->openapi->ref($this->ref);
} catch (\Exception $e) {
$this->_context->logger->warning($e->getMessage() . ' for ' . $this->identity() . ' in ' . $this->_context, ['exception' => $e]);
$isValid = false;
}
}
} else {
// Report missing required fields (when not a $ref)
foreach (static::$_required as $property) {
}

// validate required properties
if (!property_exists($this, 'ref') || Generator::isDefault($this->ref) || !is_string($this->ref)) {
foreach ($this::$_required as $property) {
if (Generator::isDefault($this->{$property})) {
$message = 'Missing required field "' . $property . '" for ' . $this->identity() . ' in ' . $this->_context;
foreach (static::$_nested as $class => $nested) {
foreach ($this::$_nested as $class => $nested) {
$nestedProperty = is_array($nested) ? $nested[0] : $nested;
if ($property === $nestedProperty) {
if ($this instanceof OpenApi) {
<<<<<<< HEAD
$message = 'Required ' . Util::shorten($class) . '() not found';
} elseif (is_array($nested)) {
$message = $this->identity() . ' requires at least one ' . Util::shorten($class) . '() in ' . $this->_context;
} else {
$message = $this->identity() . ' requires a ' . Util::shorten($class) . '() in ' . $this->_context;
=======
$message = 'Required ' . AbstractAnnotation::shorten($class) . '() not found';
} elseif (is_array($nested)) {
$message = $this->identity() . ' requires at least one ' . AbstractAnnotation::shorten($class) . '() in ' . $this->_context;
} else {
$message = $this->identity() . ' requires a ' . AbstractAnnotation::shorten($class) . '() in ' . $this->_context;
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
}
break;
}
Expand All @@ -548,73 +620,36 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
}
}

// Report invalid types
foreach (static::$_types as $property => $type) {
// validate types
foreach ($this::$_types as $property => $type) {
$value = $this->{$property};
if (Generator::isDefault($value) || $value === null) {
continue;
}
if (is_string($type)) {
if ($this->validateType($type, $value) === false) {
$valid = false;
if (!$this->validateValueType($type, $value)) {
$this->_context->logger->warning($this->identity() . '->' . $property . ' is a "' . gettype($value) . '", expecting a "' . $type . '" in ' . $this->_context);
$isValid = false;
}
} elseif (is_array($type)) { // enum?
if (in_array($value, $type) === false) {
if (!in_array($value, $type)) {
$this->_context->logger->warning($this->identity() . '->' . $property . ' "' . $value . '" is invalid, expecting "' . implode('", "', $type) . '" in ' . $this->_context);
}
} else {
throw new OpenApiException('Invalid ' . get_class($this) . '::$_types[' . $property . ']');
}
}
$stack[] = $this;

// validate example/examples
if (property_exists($this, 'example') && property_exists($this, 'examples')) {
if (!Generator::isDefault($this->example) && !Generator::isDefault($this->examples)) {
$valid = false;
$this->_context->logger->warning($this->identity() . ': "example" and "examples" are mutually exclusive');
}
}

return self::_validate($this, $stack, $skip, $ref, $context) && $valid;
}

/**
* Recursively validate all annotation properties.
*
* @param array|object $fields
*/
private static function _validate($fields, array $stack, array $skip, string $baseRef, ?object $context): bool
{
$valid = true;
$blacklist = [];
if (is_object($fields)) {
if (in_array($fields, $skip, true)) {
return true;
}
$skip[] = $fields;
$blacklist = property_exists($fields, '_blacklist') ? $fields::$_blacklist : [];
}

foreach ($fields as $field => $value) {
if ($value === null || is_scalar($value) || in_array($field, $blacklist)) {
continue;
}
$ref = $baseRef !== '' ? $baseRef . '/' . urlencode((string) $field) : urlencode((string) $field);
if (is_object($value)) {
if (method_exists($value, 'validate')) {
if (!$value->validate($stack, $skip, $ref, $context)) {
$valid = false;
}
} elseif (!self::_validate($value, $stack, $skip, $ref, $context)) {
$valid = false;
}
} elseif (is_array($value) && !self::_validate($value, $stack, $skip, $ref, $context)) {
$valid = false;
$isValid = false;
}
}

return $valid;
return $isValid;
}

/**
Expand Down Expand Up @@ -652,7 +687,7 @@ public function identity(?array $properties = null): string
}

/**
* Check if <code>$other</code> can be nested and if so return details about where/how.
* Check if <code>$other</code> can be nested, and if so, return details about where/how.
*
* @param AbstractAnnotation $other the other annotation
*
Expand Down Expand Up @@ -691,10 +726,11 @@ public function getRoot(): string
/**
* Match the annotation root.
*
* @param class-string $rootClass the root class to match
* @param class-string $thisClass the root class to match
*/
public function isRoot(string $rootClass): bool
public function isRoot(string $thisClass): bool
{
<<<<<<< HEAD
return get_class($this) === $rootClass || $this->getRoot() === $rootClass;
}

Expand Down Expand Up @@ -784,6 +820,9 @@ private function validateArrayType($value): bool
}

return true;
=======
return static::class === $thisClass || $this->getRoot() === $thisClass;
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/Annotations/Items.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Items extends Schema
XmlContent::class,
Items::class,
];
<<<<<<< HEAD

/**
* @inheritdoc
Expand All @@ -58,4 +59,6 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',

return $valid;
}
=======
>>>>>>> e7fa8bb (Refactor annotation validation (#1971))
}
17 changes: 8 additions & 9 deletions src/Annotations/License.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace OpenApi\Annotations;

use OpenApi\Analysis;
use OpenApi\Generator;

/**
Expand Down Expand Up @@ -84,20 +85,18 @@ public function jsonSerialize()
return $data;
}

/**
* @inheritdoc
*/
public function validate(array $stack = [], array $skip = [], string $ref = '', ?object $context = null): bool
#[\Override]
public function validate(?Analysis $analysis = null, string $version = OpenApi::DEFAULT_VERSION, ?object $context = null): bool
{
$valid = parent::validate($stack, $skip, $ref, $context);
$isValid = parent::validate($analysis, $version, $context);

if (!$this->_context->isVersion('3.0.x')) {
if (!OpenApi::versionMatch($version, '3.0.x')) {
if (!Generator::isDefault($this->url) && !Generator::isDefault($this->identifier)) {
$this->_context->logger->warning($this->identity() . ' url and identifier are mutually exclusive');
$valid = false;
$this->_context->logger->warning($this->identity() . ' url and identifier are mutually exclusive in ' . $this->_context);
$isValid = false;
}
}

return $valid;
return $isValid;
}
}
Loading