Browse Source

Fixes for array/object representation in OpenAPI

shalvah 4 years ago
parent
commit
27a77136b1

+ 1 - 1
src/Extracting/ParamHelpers.php

@@ -142,7 +142,7 @@ trait ParamHelpers
      *
      *
      * @return string
      * @return string
      */
      */
-    protected function normalizeParameterType(?string $typeName): string
+    protected function normalizeTypeName(?string $typeName): string
     {
     {
         if (!$typeName) {
         if (!$typeName) {
             return 'string';
             return 'string';

+ 2 - 2
src/Extracting/Strategies/BodyParameters/GetFromBodyParamTag.php

@@ -88,13 +88,13 @@ class GetFromBodyParamTag extends Strategy
                 $required = trim($required) === 'required';
                 $required = trim($required) === 'required';
             }
             }
 
 
-            $type = $this->normalizeParameterType($type);
+            $type = $this->normalizeTypeName($type);
             [$description, $example] = $this->parseExampleFromParamDescription($description, $type);
             [$description, $example] = $this->parseExampleFromParamDescription($description, $type);
             $value = is_null($example) && !$this->shouldExcludeExample($tagContent)
             $value = is_null($example) && !$this->shouldExcludeExample($tagContent)
                 ? $this->generateDummyValue($type)
                 ? $this->generateDummyValue($type)
                 : $example;
                 : $example;
 
 
-            $parameters[$name] = compact('type', 'description', 'required', 'value');
+            $parameters[$name] = compact('name', 'type', 'description', 'required', 'value');
         }
         }
 
 
         return $parameters;
         return $parameters;

+ 1 - 0
src/Extracting/Strategies/BodyParameters/GetFromFormRequest.php

@@ -116,6 +116,7 @@ class GetFromFormRequest extends Strategy
             $userSpecifiedParameterInfo = $customParameterData[$parameter] ?? [];
             $userSpecifiedParameterInfo = $customParameterData[$parameter] ?? [];
 
 
             $parameterData = [
             $parameterData = [
+                'name' => $parameter,
                 'required' => false,
                 'required' => false,
                 'type' => null,
                 'type' => null,
                 'value' => self::$MISSING_VALUE,
                 'value' => self::$MISSING_VALUE,

+ 3 - 3
src/Extracting/Strategies/QueryParameters/GetFromQueryParamTag.php

@@ -105,7 +105,7 @@ class GetFromQueryParamTag extends Strategy
                         $type = 'string';
                         $type = 'string';
                         $required = true;
                         $required = true;
                     } else {
                     } else {
-                        $type = $this->normalizeParameterType($type);
+                        $type = $this->normalizeTypeName($type);
                         // Type in annotation is optional
                         // Type in annotation is optional
                         if (!$this->isSupportedTypeInDocBlocks($type)) {
                         if (!$this->isSupportedTypeInDocBlocks($type)) {
                             // Then that wasn't a type, but part of the description
                             // Then that wasn't a type, but part of the description
@@ -121,7 +121,7 @@ class GetFromQueryParamTag extends Strategy
 
 
                 $type = empty($type)
                 $type = empty($type)
                     ? (Str::contains($description, ['number', 'count', 'page']) ? 'integer' : 'string')
                     ? (Str::contains($description, ['number', 'count', 'page']) ? 'integer' : 'string')
-                    : $this->normalizeParameterType($type);
+                    : $this->normalizeTypeName($type);
 
 
             }
             }
 
 
@@ -130,7 +130,7 @@ class GetFromQueryParamTag extends Strategy
                 $value = $this->generateDummyValue($type);
                 $value = $this->generateDummyValue($type);
             }
             }
 
 
-            $parameters[$name] = compact('description', 'required', 'value', 'type');
+            $parameters[$name] = compact('name', 'description', 'required', 'value', 'type');
         }
         }
 
 
         return $parameters;
         return $parameters;

+ 2 - 2
src/Extracting/Strategies/ResponseFields/GetFromResponseFieldTag.php

@@ -53,7 +53,7 @@ class GetFromResponseFieldTag extends Strategy
                     $description = trim($description);
                     $description = trim($description);
                 }
                 }
 
 
-                $type = $this->normalizeParameterType($type);
+                $type = $this->normalizeTypeName($type);
 
 
                 // Support optional type in annotation
                 // Support optional type in annotation
                 if (!$this->isSupportedTypeInDocBlocks($type)) {
                 if (!$this->isSupportedTypeInDocBlocks($type)) {
@@ -76,7 +76,7 @@ class GetFromResponseFieldTag extends Strategy
                             ?? $validResponse['data'][0][$name] // Maybe an Api Resource Collection?
                             ?? $validResponse['data'][0][$name] // Maybe an Api Resource Collection?
                             ?? $nonexistent;
                             ?? $nonexistent;
                         if ($value !== $nonexistent) {
                         if ($value !== $nonexistent) {
-                            $type =  $this->normalizeParameterType(gettype($value));
+                            $type =  $this->normalizeTypeName(gettype($value));
                         } else {
                         } else {
                             $type = '';
                             $type = '';
                         }
                         }

+ 2 - 2
src/Extracting/Strategies/UrlParameters/GetFromUrlParamTag.php

@@ -99,7 +99,7 @@ class GetFromUrlParamTag extends Strategy
 
 
                 $type = empty($type)
                 $type = empty($type)
                     ? (Str::contains($description, ['number', 'count', 'page']) ? 'integer' : 'string')
                     ? (Str::contains($description, ['number', 'count', 'page']) ? 'integer' : 'string')
-                    : $this->normalizeParameterType($type);
+                    : $this->normalizeTypeName($type);
             }
             }
 
 
             [$description, $value] = $this->parseExampleFromParamDescription($description, $type);
             [$description, $value] = $this->parseExampleFromParamDescription($description, $type);
@@ -107,7 +107,7 @@ class GetFromUrlParamTag extends Strategy
                 $value = $this->generateDummyValue($type);
                 $value = $this->generateDummyValue($type);
             }
             }
 
 
-            $parameters[$name] = compact('description', 'required', 'value', 'type');
+            $parameters[$name] = compact('name', 'description', 'required', 'value', 'type');
         }
         }
 
 
         return $parameters;
         return $parameters;

+ 12 - 2
src/Tools/Utils.php

@@ -5,6 +5,7 @@ namespace Knuckles\Scribe\Tools;
 use Closure;
 use Closure;
 use Exception;
 use Exception;
 use Illuminate\Routing\Route;
 use Illuminate\Routing\Route;
+use Illuminate\Support\Str;
 use Knuckles\Scribe\Tools\ConsoleOutputUtils as c;
 use Knuckles\Scribe\Tools\ConsoleOutputUtils as c;
 use League\Flysystem\Adapter\Local;
 use League\Flysystem\Adapter\Local;
 use League\Flysystem\Filesystem;
 use League\Flysystem\Filesystem;
@@ -44,7 +45,7 @@ class Utils
             ];
             ];
         }
         }
 
 
-        throw new Exception("Couldn't get class and method names for route ". c::getRouteRepresentation($route).'.');
+        throw new Exception("Couldn't get class and method names for route " . c::getRouteRepresentation($route) . '.');
     }
     }
 
 
     /**
     /**
@@ -101,9 +102,9 @@ class Utils
      *
      *
      * @param array $routeControllerAndMethod
      * @param array $routeControllerAndMethod
      *
      *
+     * @return ReflectionFunctionAbstract
      * @throws ReflectionException
      * @throws ReflectionException
      *
      *
-     * @return ReflectionFunctionAbstract
      */
      */
     public static function getReflectedRouteMethod(array $routeControllerAndMethod): ReflectionFunctionAbstract
     public static function getReflectedRouteMethod(array $routeControllerAndMethod): ReflectionFunctionAbstract
     {
     {
@@ -116,4 +117,13 @@ class Utils
         return (new ReflectionClass($class))->getMethod($method);
         return (new ReflectionClass($class))->getMethod($method);
     }
     }
 
 
+    public static function isArrayType(string $typeName)
+    {
+        return Str::endsWith($typeName, '[]');
+    }
+
+    public static function getBaseTypeFromArrayType(string $typeName)
+    {
+        return substr($typeName, 0, -2);
+    }
 }
 }

+ 60 - 22
src/Writing/OpenAPISpecWriter.php

@@ -4,10 +4,14 @@ namespace Knuckles\Scribe\Writing;
 
 
 use Illuminate\Support\Collection;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
 use Illuminate\Support\Str;
+use Knuckles\Scribe\Extracting\ParamHelpers;
 use Knuckles\Scribe\Tools\DocumentationConfig;
 use Knuckles\Scribe\Tools\DocumentationConfig;
+use Knuckles\Scribe\Tools\Utils;
 
 
 class OpenAPISpecWriter
 class OpenAPISpecWriter
 {
 {
+    use ParamHelpers;
+
     const VERSION = '3.0.3';
     const VERSION = '3.0.3';
 
 
     /**
     /**
@@ -145,9 +149,7 @@ class OpenAPISpecWriter
                     'description' => $details['description'] ?? '',
                     'description' => $details['description'] ?? '',
                     'example' => $details['value'] ?? null,
                     'example' => $details['value'] ?? null,
                     'required' => $details['required'] ?? false,
                     'required' => $details['required'] ?? false,
-                    'schema' => [
-                        'type' => $details['type'] ?? 'string',
-                    ],
+                    'schema' => $this->generateFieldData($details),
                 ];
                 ];
                 $parameters[] = $parameterData;
                 $parameters[] = $parameterData;
             }
             }
@@ -183,7 +185,7 @@ class OpenAPISpecWriter
             $hasRequiredParameter = false;
             $hasRequiredParameter = false;
             $hasFileParameter = false;
             $hasFileParameter = false;
 
 
-            foreach ($endpoint['bodyParameters'] as $name => $details) {
+            foreach ($endpoint['nestedBodyParameters'] as $name => $details) {
                 if ($details['required']) {
                 if ($details['required']) {
                     $hasRequiredParameter = true;
                     $hasRequiredParameter = true;
                     // Don't declare this earlier.
                     // Don't declare this earlier.
@@ -193,26 +195,11 @@ class OpenAPISpecWriter
 
 
 
 
                 if ($details['type'] === 'file') {
                 if ($details['type'] === 'file') {
-                    // See https://swagger.io/docs/specification/describing-request-body/file-upload/
                     $hasFileParameter = true;
                     $hasFileParameter = true;
-                    $fieldData = [
-                        'type' => 'string',
-                        'format' => 'binary',
-                        'description' => $details['description'] ?? '',
-                    ];
-                } else {
-                    $fieldData = [
-                        'type' => $this->convertScribeOrPHPTypeToOpenAPIType($details['type']),
-                        'description' => $details['description'] ?? '',
-                        'example' => $details['value'] ?? null,
-                    ];
-                    if ($fieldData['type'] === 'array') {
-                        $fieldData['items'] = [
-                            'type' => empty($details['value'] ?? null) ? 'object' : $this->convertScribeOrPHPTypeToOpenAPIType(gettype($details['value'][0])),
-                        ];
-                    }
                 }
                 }
 
 
+                $fieldData = $this->generateFieldData($details);
+
                 $schema['properties'][$name] = $fieldData;
                 $schema['properties'][$name] = $fieldData;
             }
             }
 
 
@@ -415,7 +402,7 @@ class OpenAPISpecWriter
                     'description' => '',
                     'description' => '',
                 ];
                 ];
                 break;
                 break;
-                // OpenAPI doesn't support auth with body parameter
+            // OpenAPI doesn't support auth with body parameter
         }
         }
 
 
         return [
         return [
@@ -447,4 +434,55 @@ class OpenAPISpecWriter
                 return $type;
                 return $type;
         }
         }
     }
     }
+
+    public function generateFieldData(array $field): array
+    {
+        if ($field['type'] === 'file') {
+            // See https://swagger.io/docs/specification/describing-request-body/file-upload/
+            return [
+                'type' => 'string',
+                'format' => 'binary',
+                'description' => $field['description'] ?? '',
+            ];
+        } else if (Utils::isArrayType($field['type'])) {
+            $baseType = Utils::getBaseTypeFromArrayType($field['type']);
+            $fieldData = [
+                'type' => 'array',
+                'description' => $field['description'] ?? '',
+                'example' => $field['value'] ?? null,
+                'items' => Utils::isArrayType($baseType)
+                    ? $this->generateFieldData(['name' => '', 'type' => $baseType, 'value' => ($field['value'] ?? [null])[0]])
+                : [
+                'type' => $baseType,
+            ],
+        ];
+
+        if ($baseType === 'object') {
+            foreach ($field['fields'] as $subfield) {
+                $fieldSimpleName = preg_replace("/^{$field['name']}\\[\]\\./", '', $subfield['name']);
+                $fieldData['items']['properties'][$fieldSimpleName] = $this->generateFieldData($subfield);
+                if ($subfield['required']) {
+                    $fieldData['items']['required'][] = $fieldSimpleName;
+                }
+            }
+        }
+
+        return $fieldData;
+    } else if ($field['type'] === 'object') {
+            return [
+                'type' => 'object',
+                'description' => $field['description'] ?? '',
+                'example' => $field['value'] ?? null,
+                'properties' => collect($field['fields'])->mapWithKeys(function ($subfield) use ($field) {
+                    return [preg_replace("/^{$field['name']}\\./", '', $subfield['name']) => $this->generateFieldData($subfield)];
+                })->all(),
+            ];
+        } else {
+            return [
+                'type' => $this->normalizeTypeName($field['type']),
+                'description' => $field['description'] ?? '',
+                'example' => $field['value'] ?? null,
+            ];
+        }
+    }
 }
 }

+ 3 - 0
src/Writing/PostmanCollectionWriter.php

@@ -225,6 +225,9 @@ class PostmanCollectionWriter
             if (Str::endsWith($parameterData['type'], '[]')) {
             if (Str::endsWith($parameterData['type'], '[]')) {
                 $values = empty($parameterData['value']) ? [] : $parameterData['value'];
                 $values = empty($parameterData['value']) ? [] : $parameterData['value'];
                 foreach ($values as $index => $value) {
                 foreach ($values as $index => $value) {
+                    // PHP's parse_str supports array query parameters as filters[0]=name&filters[1]=age OR filters[]=name&filters[]=age
+                    // Going with the first to also support object query parameters
+                    // See https://www.php.net/manual/en/function.parse-str.php
                     $query[] = [
                     $query[] = [
                         'key' => "{$name}[$index]",
                         'key' => "{$name}[$index]",
                         'value' => urlencode($value),
                         'value' => urlencode($value),

+ 101 - 12
tests/Unit/OpenAPISpecWriterTest.php

@@ -4,6 +4,7 @@ namespace Knuckles\Scribe\Tests\Unit;
 
 
 use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts;
 use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts;
 use Faker\Factory;
 use Faker\Factory;
+use Knuckles\Scribe\Extracting\Generator;
 use Knuckles\Scribe\Tools\DocumentationConfig;
 use Knuckles\Scribe\Tools\DocumentationConfig;
 use Knuckles\Scribe\Writing\OpenAPISpecWriter;
 use Knuckles\Scribe\Writing\OpenAPISpecWriter;
 use Orchestra\Testbench\TestCase;
 use Orchestra\Testbench\TestCase;
@@ -116,12 +117,14 @@ class OpenAPISpecWriterTest extends TestCase
             'urlParameters.param' => [
             'urlParameters.param' => [
                 'description' => 'Something',
                 'description' => 'Something',
                 'required' => true,
                 'required' => true,
-                'value' => '56',
+                'value' => 56,
+                'type' => 'integer',
             ],
             ],
             'urlParameters.optionalParam' => [
             'urlParameters.optionalParam' => [
                 'description' => 'Another',
                 'description' => 'Another',
                 'required' => false,
                 'required' => false,
                 'value' => '69',
                 'value' => '69',
+                'type' => 'string',
             ],
             ],
         ]);
         ]);
         $fakeRoute2 = $this->createMockRouteData(['uri' => 'path1', 'methods' => ['POST']]);
         $fakeRoute2 = $this->createMockRouteData(['uri' => 'path1', 'methods' => ['POST']]);
@@ -137,8 +140,8 @@ class OpenAPISpecWriterTest extends TestCase
             'required' => true,
             'required' => true,
             'name' => 'param',
             'name' => 'param',
             'description' => 'Something',
             'description' => 'Something',
-            'example' => '56',
-            'schema' => ['type' => 'string'],
+            'example' => 56,
+            'schema' => ['type' => 'integer'],
         ], $results['paths']['/path1/{param}/{optionalParam}']['parameters'][0]);
         ], $results['paths']['/path1/{param}/{optionalParam}']['parameters'][0]);
         $this->assertEquals([
         $this->assertEquals([
             'in' => 'path',
             'in' => 'path',
@@ -194,6 +197,7 @@ class OpenAPISpecWriterTest extends TestCase
                     'description' => 'A query param',
                     'description' => 'A query param',
                     'required' => false,
                     'required' => false,
                     'value' => 'hahoho',
                     'value' => 'hahoho',
+                    'type' => 'string',
                 ],
                 ],
             ],
             ],
         ]);
         ]);
@@ -212,7 +216,11 @@ class OpenAPISpecWriterTest extends TestCase
             'name' => 'param',
             'name' => 'param',
             'description' => 'A query param',
             'description' => 'A query param',
             'example' => 'hahoho',
             'example' => 'hahoho',
-            'schema' => ['type' => 'string'],
+            'schema' => [
+                'type' => 'string',
+                'description' => 'A query param',
+                'example' => 'hahoho',
+            ],
         ], $results['paths']['/path1']['get']['parameters'][0]);
         ], $results['paths']['/path1']['get']['parameters'][0]);
     }
     }
 
 
@@ -224,44 +232,86 @@ class OpenAPISpecWriterTest extends TestCase
             'uri' => '/path1',
             'uri' => '/path1',
             'bodyParameters' => [
             'bodyParameters' => [
                 'stringParam' => [
                 'stringParam' => [
+                    'name' => 'stringParam',
                     'description' => 'String param',
                     'description' => 'String param',
                     'required' => false,
                     'required' => false,
                     'value' => 'hahoho',
                     'value' => 'hahoho',
                     'type' => 'string',
                     'type' => 'string',
                 ],
                 ],
                 'integerParam' => [
                 'integerParam' => [
+                    'name' => 'integerParam',
                     'description' => 'Integer param',
                     'description' => 'Integer param',
                     'required' => true,
                     'required' => true,
                     'value' => 99,
                     'value' => 99,
                     'type' => 'integer',
                     'type' => 'integer',
                 ],
                 ],
                 'booleanParam' => [
                 'booleanParam' => [
+                    'name' => 'booleanParam',
                     'description' => 'Boolean param',
                     'description' => 'Boolean param',
                     'required' => true,
                     'required' => true,
                     'value' => false,
                     'value' => false,
                     'type' => 'boolean',
                     'type' => 'boolean',
                 ],
                 ],
+                'objectParam' => [
+                    'name' => 'objectParam',
+                    'description' => 'Object param',
+                    'required' => false,
+                    'value' => [],
+                    'type' => 'object',
+                ],
+                'objectParam.field' => [
+                    'name' => 'objectParam.field',
+                    'description' => 'Object param field',
+                    'required' => false,
+                    'value' => 119.0,
+                    'type' => 'number',
+                ],
             ],
             ],
         ]);
         ]);
+        $fakeRoute1['nestedBodyParameters'] = Generator::nestArrayAndObjectFields($fakeRoute1['bodyParameters']);
         $fakeRoute2 = $this->createMockRouteData(['methods' => ['GET'], 'uri' => '/path1']);
         $fakeRoute2 = $this->createMockRouteData(['methods' => ['GET'], 'uri' => '/path1']);
         $fakeRoute3 = $this->createMockRouteData([
         $fakeRoute3 = $this->createMockRouteData([
             'methods' => ['PUT'],
             'methods' => ['PUT'],
             'uri' => '/path2',
             'uri' => '/path2',
             'bodyParameters' => [
             'bodyParameters' => [
                 'fileParam' => [
                 'fileParam' => [
+                    'name' => 'fileParam',
                     'description' => 'File param',
                     'description' => 'File param',
                     'required' => false,
                     'required' => false,
                     'value' => null,
                     'value' => null,
                     'type' => 'file',
                     'type' => 'file',
                 ],
                 ],
-                'numberParam' => [
-                    'description' => 'Number param',
+                'numberArrayParam' => [
+                    'name' => 'numberArrayParam',
+                    'description' => 'Number array param',
                     'required' => false,
                     'required' => false,
-                    'value' => 186.9,
-                    'type' => 'number',
+                    'value' => [186.9],
+                    'type' => 'number[]',
+                ],
+                'objectArrayParam' => [
+                    'name' => 'objectArrayParam',
+                    'description' => 'Object array param',
+                    'required' => false,
+                    'value' => [[]],
+                    'type' => 'object[]',
+                ],
+                'objectArrayParam[].field1' => [
+                    'name' => 'objectArrayParam[].field1',
+                    'description' => 'Object array param first field',
+                    'required' => true,
+                    'value' => ["hello"],
+                    'type' => 'string[]',
+                ],
+                'objectArrayParam[].field2' => [
+                    'name' => 'objectArrayParam[].field2',
+                    'description' => '',
+                    'required' => false,
+                    'value' => "hi",
+                    'type' => 'string',
                 ],
                 ],
             ],
             ],
         ]);
         ]);
+        $fakeRoute3['nestedBodyParameters'] = Generator::nestArrayAndObjectFields($fakeRoute3['bodyParameters']);
         $groupedEndpoints = collect([$fakeRoute1, $fakeRoute2, $fakeRoute3])->groupBy('metadata.groupName');
         $groupedEndpoints = collect([$fakeRoute1, $fakeRoute2, $fakeRoute3])->groupBy('metadata.groupName');
 
 
         $writer = new OpenAPISpecWriter(new DocumentationConfig($this->config));
         $writer = new OpenAPISpecWriter(new DocumentationConfig($this->config));
@@ -291,6 +341,18 @@ class OpenAPISpecWriterTest extends TestCase
                                 'example' => 99,
                                 'example' => 99,
                                 'type' => 'integer',
                                 'type' => 'integer',
                             ],
                             ],
+                            'objectParam' => [
+                                'description' => 'Object param',
+                                'example' => [],
+                                'type' => 'object',
+                                'properties' => [
+                                    'field' => [
+                                        'description' => 'Object param field',
+                                        'example' => 119.0,
+                                        'type' => 'number',
+                                    ],
+                                ],
+                            ],
                         ],
                         ],
                         'required' => [
                         'required' => [
                             'integerParam',
                             'integerParam',
@@ -312,10 +374,37 @@ class OpenAPISpecWriterTest extends TestCase
                                 'type' => 'string',
                                 'type' => 'string',
                                 'format' => 'binary',
                                 'format' => 'binary',
                             ],
                             ],
-                            'numberParam' => [
-                                'description' => 'Number param',
-                                'example' => 186.9,
-                                'type' => 'number',
+                            'numberArrayParam' => [
+                                'description' => 'Number array param',
+                                'example' => [186.9],
+                                'type' => 'array',
+                                'items' => [
+                                    'type' => 'number',
+                                ],
+                            ],
+                            'objectArrayParam' => [
+                                'description' => 'Object array param',
+                                'example' => [[]],
+                                'type' => 'array',
+                                'items' => [
+                                    'type' => 'object',
+                                    'required' => ['field1'],
+                                    'properties' => [
+                                        'field1' => [
+                                            'type' => 'array',
+                                            'items' => [
+                                                'type' => 'string'
+                                            ],
+                                            'description' => 'Object array param first field',
+                                            'example' => ["hello"],
+                                        ],
+                                        'field2' => [
+                                            'type' => 'string',
+                                            'description' => '',
+                                            'example' => "hi",
+                                        ],
+                                    ],
+                                ],
                             ],
                             ],
                         ],
                         ],
                     ],
                     ],