Extend ExpandXXX processors to also consider ancestor traits (#1331)

zircote · Nov 3, 2022 · 6bd5335 · 6bd5335
1 parent b676979
commit 6bd5335
Show file tree

Hide file tree

Showing 18 changed files with 391 additions and 58 deletions.
diff --git a/Examples/polymorphism/polymorphism-3.1.0.yaml b/Examples/polymorphism/polymorphism-3.1.0.yaml
@@ -45,6 +45,7 @@ components:
         -
           $ref: '#/components/schemas/EmployeeResponsible'
     EmployeeResponsible:
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Responsible'
@@ -56,6 +57,7 @@ components:
               type: string
               const: Virtual
     FlResponsible:
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Responsible'

diff --git a/Examples/polymorphism/polymorphism.yaml b/Examples/polymorphism/polymorphism.yaml
@@ -46,6 +46,7 @@ components:
         -
           $ref: '#/components/schemas/EmployeeResponsible'
     EmployeeResponsible:
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Responsible'
@@ -59,6 +60,7 @@ components:
               enum:
                 - Virtual
     FlResponsible:
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Responsible'

diff --git a/Examples/using-interfaces/using-interfaces.yaml b/Examples/using-interfaces/using-interfaces.yaml
@@ -67,6 +67,7 @@ components:
               example: blue
     Product:
       title: 'Product model'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/ProductInterface'

diff --git a/Examples/using-traits/using-traits.yaml b/Examples/using-traits/using-traits.yaml
@@ -46,6 +46,7 @@ components:
   schemas:
     BellsAndWhistles:
       title: 'Bells and Whistles trait'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Bells'
@@ -86,6 +87,7 @@ components:
       type: object
     Product:
       title: 'Product model'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Colour'
@@ -103,6 +105,7 @@ components:
               example: gong
     SimpleProduct:
       title: 'SimpleProduct model'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/Bells'
@@ -115,6 +118,7 @@ components:
               example: 1
     TrickyProduct:
       title: 'TrickyProduct model'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/SimpleProduct'

diff --git a/src/Processors/AugmentSchemas.php b/src/Processors/AugmentSchemas.php
@@ -57,28 +57,6 @@ public function __invoke(Analysis $analysis)
                             continue;
                         }
 
-                        if (!Generator::isDefault($annotation->allOf)) {
-                            $schema = null;
-                            foreach ($annotation->allOf as $nestedSchema) {
-                                if (!Generator::isDefault($nestedSchema->ref)) {
-                                    continue;
-                                }
-
-                                $schema = $nestedSchema;
-                            }
-
-                            if ($schema === null) {
-                                $schema = new OA\Schema([
-                                    '_context' => new Context(['generated' => true], $annotation->_context),
-                                ]);
-                                $analysis->addAnnotation($schema, $schema->_context);
-                                $annotation->allOf[] = $schema;
-                            }
-
-                            $schema->merge([$property], true);
-                            break;
-                        }
-
                         $annotation->merge([$property], true);
                         break;
                     }

diff --git a/src/Processors/Concerns/MergeTrait.php → ...cessors/Concerns/MergePropertiesTrait.php b/src/Processors/Concerns/MergeTrait.php → ...cessors/Concerns/MergePropertiesTrait.php
@@ -20,7 +20,7 @@
  *      - merge from all without schema
  *        => update all $ref that might reference a property merged.
  */
-trait MergeTrait
+trait MergePropertiesTrait
 {
     protected function inheritFrom(Analysis $analysis, OA\Schema $schema, OA\Schema $from, string $refPath, Context $context): void
     {
@@ -35,23 +35,11 @@ protected function inheritFrom(Analysis $analysis, OA\Schema $schema, OA\Schema
         $analysis->addAnnotation($refSchema, $refSchema->_context);
     }
 
-    protected function mergeAnnotations(OA\Schema $schema, array $from, array &$existing): void
-    {
-        if (is_iterable($from['context']->annotations)) {
-            foreach ($from['context']->annotations as $annotation) {
-                if ($annotation instanceof OA\Property && !in_array($annotation->_context->property, $existing, true)) {
-                    $existing[] = $annotation->_context->property;
-                    $schema->merge([$annotation], true);
-                }
-            }
-        }
-    }
-
     protected function mergeProperties(OA\Schema $schema, array $from, array &$existing): void
     {
-        foreach ($from['properties'] as $method) {
-            if (is_iterable($method->annotations)) {
-                foreach ($method->annotations as $annotation) {
+        foreach ($from['properties'] as $context) {
+            if (is_iterable($context->annotations)) {
+                foreach ($context->annotations as $annotation) {
                     if ($annotation instanceof OA\Property && !in_array($annotation->_context->property, $existing, true)) {
                         $existing[] = $annotation->_context->property;
                         $schema->merge([$annotation], true);
@@ -63,9 +51,9 @@ protected function mergeProperties(OA\Schema $schema, array $from, array &$exist
 
     protected function mergeMethods(OA\Schema $schema, array $from, array &$existing): void
     {
-        foreach ($from['methods'] as $method) {
-            if (is_iterable($method->annotations)) {
-                foreach ($method->annotations as $annotation) {
+        foreach ($from['methods'] as $context) {
+            if (is_iterable($context->annotations)) {
+                foreach ($context->annotations as $annotation) {
                     if ($annotation instanceof OA\Property && !in_array($annotation->_context->property, $existing, true)) {
                         $existing[] = $annotation->_context->property;
                         $schema->merge([$annotation], true);

diff --git a/src/Processors/ExpandClasses.php b/src/Processors/ExpandClasses.php
@@ -13,12 +13,14 @@
 
 /**
  * Iterate over the chain of ancestors of a schema and:
- * - merge ancestor annotations/methods/properties into the schema if the ancestor doesn't have a schema itself
- * - inherit from the ancestor if it has a schema (allOf) and stop.
+ * - if the ancestor has a schema
+ *   => inherit from the ancestor if it has a schema (allOf) and stop.
+ * - else
+ *   => merge ancestor properties into the schema.
  */
 class ExpandClasses
 {
-    use Concerns\MergeTrait;
+    use Concerns\MergePropertiesTrait;
 
     public function __invoke(Analysis $analysis)
     {
@@ -38,7 +40,6 @@ public function __invoke(Analysis $analysis)
                         // one ancestor is enough
                         break;
                     } else {
-                        $this->mergeAnnotations($schema, $ancestor, $existing);
                         $this->mergeMethods($schema, $ancestor, $existing);
                         $this->mergeProperties($schema, $ancestor, $existing);
                     }

diff --git a/src/Processors/ExpandInterfaces.php b/src/Processors/ExpandInterfaces.php
@@ -18,7 +18,7 @@
  */
 class ExpandInterfaces
 {
-    use Concerns\MergeTrait;
+    use Concerns\MergePropertiesTrait;
 
     public function __invoke(Analysis $analysis)
     {
@@ -47,7 +47,6 @@ public function __invoke(Analysis $analysis)
                         $refPath = !Generator::isDefault($interfaceSchema->schema) ? $interfaceSchema->schema : $interface['interface'];
                         $this->inheritFrom($analysis, $schema, $interfaceSchema, $refPath, $interface['context']);
                     } else {
-                        $this->mergeAnnotations($schema, $interface, $existing);
                         $this->mergeMethods($schema, $interface, $existing);
                     }
                 }

diff --git a/src/Processors/ExpandTraits.php b/src/Processors/ExpandTraits.php
@@ -18,26 +18,58 @@
  */
 class ExpandTraits
 {
-    use Concerns\MergeTrait;
+    use Concerns\MergePropertiesTrait;
 
     public function __invoke(Analysis $analysis)
     {
         /** @var OA\Schema[] $schemas */
         $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
 
+        // do regular trait inheritance / merge
         foreach ($schemas as $schema) {
-            if ($schema->_context->is('class') || $schema->_context->is('trait')) {
-                $source = $schema->_context->class ?: $schema->_context->trait;
-                $traits = $analysis->getTraitsOfClass($schema->_context->fullyQualifiedName($source), true);
+            if ($schema->_context->is('trait')) {
+                $traits = $analysis->getTraitsOfClass($schema->_context->fullyQualifiedName($schema->_context->trait), true);
                 $existing = [];
                 foreach ($traits as $trait) {
                     $traitSchema = $analysis->getSchemaForSource($trait['context']->fullyQualifiedName($trait['trait']));
                     if ($traitSchema) {
                         $refPath = !Generator::isDefault($traitSchema->schema) ? $traitSchema->schema : $trait['trait'];
                         $this->inheritFrom($analysis, $schema, $traitSchema, $refPath, $trait['context']);
                     } else {
-                        if ($schema->_context->is('class')) {
-                            $this->mergeAnnotations($schema, $trait, $existing);
+                        $this->mergeMethods($schema, $trait, $existing);
+                        $this->mergeProperties($schema, $trait, $existing);
+                    }
+                }
+            }
+        }
+
+        foreach ($schemas as $schema) {
+            if ($schema->_context->is('class') && !$schema->_context->is('generated')) {
+                // look at class traits
+                $traits = $analysis->getTraitsOfClass($schema->_context->fullyQualifiedName($schema->_context->class), true);
+                $existing = [];
+                foreach ($traits as $trait) {
+                    $traitSchema = $analysis->getSchemaForSource($trait['context']->fullyQualifiedName($trait['trait']));
+                    if ($traitSchema) {
+                        $refPath = !Generator::isDefault($traitSchema->schema) ? $traitSchema->schema : $trait['trait'];
+                        $this->inheritFrom($analysis, $schema, $traitSchema, $refPath, $trait['context']);
+                    } else {
+                        $this->mergeMethods($schema, $trait, $existing);
+                        $this->mergeProperties($schema, $trait, $existing);
+                    }
+                }
+
+                // also merge ancestor traits of non schema parents
+                $ancestors = $analysis->getSuperClasses($schema->_context->fullyQualifiedName($schema->_context->class));
+                $existing = [];
+                foreach ($ancestors as $ancestor) {
+                    $ancestorSchema = $analysis->getSchemaForSource($ancestor['context']->fullyQualifiedName($ancestor['class']));
+                    if ($ancestorSchema) {
+                        // stop here as we inherit everything above
+                        break;
+                    } else {
+                        $traits = $analysis->getTraitsOfClass($schema->_context->fullyQualifiedName($ancestor['class']), true);
+                        foreach ($traits as $trait) {
                             $this->mergeMethods($schema, $trait, $existing);
                             $this->mergeProperties($schema, $trait, $existing);
                         }

diff --git a/tests/Fixtures/Apis/basic.yaml b/tests/Fixtures/Apis/basic.yaml
@@ -129,6 +129,7 @@ components:
     Product:
       title: Product
       description: 'A Product.'
+      type: object
       allOf:
         -
           $ref: '#/components/schemas/NameTrait'
@@ -144,12 +145,12 @@ components:
               example: null
             colour:
               $ref: '#/components/schemas/Colour'
+            releasedAt:
+              type: string
             id:
               description: 'The id.'
               format: int64
               example: 1
-            releasedAt:
-              type: string
             kind:
               description: 'The kind.'
               type: string

diff --git a/tests/Fixtures/Scratch/MergeTraits.php b/tests/Fixtures/Scratch/MergeTraits.php
@@ -0,0 +1,79 @@
+<?php declare(strict_types=1);
+
+/**
+ * @license Apache 2.0
+ */
+
+namespace OpenApi\Tests\Fixtures\Scratch;
+
+use OpenApi\Annotations as OA;
+
+trait HasId
+{
+    /**
+     * @OA\Property(
+     *     format="int64",
+     *     readOnly=true,
+     * )
+     *
+     * @var int
+     */
+    public int $id;
+}
+
+trait HasTimestamps
+{
+    /**
+     * @OA\Property(
+     *     format="date-time",
+     *     type="string",
+     *     readOnly=true,
+     * )
+     */
+    public \DateTime $created_at;
+
+    /**
+     * @OA\Property(
+     *     format="date-time",
+     *     type="string",
+     *     readOnly=true,
+     * )
+     */
+    public \DateTime $updated_at;
+}
+
+abstract class Model
+{
+    use HasId;
+}
+
+/**
+ * @OA\Schema(
+ *     required={"street"},
+ *     @OA\Xml(
+ *         name="Address"
+ *     )
+ * )
+ */
+class Address extends Model
+{
+    use HasTimestamps;
+
+    /** @OA\Property */
+    public string $street;
+}
+
+/**
+ * @OA\Info(title="API", version="1.0")
+ * @OA\Get(
+ *     path="/api/endpoint",
+ *     @OA\Response(
+ *         response=200,
+ *         description="successful operation",
+ *         @OA\JsonContent(ref="#/components/schemas/Address")
+ *     )
+ * )
+ */
+class Endpoint
+{
+}
diff --git a/tests/Fixtures/Scratch/MergeTraits.yaml b/tests/Fixtures/Scratch/MergeTraits.yaml
@@ -0,0 +1,37 @@
+openapi: 3.0.0
+info:
+  title: API
+  version: '1.0'
+paths:
+  /api/endpoint:
+    get:
+      responses:
+        '200':
+          description: 'successful operation'
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Address'
+components:
+  schemas:
+    Address:
+      required:
+        - street
+      properties:
+        id:
+          type: integer
+          format: int64
+          readOnly: true
+        created_at:
+          type: string
+          format: date-time
+          readOnly: true
+        updated_at:
+          type: string
+          format: date-time
+          readOnly: true
+        street:
+          type: string
+      type: object
+      xml:
+        name: Address