Correctly handle a case when both form request and inline validation …

…used (#552) * added params extraction splitting * fixed properties being skipped * removed unused dto * Fix styling --------- Co-authored-by: romalytvynenko <[email protected]>
dedoc · Sep 24, 2024 · e44fff1 · e44fff1
1 parent 24dd581
commit e44fff1
Show file tree

Hide file tree

Showing 10 changed files with 394 additions and 155 deletions.
diff --git a/src/Support/OperationExtensions/RequestBodyExtension.php b/src/Support/OperationExtensions/RequestBodyExtension.php
@@ -3,19 +3,22 @@
 namespace Dedoc\Scramble\Support\OperationExtensions;
 
 use Dedoc\Scramble\Extensions\OperationExtension;
+use Dedoc\Scramble\Support\Generator\Combined\AllOf;
 use Dedoc\Scramble\Support\Generator\Operation;
 use Dedoc\Scramble\Support\Generator\Parameter;
 use Dedoc\Scramble\Support\Generator\Reference;
 use Dedoc\Scramble\Support\Generator\RequestBodyObject;
 use Dedoc\Scramble\Support\Generator\Schema;
+use Dedoc\Scramble\Support\Generator\Types\Type;
 use Dedoc\Scramble\Support\OperationExtensions\RulesExtractor\FormRequestRulesExtractor;
-use Dedoc\Scramble\Support\OperationExtensions\RulesExtractor\RulesToParameters;
+use Dedoc\Scramble\Support\OperationExtensions\RulesExtractor\ParametersExtractionResult;
+use Dedoc\Scramble\Support\OperationExtensions\RulesExtractor\RequestMethodCallsExtractor;
 use Dedoc\Scramble\Support\OperationExtensions\RulesExtractor\ValidateCallExtractor;
 use Dedoc\Scramble\Support\RouteInfo;
 use Illuminate\Routing\Route;
 use Illuminate\Support\Arr;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
-use PhpParser\Node\Stmt\ClassMethod;
 use Throwable;
 
 class RequestBodyExtension extends OperationExtension
@@ -32,9 +35,10 @@ public function handle(Operation $operation, RouteInfo $routeInfo)
          */
         $routeInfo->getMethodType();
 
-        [$bodyParams, $schemaName, $schemaDescription] = [[], null, null];
+        $rulesResults = collect();
+
         try {
-            [$bodyParams, $schemaName, $schemaDescription] = $this->extractParamsFromRequestValidationRules($routeInfo->route, $routeInfo->methodNode());
+            $rulesResults = collect($this->extractRouteRequestValidationRules($routeInfo, $routeInfo->methodNode()));
         } catch (Throwable $exception) {
             if (app()->environment('testing')) {
                 throw $exception;
@@ -46,21 +50,11 @@ public function handle(Operation $operation, RouteInfo $routeInfo)
             ->summary(Str::of($routeInfo->phpDoc()->getAttribute('summary'))->rtrim('.'))
             ->description($description);
 
-        $bodyParamsNames = array_map(fn ($p) => $p->name, $bodyParams);
+        $allParams = $rulesResults->flatMap->parameters->unique('name')->values()->all();
 
-        $allParams = [
-            ...$bodyParams,
-            ...array_filter(
-                array_values($routeInfo->requestParametersFromCalls->data),
-                fn ($p) => ! in_array($p->name, $bodyParamsNames),
-            ),
-        ];
         [$queryParams, $bodyParams] = collect($allParams)
-            ->partition(function (Parameter $parameter) {
-                return $parameter->getAttribute('isInQuery');
-            });
-        $queryParams = $queryParams->toArray();
-        $bodyParams = $bodyParams->toArray();
+            ->partition(fn (Parameter $p) => $p->getAttribute('isInQuery'))
+            ->map->toArray();
 
         $mediaType = $this->getMediaType($operation, $routeInfo, $allParams);
 
@@ -75,41 +69,66 @@ public function handle(Operation $operation, RouteInfo $routeInfo)
             return;
         }
 
-        $this->addRequestBody(
-            $operation,
-            $mediaType,
-            $bodyParams,
-            $schemaName,
-            $schemaDescription,
-        );
-    }
+        [$schemaResults, $schemalessResults] = $rulesResults->partition('schemaName');
+        $schemalessResults = collect([$this->mergeSchemalessRulesResults($schemalessResults->values())]);
 
-    protected function addRequestBody(Operation $operation, string $mediaType, array $bodyParams, ?string $schemaName, ?string $schemaDescription)
-    {
-        if (empty($bodyParams)) {
+        $schemas = $schemaResults->merge($schemalessResults)
+            ->filter(fn (ParametersExtractionResult $r) => count($r->parameters) || $r->schemaName)
+            ->map(function (ParametersExtractionResult $r) use ($queryParams) {
+                $qpNames = collect($queryParams)->keyBy('name');
+
+                $r->parameters = collect($r->parameters)->filter(fn ($p) => ! $qpNames->has($p->name))->values()->all();
+
+                return $r;
+            })
+            ->values()
+            ->map($this->makeSchemaFromResults(...));
+
+        if ($schemas->isEmpty()) {
             return;
         }
 
-        $requestBodySchema = Schema::createFromParameters($bodyParams);
+        $schema = $this->makeComposedRequestBodySchema($schemas);
+        if (! $schema instanceof Reference) {
+            $schema = Schema::fromType($schema);
+        }
+
+        $operation->addRequestBodyObject(
+            RequestBodyObject::make()->setContent($mediaType, $schema),
+        );
+    }
 
-        if (! $schemaName) {
-            $operation->addRequestBodyObject(RequestBodyObject::make()->setContent($mediaType, $requestBodySchema));
+    protected function makeSchemaFromResults(ParametersExtractionResult $result): Type
+    {
+        $requestBodySchema = Schema::createFromParameters($result->parameters);
 
-            return;
+        if (! $result->schemaName) {
+            return $requestBodySchema->type;
         }
 
         $components = $this->openApiTransformer->getComponents();
-        if (! $components->hasSchema($schemaName)) {
-            $requestBodySchema->type->setDescription($schemaDescription ?: '');
+        if (! $components->hasSchema($result->schemaName)) {
+            $requestBodySchema->type->setDescription($result->description ?: '');
 
-            $components->addSchema($schemaName, $requestBodySchema);
+            $components->addSchema($result->schemaName, $requestBodySchema);
         }
 
-        $operation->addRequestBodyObject(
-            RequestBodyObject::make()->setContent(
-                $mediaType,
-                new Reference('schemas', $schemaName, $components),
-            )
+        return new Reference('schemas', $result->schemaName, $components);
+    }
+
+    protected function makeComposedRequestBodySchema(Collection $schemas)
+    {
+        if ($schemas->count() === 1) {
+            return $schemas->first();
+        }
+
+        return (new AllOf)->setItems($schemas->all());
+    }
+
+    protected function mergeSchemalessRulesResults(Collection $schemalessResults): ParametersExtractionResult
+    {
+        return new ParametersExtractionResult(
+            parameters: $schemalessResults->values()->flatMap->parameters->unique('name')->values()->all(),
         );
     }
 
@@ -141,37 +160,50 @@ protected function hasBinary($bodyParams): bool
         });
     }
 
-    protected function extractParamsFromRequestValidationRules(Route $route, ?ClassMethod $methodNode)
+    protected function extractRouteRequestValidationRules(RouteInfo $routeInfo, $methodNode)
     {
-        [$rules, $nodesResults] = $this->extractRouteRequestValidationRules($route, $methodNode);
-
-        return [
-            (new RulesToParameters($rules, $nodesResults, $this->openApiTransformer))->handle(),
-            $nodesResults[0]->schemaName ?? null,
-            $nodesResults[0]->description ?? null,
+        /*
+         * These are the extractors that are getting types from the validation rules, so it is
+         * certain that a property must have the extracted type.
+         */
+        $typeDefiningHandlers = [
+            new FormRequestRulesExtractor($methodNode, $this->openApiTransformer),
+            new ValidateCallExtractor($methodNode, $this->openApiTransformer),
         ];
+
+        $validationRulesExtractedResults = collect($typeDefiningHandlers)
+            ->filter(fn ($h) => $h->shouldHandle())
+            ->map(fn ($h) => $h->extract($routeInfo))
+            ->values()
+            ->toArray();
+
+        /*
+         * This is the extractor that cannot re-define the incoming type but can add new properties.
+         * Also, it is useful for additional details.
+         */
+        $detailsExtractor = new RequestMethodCallsExtractor;
+
+        $methodCallsExtractedResults = $detailsExtractor->extract($routeInfo);
+
+        return $this->mergeExtractedProperties($validationRulesExtractedResults, $methodCallsExtractedResults);
     }
 
-    protected function extractRouteRequestValidationRules(Route $route, $methodNode)
+    /**
+     * @param  ParametersExtractionResult[]  $rulesExtractedResults
+     */
+    protected function mergeExtractedProperties(array $rulesExtractedResults, ParametersExtractionResult $methodCallsExtractedResult)
     {
-        $rules = [];
-        $nodesResults = [];
-
-        // Custom form request's class `validate` method
-        if (($formRequestRulesExtractor = new FormRequestRulesExtractor($methodNode))->shouldHandle()) {
-            if (count($formRequestRules = $formRequestRulesExtractor->extract($route))) {
-                $rules = array_merge($rules, $formRequestRules);
-                $nodesResults[] = $formRequestRulesExtractor->node();
-            }
-        }
+        $rulesParameters = collect($rulesExtractedResults)->flatMap->parameters->keyBy('name');
 
-        if (($validateCallExtractor = new ValidateCallExtractor($methodNode))->shouldHandle()) {
-            if ($validateCallRules = $validateCallExtractor->extract()) {
-                $rules = array_merge($rules, $validateCallRules);
-                $nodesResults[] = $validateCallExtractor->node();
-            }
-        }
+        $methodCallsExtractedResult->parameters = collect($methodCallsExtractedResult->parameters)
+            ->filter(fn (Parameter $p) => ! $rulesParameters->has($p->name))
+            ->values()
+            ->all();
+
+        /*
+         * Possible improvements here: using defaults when merging results, etc.
+         */
 
-        return [$rules, array_filter($nodesResults)];
+        return [...$rulesExtractedResults, $methodCallsExtractedResult];
     }
 }
diff --git a/src/Support/OperationExtensions/RulesExtractor/FormRequestRulesExtractor.php b/src/Support/OperationExtensions/RulesExtractor/FormRequestRulesExtractor.php
@@ -3,6 +3,8 @@
 namespace Dedoc\Scramble\Support\OperationExtensions\RulesExtractor;
 
 use Dedoc\Scramble\Infer;
+use Dedoc\Scramble\Support\Generator\TypeTransformer;
+use Dedoc\Scramble\Support\RouteInfo;
 use Dedoc\Scramble\Support\SchemaClassDocReflector;
 use Illuminate\Http\Request;
 use Illuminate\Routing\Route;
@@ -14,16 +16,13 @@
 use ReflectionClass;
 use Spatie\LaravelData\Contracts\BaseData;
 
-class FormRequestRulesExtractor
+class FormRequestRulesExtractor implements RulesExtractor
 {
-    private ?FunctionLike $handler;
+    use GeneratesParametersFromRules;
 
-    public function __construct(?FunctionLike $handler)
-    {
-        $this->handler = $handler;
-    }
+    public function __construct(private ?FunctionLike $handler, private TypeTransformer $typeTransformer) {}
 
-    public function shouldHandle()
+    public function shouldHandle(): bool
     {
         if (! $this->handler) {
             return false;
@@ -42,7 +41,7 @@ public function shouldHandle()
         return true;
     }
 
-    public function node()
+    public function extract(RouteInfo $routeInfo): ParametersExtractionResult
     {
         $requestClassName = $this->getFormRequestClassName();
 
@@ -54,19 +53,23 @@ public function node()
             ? null
             : $phpDocReflector->getSchemaName($requestClassName);
 
-        return new ValidationNodesResult(
-            (new NodeFinder)->find(
-                Arr::wrap($classReflector->getMethod('rules')->getAstNode()->stmts),
-                fn (Node $node) => $node instanceof Node\Expr\ArrayItem
-                    && $node->key instanceof Node\Scalar\String_
-                    && $node->getAttribute('parsedPhpDoc'),
+        return new ParametersExtractionResult(
+            parameters: $this->makeParameters(
+                node: (new NodeFinder)->find(
+                    Arr::wrap($classReflector->getMethod('rules')->getAstNode()->stmts),
+                    fn (Node $node) => $node instanceof Node\Expr\ArrayItem
+                        && $node->key instanceof Node\Scalar\String_
+                        && $node->getAttribute('parsedPhpDoc'),
+                ),
+                rules: $this->rules($routeInfo->route),
+                typeTransformer: $this->typeTransformer,
             ),
             schemaName: $schemaName,
             description: $phpDocReflector->getDescription(),
         );
     }
 
-    public function extract(Route $route)
+    protected function rules(Route $route)
     {
         $requestClassName = $this->getFormRequestClassName();
 

diff --git a/src/Support/OperationExtensions/RulesExtractor/GeneratesParametersFromRules.php b/src/Support/OperationExtensions/RulesExtractor/GeneratesParametersFromRules.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace Dedoc\Scramble\Support\OperationExtensions\RulesExtractor;
+
+use Dedoc\Scramble\Support\Generator\TypeTransformer;
+
+trait GeneratesParametersFromRules
+{
+    private function makeParameters($node, $rules, TypeTransformer $typeTransformer)
+    {
+        return (new RulesToParameters($rules, $node, $typeTransformer))->handle();
+    }
+}
diff --git a/.../RulesExtractor/ValidationNodesResult.php → ...sExtractor/ParametersExtractionResult.php b/.../RulesExtractor/ValidationNodesResult.php → ...sExtractor/ParametersExtractionResult.php
@@ -2,10 +2,13 @@
 
 namespace Dedoc\Scramble\Support\OperationExtensions\RulesExtractor;
 
-class ValidationNodesResult
+/**
+ * @internal
+ */
+class ParametersExtractionResult
 {
     public function __construct(
-        public $node,
+        public array $parameters,
         public ?string $schemaName = null,
         public ?string $description = null,
     ) {}

diff --git a/src/Support/OperationExtensions/RulesExtractor/RequestMethodCallsExtractor.php b/src/Support/OperationExtensions/RulesExtractor/RequestMethodCallsExtractor.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Dedoc\Scramble\Support\OperationExtensions\RulesExtractor;
+
+use Dedoc\Scramble\Support\RouteInfo;
+
+class RequestMethodCallsExtractor implements RulesExtractor
+{
+    public function shouldHandle(): bool
+    {
+        return true;
+    }
+
+    public function extract(RouteInfo $routeInfo): ParametersExtractionResult
+    {
+        return new ParametersExtractionResult(
+            parameters: array_values($routeInfo->requestParametersFromCalls->data),
+        );
+    }
+}
diff --git a/src/Support/OperationExtensions/RulesExtractor/RulesExtractor.php b/src/Support/OperationExtensions/RulesExtractor/RulesExtractor.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Dedoc\Scramble\Support\OperationExtensions\RulesExtractor;
+
+use Dedoc\Scramble\Support\RouteInfo;
+
+interface RulesExtractor
+{
+    public function shouldHandle(): bool;
+
+    public function extract(RouteInfo $routeInfo): ParametersExtractionResult;
+}