Skip to content

Commit 2f117df

Browse files
DerManoMannDerManoMann
authored
Fix/clean unused components performance (#2030)
1 parent 9251c07 commit 2f117df

8 files changed

Lines changed: 269 additions & 95 deletions

File tree

‎composer.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
"rector": "Automatic refactoring",
8686
"lint": "Test codestyle",
8787
"test": "Run all PHP, codestyle and rector tests",
88+
"performance": "Run performance regression tests",
8889
"analyse": "Run static analysis (phpstan)",
8990
"spectral-examples": "Run spectral lint over all .yaml files in the docs/examples folder",
9091
"spectral-scratch": "Run spectral lint over all .yaml files in the tests/Fixtures/Scratch folder",
@@ -107,6 +108,7 @@
107108
"export XDEBUG_MODE=off && phpunit --display-all-issues",
108109
"@lint"
109110
],
111+
"performance": "export XDEBUG_MODE=off && phpunit --group performance",
110112
"analyse": [
111113
"export XDEBUG_MODE=off && phpstan analyse --memory-limit=3G"
112114
],

‎phpunit.xml.dist‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
processIsolation="false" stopOnFailure="false"
44
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.5/phpunit.xsd" cacheDirectory=".phpunit.cache"
55
backupStaticProperties="false">
6+
<groups>
7+
<exclude>
8+
<group>performance</group>
9+
</exclude>
10+
</groups>
611
<coverage/>
712
<testsuites>
813
<testsuite name="Swagger PHP Test Suite">

‎src/Generator.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ public function getDefaultConfig(): array
164164
'pathFilter' => [
165165
'tags' => [],
166166
'paths' => [],
167-
'recurseCleanup' => false,
168167
],
169168
'cleanUnusedComponents' => [
170169
'enabled' => false,

‎src/Processors/CleanUnusedComponents.php‎

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ public function __invoke(Analysis $analysis): void
4545
return;
4646
}
4747

48-
$analysis->annotations = $this->collectAnnotations($analysis->annotations);
49-
5048
// allow multiple runs to catch nested dependencies
5149
for ($ii = 0; $ii < 10; ++$ii) {
5250
if (!$this->cleanup($analysis)) {
@@ -60,14 +58,14 @@ protected function cleanup(Analysis $analysis): bool
6058
$usedRefs = [];
6159
foreach ($analysis->annotations as $annotation) {
6260
if (property_exists($annotation, 'ref') && !Generator::isDefault($annotation->ref) && $annotation->ref !== null) {
63-
$usedRefs[$annotation->ref] = $annotation->ref;
61+
$usedRefs[$annotation->ref] = true;
6462
}
6563

6664
foreach (['allOf', 'anyOf', 'oneOf'] as $sub) {
6765
if (property_exists($annotation, $sub) && !Generator::isDefault($annotation->{$sub})) {
6866
foreach ($annotation->{$sub} as $subElem) {
6967
if (is_object($subElem) && property_exists($subElem, 'ref') && !Generator::isDefault($subElem->ref) && $subElem->ref !== null) {
70-
$usedRefs[$subElem->ref] = $subElem->ref;
68+
$usedRefs[$subElem->ref] = true;
7169
}
7270
}
7371
}
@@ -77,47 +75,37 @@ protected function cleanup(Analysis $analysis): bool
7775
if (!Generator::isDefault($annotation->security)) {
7876
foreach ($annotation->security as $security) {
7977
foreach (array_keys($security) as $securityName) {
80-
$ref = OA\Components::COMPONENTS_PREFIX . 'securitySchemes/' . $securityName;
81-
$usedRefs[$ref] = $ref;
78+
$usedRefs[OA\Components::COMPONENTS_PREFIX . 'securitySchemes/' . $securityName] = true;
8279
}
8380
}
8481
}
8582
}
8683
}
8784

88-
$unusedRefs = [];
85+
$unusedComponents = [];
8986
foreach (OA\Components::$_nested as $nested) {
9087
if (2 == count($nested)) {
91-
// $nested[1] is the name of the property that holds the component name
9288
[$componentType, $nameProperty] = $nested;
9389
if (!Generator::isDefault($analysis->openapi->components->{$componentType})) {
94-
foreach ($analysis->openapi->components->{$componentType} as $component) {
90+
foreach ($analysis->openapi->components->{$componentType} as $ii => $component) {
9591
$ref = OA\Components::ref($component);
96-
if (!in_array($ref, $usedRefs)) {
97-
$unusedRefs[$ref] = [$ref, $nameProperty];
92+
if (!isset($usedRefs[$ref])) {
93+
$unusedComponents[] = [$componentType, $ii, $component];
9894
}
9995
}
10096
}
10197
}
10298
}
10399

104-
// remove unused
105-
foreach ($unusedRefs as $refDetails) {
106-
[$ref, $nameProperty] = $refDetails;
107-
[$hash, $components, $componentType, $name] = explode('/', $ref);
108-
foreach ($analysis->openapi->components->{$componentType} as $ii => $component) {
109-
if ($component->{$nameProperty} == $name) {
110-
$annotation = $analysis->openapi->components->{$componentType}[$ii];
111-
$this->removeAnnotation($analysis->annotations, $annotation);
112-
unset($analysis->openapi->components->{$componentType}[$ii]);
113-
114-
if (!$analysis->openapi->components->{$componentType}) {
115-
$analysis->openapi->components->{$componentType} = Generator::UNDEFINED;
116-
}
117-
}
100+
foreach ($unusedComponents as [$componentType, $ii, $component]) {
101+
$this->removeAnnotationRecursive($analysis, $component);
102+
unset($analysis->openapi->components->{$componentType}[$ii]);
103+
104+
if (!$analysis->openapi->components->{$componentType}) {
105+
$analysis->openapi->components->{$componentType} = Generator::UNDEFINED;
118106
}
119107
}
120108

121-
return [] !== $unusedRefs;
109+
return [] !== $unusedComponents;
122110
}
123111
}

‎src/Processors/Concerns/AnnotationTrait.php‎

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,61 +6,27 @@
66

77
namespace OpenApi\Processors\Concerns;
88

9+
use OpenApi\Analysis;
910
use OpenApi\Annotations as OA;
1011

1112
trait AnnotationTrait
1213
{
13-
/**
14-
* Collects a (complete) list of all nested/referenced annotations starting from the given root.
15-
*
16-
* @param string|array|iterable|OA\AbstractAnnotation $root
17-
*/
18-
public function collectAnnotations($root): \SplObjectStorage
14+
protected function removeAnnotationRecursive(Analysis $analysis, OA\AbstractAnnotation $annotation): void
1915
{
20-
$storage = new \SplObjectStorage();
16+
$analysis->removeAnnotation($annotation);
2117

22-
$this->traverseAnnotations($root, static function ($item) use (&$storage): void {
23-
if ($item instanceof OA\AbstractAnnotation && !$storage->offsetExists($item)) {
24-
$storage->offsetSet($item);
18+
foreach (get_object_vars($annotation) as $property => $value) {
19+
if (in_array($property, $annotation::$_blacklist)) {
20+
continue;
2521
}
26-
});
27-
28-
return $storage;
29-
}
30-
31-
/**
32-
* Remove all annotations that are part of the <code>$annotation</code> tree.
33-
*/
34-
public function removeAnnotation(iterable $root, OA\AbstractAnnotation $annotation, bool $recurse = true): void
35-
{
36-
$remove = $this->collectAnnotations($annotation);
37-
$this->traverseAnnotations($root, static function ($item) use ($remove): void {
38-
if ($item instanceof \SplObjectStorage) {
39-
foreach ($remove as $annotation) {
40-
$item->offsetUnset($annotation);
41-
}
42-
}
43-
}, $recurse);
44-
}
45-
46-
/**
47-
* @param string|array|iterable|OA\AbstractAnnotation $root
48-
*/
49-
public function traverseAnnotations($root, callable $callable, bool $recurse = true): void
50-
{
51-
$callable($root);
52-
53-
if (is_iterable($root) && $recurse) {
54-
foreach ($root as $value) {
55-
$this->traverseAnnotations($value, $callable, $recurse);
56-
}
57-
} elseif ($root instanceof OA\AbstractAnnotation) {
58-
foreach (array_merge($root::$_nested, ['allOf', 'anyOf', 'oneOf', 'callbacks']) as $properties) {
59-
foreach ((array) $properties as $property) {
60-
if (isset($root->{$property})) {
61-
$this->traverseAnnotations($root->{$property}, $callable, $recurse);
22+
if (is_array($value)) {
23+
foreach ($value as $item) {
24+
if ($item instanceof OA\AbstractAnnotation) {
25+
$this->removeAnnotationRecursive($analysis, $item);
6226
}
6327
}
28+
} elseif ($value instanceof OA\AbstractAnnotation) {
29+
$this->removeAnnotationRecursive($analysis, $value);
6430
}
6531
}
6632
}

‎src/Processors/PathFilter.php‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
use OpenApi\Analysis;
1010
use OpenApi\Generator;
11-
use OpenApi\Processors\Concerns\AnnotationTrait;
1211

1312
/**
1413
* Allows to filter endpoints based on tags and/or path.
@@ -19,19 +18,16 @@
1918
*/
2019
class PathFilter
2120
{
22-
use AnnotationTrait;
21+
use Concerns\AnnotationTrait;
2322

2423
protected array $tags;
2524

2625
protected array $paths;
2726

28-
protected bool $recurseCleanup;
29-
30-
public function __construct(array $tags = [], array $paths = [], bool $recurseCleanup = false)
27+
public function __construct(array $tags = [], array $paths = [])
3128
{
3229
$this->tags = $tags;
3330
$this->paths = $paths;
34-
$this->recurseCleanup = $recurseCleanup;
3531
}
3632

3733
public function getTags(): array
@@ -68,19 +64,6 @@ public function setPaths(array $paths): PathFilter
6864
return $this;
6965
}
7066

71-
public function isRecurseCleanup(): bool
72-
{
73-
return $this->recurseCleanup;
74-
}
75-
76-
/**
77-
* Flag to do a recursive cleanup of unused paths and their nested annotations.
78-
*/
79-
public function setRecurseCleanup(bool $recurseCleanup): void
80-
{
81-
$this->recurseCleanup = $recurseCleanup;
82-
}
83-
8467
public function __invoke(Analysis $analysis): void
8568
{
8669
if (($this->tags || $this->paths) && !Generator::isDefault($analysis->openapi->paths)) {
@@ -110,7 +93,7 @@ public function __invoke(Analysis $analysis): void
11093
if ($matched) {
11194
$filtered[] = $matched;
11295
} else {
113-
$this->removeAnnotation($analysis->annotations, $pathItem, $this->recurseCleanup);
96+
$this->removeAnnotationRecursive($analysis, $pathItem);
11497
}
11598
}
11699

0 commit comments

Comments
 (0)