Browse Source

Auto-migrate old route rules to new settings

Shalvah 1 year ago
parent
commit
efd6f6ce18

+ 1 - 3
src/Config/Extracting.php

@@ -2,8 +2,6 @@
 
 namespace Knuckles\Scribe\Config;
 
-use Illuminate\Support\Arr;
-
 class Extracting
 {
     public function __construct(
@@ -43,7 +41,7 @@ class Extracting
         StrategyListWrapper $responseFields,
     ): array
     {
-        return Arr::map(get_defined_vars(), fn($listWrapper) => $listWrapper->toArray());
+        return array_map(fn($listWrapper) => $listWrapper->toArray(), get_defined_vars());
     }
 
     public static function with(

+ 2 - 2
src/Config/Serializer.php

@@ -61,8 +61,8 @@ class Serializer
 
     protected static function translateKeys($array)
     {
-        return Arr::mapWithKeys($array, function ($value, $key) {
+        return collect($array)->mapWithKeys(function ($value, $key) {
             return [Str::snake($key) => $value];
-        });
+        })->toArray();
     }
 }

+ 2 - 3
src/Config/StrategyListWrapper.php

@@ -3,7 +3,6 @@
 namespace Knuckles\Scribe\Config;
 
 use Illuminate\Support\Arr;
-use Knuckles\Scribe\Extracting\Strategies\StrategyInterface;
 
 /**
  * @internal
@@ -49,10 +48,10 @@ class StrategyListWrapper
 
     public function configure(array $configurationTuple): self
     {
-        $this->strategies = Arr::map($this->strategies, function ($strategy) use ($configurationTuple) {
+        $this->strategies = array_map(function ($strategy) use ($configurationTuple) {
             $strategyName = is_string($strategy) ? $strategy : $strategy[0];
             return $strategyName == $configurationTuple[0] ? $configurationTuple : $strategy;
-        });
+        }, $this->strategies);
 
         return $this;
     }

+ 35 - 2
src/Extracting/Extractor.php

@@ -214,8 +214,9 @@ class Extractor
                     continue;
                 }
 
-                $routesToSkip = $settings['except'] ?? [];
-                $routesToInclude = $settings['only'] ?? [];
+                $settings = self::transformOldRouteRulesIntoNewSettings($stage, $rulesToApply, $strategyClass, $settings);
+                $routesToSkip = $settings["except"] ?? [];
+                $routesToInclude = $settings["only"] ?? [];
 
                 if (!empty($routesToSkip)) {
                     if (RoutePatternMatcher::matches($endpointData->route, $routesToSkip)) {
@@ -466,4 +467,36 @@ class Extractor
             };
         }
     }
+
+    public static function transformOldRouteRulesIntoNewSettings($stage, $rulesToApply, $strategyName, $strategySettings = [])
+    {
+        if ($stage == 'headers' && Str::is('*RouteRules*', $strategyName)) {
+            return $rulesToApply;
+        }
+
+        if ($stage == 'responses' && Str::is('*ResponseCalls*', $strategyName) &&!empty($rulesToApply['response_calls'])) {
+            // Transform `methods` to `only`
+            $strategySettings = [];
+
+            if (isset($rulesToApply['response_calls']['methods'])) {
+                if(empty($rulesToApply['response_calls']['methods'])) {
+                    // This means all routes are forbidden
+                    $strategySettings['except'] = ["*"];
+                } else {
+                    $strategySettings['only'] = array_map(
+                        fn($method) => "$method *",
+                        $rulesToApply['response_calls']['methods']
+                    );
+                }
+            }
+
+            // Copy all other keys over as is
+            foreach (array_keys($rulesToApply['response_calls']) as $key) {
+                if ($key == 'methods') continue;
+                $strategySettings[$key] = $rulesToApply['response_calls'][$key];
+            }
+        }
+
+        return $strategySettings;
+    }
 }

+ 19 - 47
src/Extracting/Strategies/Responses/ResponseCalls.php

@@ -30,28 +30,27 @@ class ResponseCalls extends Strategy
 
     protected array $previousConfigs = [];
 
-    public function __invoke(ExtractedEndpointData $endpointData, array $routeRules = []): ?array
+    public function __invoke(ExtractedEndpointData $endpointData, array $settings = []): ?array
     {
-        return $this->makeResponseCallIfConditionsPass($endpointData, $routeRules);
+        return $this->makeResponseCallIfConditionsPass($endpointData, $settings);
     }
 
-    public function makeResponseCallIfConditionsPass(ExtractedEndpointData $endpointData, array $routeRules): ?array
+    public function makeResponseCallIfConditionsPass(ExtractedEndpointData $endpointData, array $settings): ?array
     {
-        $rulesToApply = $routeRules['response_calls'] ?? $routeRules;
-        if (!$this->shouldMakeApiCall($endpointData, $rulesToApply)) {
+        if (!$this->shouldMakeApiCall($endpointData)) {
             return null;
         }
 
-        return $this->makeResponseCall($endpointData, $rulesToApply);
+        return $this->makeResponseCall($endpointData, $settings);
     }
 
-    public function makeResponseCall(ExtractedEndpointData $endpointData, array $rulesToApply): ?array
+    public function makeResponseCall(ExtractedEndpointData $endpointData, array $settings): ?array
     {
-        $this->configureEnvironment($rulesToApply);
+        $this->configureEnvironment($settings);
 
         // Mix in parsed parameters with manually specified parameters.
-        $bodyParameters = array_merge($endpointData->cleanBodyParameters, $rulesToApply['bodyParams'] ?? []);
-        $queryParameters = array_merge($endpointData->cleanQueryParameters, $rulesToApply['queryParams'] ?? []);
+        $bodyParameters = array_merge($endpointData->cleanBodyParameters, $settings['bodyParams'] ?? []);
+        $queryParameters = array_merge($endpointData->cleanQueryParameters, $settings['queryParams'] ?? []);
         $urlParameters = $endpointData->cleanUrlParameters;
         $headers = $endpointData->headers;
 
@@ -72,7 +71,7 @@ class ResponseCalls extends Strategy
             }
         }
 
-        $hardcodedFileParams = $rulesToApply['fileParams'] ?? [];
+        $hardcodedFileParams = $settings['fileParams'] ?? [];
         $hardcodedFileParams = collect($hardcodedFileParams)->map(function ($filePath) {
             $fileName = basename($filePath);
             return new UploadedFile(
@@ -82,7 +81,7 @@ class ResponseCalls extends Strategy
         $fileParameters = array_merge($endpointData->fileParameters, $hardcodedFileParams);
 
         $request = $this->prepareRequest(
-            $endpointData->route, $endpointData->uri, $rulesToApply, $urlParameters,
+            $endpointData->route, $endpointData->uri, $settings, $urlParameters,
             $bodyParameters, $queryParameters, $fileParameters, $headers
         );
 
@@ -110,19 +109,19 @@ class ResponseCalls extends Strategy
     }
 
     /**
-     * @param array $rulesToApply
+     * @param array $settings
      *
      * @return void
      */
-    private function configureEnvironment(array $rulesToApply)
+    private function configureEnvironment(array $settings)
     {
         $this->startDbTransaction();
-        $this->setLaravelConfigs($rulesToApply['config'] ?? []);
+        $this->setLaravelConfigs($settings['config'] ?? []);
     }
 
     /**
      * @param Route $route
-     * @param array $rulesToApply
+     * @param array $settings
      * @param array $urlParams
      * @param array $bodyParams
      * @param array $queryParams
@@ -133,14 +132,14 @@ class ResponseCalls extends Strategy
      * @return Request
      */
     protected function prepareRequest(
-        Route $route, string $url, array $rulesToApply, array $urlParams,
+        Route $route, string $url, array $settings, array $urlParams,
         array $bodyParams, array $queryParams, array $fileParameters, array $headers
     ): Request
     {
         $uri = Utils::getUrlWithBoundParameters($url, $urlParams);
         $routeMethods = $this->getMethods($route);
         $method = array_shift($routeMethods);
-        $cookies = $rulesToApply['cookies'] ?? [];
+        $cookies = $settings['cookies'] ?? [];
 
         // Note that we initialise the request with the bodyParams here
         // and later still add them to the ParameterBag (`addBodyParameters`)
@@ -313,41 +312,14 @@ class ResponseCalls extends Strategy
         return $response;
     }
 
-    protected function shouldMakeApiCall(ExtractedEndpointData $endpointData, array $rulesToApply): bool
+    protected function shouldMakeApiCall(ExtractedEndpointData $endpointData): bool
     {
         // Don't attempt a response call if there are already successful responses
         if ($endpointData->responses->hasSuccessResponse()) {
             return false;
         }
 
-        if (array_key_exists('only', $rulesToApply) || array_key_exists('except', $rulesToApply)) {
-            // We're in the v2 config. The route filtering has already been done
-            return true;
-        }
-
-        /**
-         * @deprecated The rest of this method is now determined by the strategy-level settings (`only` and `exclude` keys),
-         *   which are supported globally by all strategies.
-         */
-        $allowedMethods = $rulesToApply['methods'] ?? [];
-        if (empty($allowedMethods)) {
-            return false;
-        }
-
-        if (is_string($allowedMethods) && $allowedMethods == '*') {
-            return true;
-        }
-
-        if (array_search('*', $allowedMethods) !== false) {
-            return true;
-        }
-
-        $routeMethods = $this->getMethods($endpointData->route);
-        if (in_array(array_shift($routeMethods), $allowedMethods)) {
-            return true;
-        }
-
-        return false;
+        return true;
     }
 
     /**

+ 0 - 1
tests/GenerateDocumentation/OutputTest.php

@@ -229,7 +229,6 @@ class OutputTest extends BaseLaravelTest
 
         $generatedSpec = Yaml::parseFile($this->openapiOutputPath());
         $fixtureSpec = Yaml::parseFile(__DIR__ . '/../Fixtures/openapi.yaml');
-        ray($fixtureSpec);
         $this->assertEquals($fixtureSpec, $generatedSpec);
     }
 

+ 16 - 41
tests/Strategies/Responses/ResponseCallsTest.php

@@ -13,14 +13,11 @@ use Knuckles\Scribe\Scribe;
 use Knuckles\Scribe\Tests\BaseLaravelTest;
 use Knuckles\Scribe\Tests\Fixtures\TestController;
 use Knuckles\Scribe\Tools\DocumentationConfig;
-use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts;
 use Illuminate\Support\Facades\Route as LaravelRouteFacade;
 use Symfony\Component\HttpFoundation\Request;
 
 class ResponseCallsTest extends BaseLaravelTest
 {
-    use ArraySubsetAsserts;
-
     protected function setUp(): void
     {
         parent::setUp();
@@ -33,17 +30,13 @@ class ResponseCallsTest extends BaseLaravelTest
         $route = LaravelRouteFacade::post('/shouldFetchRouteResponse', [TestController::class, 'shouldFetchRouteResponse']);
 
         $rules = [
-            'headers' => [
-                'Content-Type' => 'application/json',
-                'Accept' => 'application/json',
-            ],
             'response_calls' => [
                 'methods' => ['*'],
             ],
         ];
 
         $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
 
         $this->assertEquals(200, $results[0]['status']);
         $this->assertArraySubset([
@@ -99,7 +92,8 @@ class ResponseCallsTest extends BaseLaravelTest
         ]);
 
         $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy($endpointData, $rules);
+        $results = $strategy($endpointData,
+            $this->convertRules($rules));
 
         $this->assertEquals(200, $results[0]['status']);
 
@@ -122,7 +116,7 @@ class ResponseCallsTest extends BaseLaravelTest
         ];
 
         $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
         $originalValue = json_decode($results[0]['content'], true)['app.env'];
 
         $now = time();
@@ -135,7 +129,7 @@ class ResponseCallsTest extends BaseLaravelTest
             ],
         ];
 
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
         $newValue = json_decode($results[0]['content'], true)['app.env'];
         $this->assertEquals($now, $newValue);
         $this->assertNotEquals($originalValue, $newValue);
@@ -175,7 +169,7 @@ class ResponseCallsTest extends BaseLaravelTest
         ]);
 
         $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy($endpointData, $rules);
+        $results = $strategy($endpointData, $this->convertRules($rules));
 
         $this->assertEquals(200, $results[0]['status']);
 
@@ -197,17 +191,13 @@ class ResponseCallsTest extends BaseLaravelTest
         $route = $this->registerDingoRoute('post', '/shouldFetchRouteResponse', 'shouldFetchRouteResponse');
 
         $rules = [
-            'headers' => [
-                'Content-Type' => 'application/json',
-                'Accept' => 'application/json',
-            ],
             'response_calls' => [
                 'methods' => ['*'],
             ],
         ];
 
         $strategy = new ResponseCalls(new DocumentationConfig());
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
 
         $this->assertEquals(200, $results[0]['status']);
         $this->assertArraySubset([
@@ -247,7 +237,7 @@ class ResponseCallsTest extends BaseLaravelTest
             ],
         ]);
         $strategy = new ResponseCalls(new DocumentationConfig());
-        $results = $strategy($endpointData, $rules);
+        $results = $strategy($endpointData, $this->convertRules($rules));
 
         $this->assertEquals(200, $results[0]['status']);
 
@@ -272,7 +262,7 @@ class ResponseCallsTest extends BaseLaravelTest
         ];
 
         $strategy = new ResponseCalls(new DocumentationConfig());
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
         $originalValue = json_decode($results[0]['content'], true)['app.env'];
 
         $now = time();
@@ -285,7 +275,7 @@ class ResponseCallsTest extends BaseLaravelTest
             ],
         ];
 
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy(ExtractedEndpointData::fromRoute($route), $this->convertRules($rules));
         $newValue = json_decode($results[0]['content'], true)['app.env'];
         $this->assertEquals($now, $newValue);
         $this->assertNotEquals($originalValue, $newValue);
@@ -297,10 +287,6 @@ class ResponseCallsTest extends BaseLaravelTest
         $route = LaravelRouteFacade::post('/shouldFetchRouteResponse', [TestController::class, 'shouldFetchRouteResponse']);
 
         $rules = [
-            'headers' => [
-                'Content-Type' => 'application/json',
-                'Accept' => 'application/json',
-            ],
             'response_calls' => [
                 'methods' => ['*'],
             ],
@@ -315,23 +301,7 @@ class ResponseCallsTest extends BaseLaravelTest
             ]),
         ]);
         $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy($endpointData, $rules);
-
-        $this->assertNull($results);
-    }
-
-    /** @test */
-    public function does_not_make_response_call_if_forbidden_by_config()
-    {
-        $route = LaravelRouteFacade::post('/shouldFetchRouteResponse', [TestController::class, 'shouldFetchRouteResponse']);
-
-        $rules = [
-            'response_calls' => [
-                'methods' => [],
-            ],
-        ];
-        $strategy = new ResponseCalls(new DocumentationConfig([]));
-        $results = $strategy(ExtractedEndpointData::fromRoute($route), $rules);
+        $results = $strategy($endpointData, $this->convertRules($rules));
 
         $this->assertNull($results);
     }
@@ -354,4 +324,9 @@ class ResponseCallsTest extends BaseLaravelTest
                 return $route->uri() === $desiredRoute->uri();
             });
     }
+
+    protected function convertRules(array $rules): mixed
+    {
+        return Extractor::transformOldRouteRulesIntoNewSettings('responses', $rules, ResponseCalls::class);
+    }
 }

+ 11 - 11
tests/Unit/ExtractorTest.php

@@ -6,6 +6,7 @@ use Illuminate\Routing\Route;
 use Knuckles\Camel\Extraction\ExtractedEndpointData;
 use Knuckles\Camel\Extraction\Parameter;
 use Knuckles\Scribe\Extracting\Extractor;
+use Knuckles\Scribe\Extracting\Strategies;
 use Knuckles\Scribe\Tests\BaseUnitTest;
 use Knuckles\Scribe\Tests\Fixtures\TestController;
 use Knuckles\Scribe\Tools\DocumentationConfig;
@@ -17,30 +18,29 @@ class ExtractorTest extends BaseUnitTest
     protected $config = [
         'strategies' => [
             'metadata' => [
-                \Knuckles\Scribe\Extracting\Strategies\Metadata\GetFromDocBlocks::class,
+                Strategies\Metadata\GetFromDocBlocks::class,
                 \Knuckles\Scribe\Tests\Fixtures\TestCustomEndpointMetadata::class,
             ],
             'urlParameters' => [
-                \Knuckles\Scribe\Extracting\Strategies\UrlParameters\GetFromLaravelAPI::class,
-                \Knuckles\Scribe\Extracting\Strategies\UrlParameters\GetFromUrlParamTag::class,
+                Strategies\UrlParameters\GetFromLaravelAPI::class,
+                Strategies\UrlParameters\GetFromUrlParamTag::class,
             ],
             'queryParameters' => [
-                \Knuckles\Scribe\Extracting\Strategies\QueryParameters\GetFromQueryParamTag::class,
+                Strategies\QueryParameters\GetFromQueryParamTag::class,
             ],
             'headers' => [
-                \Knuckles\Scribe\Extracting\Strategies\Headers\GetFromRouteRules::class,
-                \Knuckles\Scribe\Extracting\Strategies\Headers\GetFromHeaderTag::class,
+                Strategies\Headers\GetFromRouteRules::class,
+                Strategies\Headers\GetFromHeaderTag::class,
             ],
             'bodyParameters' => [
-                \Knuckles\Scribe\Extracting\Strategies\BodyParameters\GetFromBodyParamTag::class,
+                Strategies\BodyParameters\GetFromBodyParamTag::class,
             ],
             'responses' => [
-                \Knuckles\Scribe\Extracting\Strategies\Responses\UseResponseTag::class,
-                \Knuckles\Scribe\Extracting\Strategies\Responses\UseResponseFileTag::class,
-                \Knuckles\Scribe\Extracting\Strategies\Responses\ResponseCalls::class,
+                Strategies\Responses\UseResponseTag::class,
+                Strategies\Responses\UseResponseFileTag::class,
             ],
             'responseFields' => [
-                \Knuckles\Scribe\Extracting\Strategies\ResponseFields\GetFromResponseFieldTag::class,
+                Strategies\ResponseFields\GetFromResponseFieldTag::class,
             ],
         ],
     ];

+ 3 - 0
tests/Unit/RoutePatternMatcherTest.php

@@ -26,6 +26,9 @@ class RoutePatternMatcherTest extends BaseUnitTest
         $this->assertTrue(RoutePatternMatcher::matches($route, ["POST ab*"]));
         $this->assertTrue(RoutePatternMatcher::matches($route, ["POST /ab*"]));
         $this->assertTrue(RoutePatternMatcher::matches($route, ["POST *"]));
+        $this->assertTrue(RoutePatternMatcher::matches($route, ["* abc"]));
+        $this->assertTrue(RoutePatternMatcher::matches($route, ["* /abc"]));
+        $this->assertTrue(RoutePatternMatcher::matches($route, ["* *"]));
 
         $this->assertFalse(RoutePatternMatcher::matches($route, ["GET /abc"]));
         $this->assertFalse(RoutePatternMatcher::matches($route, ["GET abc"]));