Reliably pick root schema in Analysis::getSchemaForSource() (#1028)

Flag annotations created by processors as `_aux`. Also pick schema from the end of a context's annotation list as the root annotation should be last (excluding annotations created during processing).
zircote · Dec 20, 2021 · 31210d8 · 31210d8
1 parent db7c984
commit 31210d8
Show file tree

Hide file tree

Showing 18 changed files with 213 additions and 18 deletions.
diff --git a/Examples/Readme.md b/Examples/Readme.md
@@ -35,6 +35,7 @@ Collection of code/annotation examples and their corresponding OpenAPI specs gen
   * using traits: [source](using-traits) / [spec](using-traits/using-traits.yaml)
   * using refs: [source](using-refs) / [spec](using-refs/using-refs.yaml) 
   * nested schemas and class hierachies: [source](nesting) / [spec](nesting/nesting.yaml)
+  * polymorphism using `@OA\Discriminator`: [source](polymorphism) / [spec](polymorphism/polymorphism.yaml)
 
 
 ## Custom processors

diff --git a/Examples/polymorphism/AbstractResponsible.php b/Examples/polymorphism/AbstractResponsible.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace OpenApi\Examples\Polymorphism;
+
+use OpenApi\Annotations as OA;
+
+/**
+ * @OA\Schema(
+ *     schema="Responsible",
+ *     @OA\Discriminator(
+ *          propertyName="type",
+ *          mapping={
+ *              "fl": "#/components/schemas/FlResponsible",
+ *              "employee": "#/components/schemas/EmployeeResponsible"
+ *          }
+ *     ),
+ *     oneOf={
+ *          @OA\Schema(ref="#/components/schemas/FlResponsible"),
+ *          @OA\Schema(ref="#/components/schemas/EmployeeResponsible")
+ *     }
+ * )
+ */
+abstract class AbstractResponsible
+{
+    protected const TYPE = null;
+
+    /**
+     * @OA\Property(nullable=false, enum={"employee","assignee","fl"})
+     *
+     * @var string
+     */
+    protected $type;
+
+    public function __construct()
+    {
+        $this->type = static::TYPE;
+    }
+}
diff --git a/Examples/polymorphism/Controller.php b/Examples/polymorphism/Controller.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace OpenApi\Examples\Polymorphism;
+
+use OpenApi\Annotations as OA;
+
+class Controller
+{
+
+    /**
+     * @OA\Info(title="Polymorphism",version="1")
+     *
+     * @OA\Get(
+     *   path="/test",
+     *   @OA\Response(
+     *       response="default",
+     *       description="Polymorphism",
+     *       @OA\JsonContent(ref="#/components/schemas/Request")
+     *   )
+     * )
+     */
+    public function getProduct($id)
+    {
+    }
+}
diff --git a/Examples/polymorphism/Employee.php b/Examples/polymorphism/Employee.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace OpenApi\Examples\Polymorphism;
+
+use OpenApi\Annotations as OA;
+
+/**
+ * @OA\Schema(schema="EmployeeResponsible")
+ */
+final class Employee extends AbstractResponsible
+{
+    protected const TYPE = 'employee';
+
+    /**
+     * @OA\Property(nullable=false)
+     *
+     * @var string
+     */
+    public $property2;
+}
diff --git a/Examples/polymorphism/Fl.php b/Examples/polymorphism/Fl.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace OpenApi\Examples\Polymorphism;
+
+use OpenApi\Annotations as OA;
+
+/**
+ * @OA\Schema(schema="FlResponsible")
+ */
+final class Fl extends AbstractResponsible
+{
+    public const TYPE = 'fl';
+
+    /**
+     * @OA\Property(nullable=false)
+     *
+     * @var string
+     */
+    public $property3;
+}
diff --git a/Examples/polymorphism/Request.php b/Examples/polymorphism/Request.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace OpenApi\Examples\Polymorphism;
+
+use OpenApi\Annotations as OA;
+
+/**
+ * @OA\Schema(schema="Request")
+ */
+final class Request
+{
+    protected const TYPE = 'employee';
+
+    /**
+     * @OA\Property(nullable=false)
+     *
+     * @var AbstractResponsible
+     */
+    public $payload;
+}
diff --git a/Examples/polymorphism/polymorphism.yaml b/Examples/polymorphism/polymorphism.yaml
@@ -0,0 +1,61 @@
+openapi: 3.0.0
+info:
+  title: Polymorphism
+  version: '1'
+paths:
+  /test:
+    get:
+      operationId: ef7f8a05d4ce52bae215869c1decc7c1
+      responses:
+        default:
+          description: Polymorphism
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Request'
+components:
+  schemas:
+    Responsible:
+      properties:
+        type:
+          nullable: false
+          type: string
+          enum:
+            - employee
+            - assignee
+            - fl
+      type: object
+      discriminator:
+        propertyName: type
+        mapping:
+          fl: '#/components/schemas/FlResponsible'
+          employee: '#/components/schemas/EmployeeResponsible'
+      oneOf:
+        -
+          $ref: '#/components/schemas/FlResponsible'
+        -
+          $ref: '#/components/schemas/EmployeeResponsible'
+    EmployeeResponsible:
+      allOf:
+        -
+          properties:
+            property2:
+              nullable: false
+              type: string
+        -
+          $ref: '#/components/schemas/Responsible'
+    FlResponsible:
+      allOf:
+        -
+          properties:
+            property3:
+              nullable: false
+              type: string
+        -
+          $ref: '#/components/schemas/Responsible'
+    Request:
+      properties:
+        payload:
+          $ref: '#/components/schemas/Responsible'
+      type: object
+
diff --git a/src/Analysis.php b/src/Analysis.php
@@ -324,8 +324,8 @@ public function getSchemaForSource(string $fqdn): ?Schema
             if (array_key_exists($fqdn, $definitions)) {
                 $definition = $definitions[$fqdn];
                 if (is_iterable($definition['context']->annotations)) {
-                    foreach ($definition['context']->annotations as $annotation) {
-                        if (get_class($annotation) === Schema::class) {
+                    foreach (array_reverse($definition['context']->annotations) as $annotation) {
+                        if (get_class($annotation) === Schema::class && !$annotation->_aux) {
                             return $annotation;
                         }
                     }

diff --git a/src/Annotations/AbstractAnnotation.php b/src/Annotations/AbstractAnnotation.php
@@ -33,6 +33,11 @@ abstract class AbstractAnnotation implements \JsonSerializable
      */
     public $attachables = Generator::UNDEFINED;
 
+    /**
+     * @var bool
+     */
+    public $_aux = false;
+
     /**
      * @var Context
      */
@@ -88,7 +93,7 @@ abstract class AbstractAnnotation implements \JsonSerializable
      *
      * @var array
      */
-    public static $_blacklist = ['_context', '_unmerged', 'attachables'];
+    public static $_blacklist = ['_context', '_unmerged', '_analysis', '_aux', 'attachables'];
 
     public function __construct(array $properties)
     {

diff --git a/src/Annotations/Flow.php b/src/Annotations/Flow.php
@@ -57,11 +57,6 @@ abstract class AbstractFlow extends AbstractAnnotation
      */
     public static $_required = ['scopes', 'flow'];
 
-    /**
-     * @inheritdoc
-     */
-    public static $_blacklist = ['_context', '_unmerged'];
-
     /**
      * @inheritdoc
      */

diff --git a/src/Annotations/OpenApi.php b/src/Annotations/OpenApi.php
@@ -98,11 +98,6 @@ class AbstractOpenApi extends AbstractAnnotation
      */
     public $_analysis = Generator::UNDEFINED;
 
-    /**
-     * @inheritdoc
-     */
-    public static $_blacklist = ['_context', '_unmerged', '_analysis'];
-
     /**
      * @inheritdoc
      */

diff --git a/src/Processors/AugmentProperties.php b/src/Processors/AugmentProperties.php
@@ -124,8 +124,9 @@ protected function augmentType(Property $property, Context $context, array $refs
                 $refKey = $this->toRefKey($context, $type);
                 $property->oneOf = [
                     new Schema([
-                        '_context' => $property->_context,
                         'ref' => $refs[$refKey],
+                        '_context' => $property->_context,
+                        '_aux' => true,
                     ]),
                 ];
                 $property->nullable = true;
@@ -135,6 +136,7 @@ protected function augmentType(Property $property, Context $context, array $refs
                         [
                             'type' => $property->type,
                             '_context' => new Context(['generated' => true], $context),
+                            '_aux' => true,
                         ]
                     );
                     if ($property->ref !== Generator::UNDEFINED) {
@@ -204,8 +206,9 @@ protected function applyRef(Property $property, string $ref): void
         if ($property->nullable === true) {
             $property->oneOf = [
                 new Schema([
-                    '_context' => $property->_context,
                     'ref' => $ref,
+                    '_context' => $property->_context,
+                    '_aux' => true,
                 ]),
             ];
         } else {

diff --git a/src/Processors/AugmentSchemas.php b/src/Processors/AugmentSchemas.php
@@ -63,7 +63,10 @@ public function __invoke(Analysis $analysis)
                             }
 
                             if ($schema === null) {
-                                $schema = new Schema(['_context' => $annotation->_context]);
+                                $schema = new Schema([
+                                    '_context' => $annotation->_context,
+                                    '_aux' => true,
+                                ]);
                                 $annotation->allOf[] = $schema;
                             }
 
@@ -104,7 +107,11 @@ public function __invoke(Analysis $analysis)
                     }
                 }
                 if (!$allOfPropertiesSchema) {
-                    $allOfPropertiesSchema = new Schema(['_context' => $schema->_context, 'properties' => []]);
+                    $allOfPropertiesSchema = new Schema([
+                        'properties' => [],
+                        '_context' => $schema->_context,
+                        '_aux' => true,
+                    ]);
                     $schema->allOf[] = $allOfPropertiesSchema;
                 }
                 $allOfPropertiesSchema->properties = array_merge($allOfPropertiesSchema->properties, $schema->properties);

diff --git a/src/Processors/BuildPaths.php b/src/Processors/BuildPaths.php
@@ -45,6 +45,7 @@ public function __invoke(Analysis $analysis)
                         [
                             'path' => $operation->path,
                             '_context' => new Context(['generated' => true], $operation->_context),
+                            '_aux' => true,
                         ]
                     );
                     $analysis->annotations->attach($paths[$operation->path]);

diff --git a/src/Processors/MergeJsonContent.php b/src/Processors/MergeJsonContent.php
@@ -43,6 +43,7 @@ public function __invoke(Analysis $analysis)
                 'example' => $jsonContent->example,
                 'examples' => $jsonContent->examples,
                 '_context' => new Context(['generated' => true], $jsonContent->_context),
+                '_aux' => true,
             ]);
             if (!$parent instanceof Parameter) {
                 $parent->content['application/json']->mediaType = 'application/json';

diff --git a/src/Processors/MergeTrait.php b/src/Processors/MergeTrait.php
@@ -31,8 +31,9 @@ protected function inheritFrom(Schema $schema, Schema $from, string $refPath, Co
         }
         // merging other properties into allOf is done in the AugmentSchemas processor
         $schema->allOf[] = new Schema([
-            '_context' => $context,
             'ref' => Components::SCHEMA_REF . Util::refEncode($refPath),
+            '_context' => $context,
+            '_aux' => true,
         ]);
     }
 

diff --git a/src/Processors/MergeXmlContent.php b/src/Processors/MergeXmlContent.php
@@ -43,6 +43,7 @@ public function __invoke(Analysis $analysis)
                 'example' => $xmlContent->example,
                 'examples' => $xmlContent->examples,
                 '_context' => new Context(['generated' => true], $xmlContent->_context),
+                '_aux' => true,
             ]);
             if (!$parent instanceof Parameter) {
                 $parent->content['application/xml']->mediaType = 'application/xml';

diff --git a/tests/UtilTest.php b/tests/UtilTest.php
@@ -24,6 +24,7 @@ public function testExclude()
             'InheritProperties',
             'Apis',
             'PHP',
+            'Parser',
             'Analysers',
             'Processors',
             'UsingRefs.php',