Switch to opt-in for transactions (Fixes #141, #147)

knuckleswtf · Nov 30, 2020 · 5c51486 · 5c51486
1 parent f136f16
commit 5c51486
Show file tree

Hide file tree

Showing 19 changed files with 217 additions and 173 deletions.
diff --git a/composer.dingo.json b/composer.dingo.json
@@ -19,7 +19,7 @@
         "php": ">=7.2.5",
         "ext-fileinfo": "*",
         "ext-json": "*",
-        "amphp/parallel-functions": "^1.0",
+        "ext-pdo": "*",
         "dingo/api": "^2.3|^3.0",
         "erusev/parsedown": "^1.7.4",
         "fakerphp/faker": "^1.9.1",

diff --git a/composer.json b/composer.json
@@ -19,7 +19,7 @@
     "php": ">=7.2.5",
     "ext-fileinfo": "*",
     "ext-json": "*",
-    "amphp/parallel-functions": "^1.0",
+    "ext-pdo": "*",
     "erusev/parsedown": "^1.7.4",
     "fakerphp/faker": "^1.9.1",
     "illuminate/console": "^6.0|^7.0|^8.0",

diff --git a/config/scribe.php b/config/scribe.php
@@ -365,4 +365,10 @@
      * To avoid that, you can add the driver class name here. Be warned: that means all database changes will persist.
      */
     'continue_without_database_transactions' => [],
+
+    /**
+     * For response calls, api resource responses and transformer responses, Scribe will try to start database transactions, so no changes are persisted to your database.
+     * Tell Scribe which connections should be transacted here. If you only use the default db connection, you can leave this as is.
+     */
+    'database_connections_to_transact' => [config('database.default')]
 ];
diff --git a/docs/config.md b/docs/config.md
@@ -244,7 +244,24 @@ This section only applies if you're using [transformers](https://fractal.thephpl
 ### `routeMatcher`
 The route matcher class provides the algorithm that determines what routes should be documented. The default matcher used is the included `\Knuckles\Scribe\Matching\RouteMatcher::class`, and you can provide your own custom implementation if you wish to programmatically change the algorithm. The provided matcher should be an instance of `\Knuckles\Scribe\Matching\RouteMatcherInterface`.
 
-### `continue_without_database_transactions`
+### `database_connections_to_transact`
+Scribe tries to run response calls and example model creation (API Resource and Transformer strategies) in a database transaction, and then roll it back so no changes are persisted. This item is where you specify which connections Scribe should run in transactions for.
+
+By default, this is set to your default database connection (`[config('database.default)]`), so if you only use one database connections, you should be fine. If you use multiple connections, you can add the rest to the array:
+
+```php
+'database_connections_to_transact' => [
+    config('database.default),
+    'pgsql',
+],
+```
+
+### `continue_without_database_transactions` [deprecated]
+
+```eval_rst
+.. Warning:: This config item is deprecated and going away in v3. Use :code:`database_connections_to_transact` instead.
+```
+
 By default, Scribe runs response calls and example model creation in a database transaction, and then rolls them back so no changes are persisted. If one of your database drivers does not support database transactions, Scribe will log an error and exit. If you would like Scribe to proceed (and persist the data), add the database driver class name to this array. For example:
 
 ```php

diff --git a/resources/views/index.blade.php b/resources/views/index.blade.php
@@ -13,7 +13,7 @@
 <script>
     var baseUrl = "{{ $baseUrl }}";
 </script>
-<script src="js/tryitout-{{ \Knuckles\Scribe\Tools\Flags::SCRIBE_VERSION }}.js"></script>
+<script src="js/tryitout-{{ \Knuckles\Scribe\Tools\Globals::SCRIBE_VERSION }}.js"></script>
 @endif
 
 > Base URL

diff --git a/src/Commands/GenerateDocumentation.php b/src/Commands/GenerateDocumentation.php
@@ -13,7 +13,7 @@
 use Knuckles\Scribe\Tools\ConsoleOutputUtils as c;
 use Knuckles\Scribe\Tools\DocumentationConfig;
 use Knuckles\Scribe\Tools\ErrorHandlingUtils as e;
-use Knuckles\Scribe\Tools\Flags;
+use Knuckles\Scribe\Tools\Globals;
 use Knuckles\Scribe\Tools\Utils as u;
 use Knuckles\Scribe\Writing\Writer;
 use Mpociot\Reflection\DocBlock;
@@ -115,10 +115,11 @@ private function processRoutes(array $matches)
             }
 
             try {
+                c::info('Processing route: '. c::getRouteRepresentation($route));
                 $parsedRoutes[] = $generator->processRoute($route, $routeItem->getRules());
-                c::info('Processed route: '. c::getRouteRepresentation($route));
+                c::success('Processed route: '. c::getRouteRepresentation($route));
             } catch (\Exception $exception) {
-                c::warn('Skipping route: '. c::getRouteRepresentation($route) . ' - Exception encountered.');
+                c::error('Failed processing route: '. c::getRouteRepresentation($route) . ' - Exception encountered.');
                 e::dumpExceptionIfVerbose($exception);
             }
         }
@@ -197,7 +198,7 @@ public function bootstrap(): void
     {
         // Using a global static variable here, so 🙄 if you don't like it.
         // Also, the --verbose option is included with all Artisan commands.
-        Flags::$shouldBeVerbose = $this->option('verbose');
+        Globals::$shouldBeVerbose = $this->option('verbose');
 
         c::bootstrapOutput($this->output);
 

diff --git a/src/Exceptions/DatabaseTransactionsNotSupported.php b/src/Exceptions/DatabaseTransactionsNotSupported.php
@@ -10,9 +10,9 @@ class DatabaseTransactionsNotSupported extends RuntimeException implements Scrib
     public static function create(string $connectionName, string $driverName)
     {
         return new self(
-            "Database Driver [{$driverName}] for connection [{$connectionName}] does not support transactions. " .
-            "Changes to your database will be persistent. " .
-            "To allow this, add \"{$driverName}\" to the \"continue_without_database_transactions\" config."
+            "Database driver [{$driverName}] for connection [{$connectionName}] does not support transactions." .
+            " To allow Scribe to proceed, remove \"{$connectionName}\" from the \"database_connections_to_transact\" config array.".
+            " Note that any changes to your database will be persisted."
         );
     }
 }
diff --git a/src/Extracting/DatabaseTransactionHelpers.php b/src/Extracting/DatabaseTransactionHelpers.php
@@ -2,39 +2,46 @@
 
 namespace Knuckles\Scribe\Extracting;
 
-use Exception;
 use Knuckles\Scribe\Exceptions\DatabaseTransactionsNotSupported;
 use Knuckles\Scribe\Exceptions\ScribeException;
 use Knuckles\Scribe\Tools\ConsoleOutputUtils as c;
 use Knuckles\Scribe\Tools\DocumentationConfig;
-use function Amp\ParallelFunctions\parallelMap;
-use function Amp\Promise\wait;
+use Knuckles\Scribe\Tools\Globals;
+use PDOException;
 
 trait DatabaseTransactionHelpers
 {
+    private function connectionsToTransact()
+    {
+        return $this->getConfig()->get('database_connections_to_transact', []);
+    }
+
     private function startDbTransaction()
     {
-        $connection = config('database.default', 'mysql');
+        $database = app('db');
 
-        try {
-            $driver = app('db')->connection($connection);
+        $excludedDrivers = $this->excludedDrivers();
+        foreach ($this->connectionsToTransact() as $connection) {
+            $driver = $database->connection($connection);
 
             if (self::driverSupportsTransactions($driver)) {
-                $driver->beginTransaction();
-
-                return;
-            }
-
-            $driverClassName = get_class($driver);
-
-            if ($this->shouldAllowDatabasePersistence($driverClassName)) {
+                try {
+                    if (in_array(get_class($driver), $excludedDrivers)) {
+                        continue;
+                    }
+                    $driver->beginTransaction();
+                } catch (PDOException $e) {
+                    throw new \Exception(
+                        "Failed to connect to database connection '$connection'." .
+                        " Is the database running?" .
+                        " If you aren't using this database, remove it from the `database_connections_to_transact` config array."
+                    );
+                }
+                continue;
+            } else {
+                $driverClassName = get_class($driver);
                 throw DatabaseTransactionsNotSupported::create($connection, $driverClassName);
             }
-
-            c::warn("Database driver [$driverClassName] for the connection [{$connection}] does not support transactions. Any changes made to your database will persist.");
-        } catch (ScribeException $e) {
-            throw $e;
-        } catch (Exception $e) {
         }
     }
 
@@ -43,20 +50,15 @@ private function startDbTransaction()
      */
     private function endDbTransaction()
     {
-        $connection = config('database.default', 'mysql');
-
-        try {
-            $driver = app('db')->connection($connection);
-
-            if (self::driverSupportsTransactions($driver)) {
-                $driver->rollBack();
-
-                return;
+        $database = app('db');
+
+        foreach ($this->connectionsToTransact() as $connection) {
+            $driver = $database->connection($connection);
+            try {
+                $driver->rollback();
+            } catch (\Exception $e) {
+                // Any error handling should have been done on the startDbTransaction() side
             }
-
-            $driverClassName = get_class($driver);
-            c::warn("Database driver [$driverClassName] for the connection [{$connection}] does not support transactions. Any changes made to your database have been persisted.");
-        } catch (Exception $e) {
         }
     }
 
@@ -65,27 +67,26 @@ private static function driverSupportsTransactions($driver): bool
         $methods = ['beginTransaction', 'rollback'];
 
         foreach ($methods as $method) {
-            if (! method_exists($driver, $method)) {
+            if (!method_exists($driver, $method)) {
                 return false;
             }
         }
 
         return true;
     }
 
-    /**
-     * Assesses whether drivers without transaction support can proceed
-     *
-     * @param string $driverClassName
-     *
-     * @return bool
-     */
-    private function shouldAllowDatabasePersistence(string $driverClassName): bool
+    private function excludedDrivers(): array
     {
-        $config = $this->getConfig();
+        if (!is_null(Globals::$excludedDbDrivers)) {
+            return Globals::$excludedDbDrivers;
+        }
+
+        $excludedDrivers = $this->getConfig()->get('continue_without_database_transactions', []);
+        if (count($excludedDrivers)) {
+            c::deprecated('`continue_without_database_transactions`', 'use `database_connections_to_transact`');
+        }
 
-        $whitelistedDrivers = $config->get('continue_without_database_transactions', []);
-        return in_array($driverClassName, $whitelistedDrivers);
+        return Globals::$excludedDbDrivers = $excludedDrivers;
     }
 
     /**

diff --git a/src/Extracting/Strategies/Responses/UseApiResourceTags.php b/src/Extracting/Strategies/Responses/UseApiResourceTags.php
@@ -48,8 +48,9 @@ public function __invoke(Route $route, ReflectionClass $controller, ReflectionFu
         /** @var DocBlock $methodDocBlock */
         $methodDocBlock = $docBlocks['method'];
 
+        $this->startDbTransaction();
+
         try {
-            $this->startDbTransaction();
             return $this->getApiResourceResponse($methodDocBlock->getTags(), $route, $alreadyExtractedData);
         } catch (Exception $e) {
             c::warn('Exception thrown when fetching Eloquent API resource response for [' . implode(',', $route->methods) . "] {$route->uri}.");

diff --git a/src/Extracting/Strategies/Responses/UseTransformerTags.php b/src/Extracting/Strategies/Responses/UseTransformerTags.php
@@ -46,8 +46,9 @@ public function __invoke(Route $route, ReflectionClass $controller, ReflectionFu
         /** @var DocBlock $methodDocBlock */
         $methodDocBlock = $docBlocks['method'];
 
+        $this->startDbTransaction();
+
         try {
-            $this->startDbTransaction();
             return $this->getTransformerResponse($methodDocBlock->getTags());
         } catch (Exception $e) {
             c::warn('Exception thrown when fetching transformer response for [' . implode(',', $route->methods) . "] {$route->uri}.");

diff --git a/src/Tools/ConsoleOutputUtils.php b/src/Tools/ConsoleOutputUtils.php
@@ -15,7 +15,7 @@ class ConsoleOutputUtils
 
     public static function bootstrapOutput(OutputInterface $outputInterface)
     {
-        $showDebug = Flags::$shouldBeVerbose;
+        $showDebug = Globals::$shouldBeVerbose;
         self::$clara = clara('knuckleswtf/scribe', $showDebug)
             ->useOutput($outputInterface)
             ->only();
@@ -31,7 +31,9 @@ public static function deprecated($feature, $should = null, $link = null)
         if ($should) {
             $message .= "\nYou should $should instead.";
         }
-        $message .= $link ? " See $link for details" : " See the changelog for details";
+        $message .= $link
+            ? " See $link for details"
+            : (" See the changelog for details (v".Globals::SCRIBE_VERSION.").");
 
         self::$clara->warn($message);
     }
@@ -68,6 +70,14 @@ public static function success($message)
         self::$clara->success($message);
     }
 
+    public static function error($message)
+    {
+        if (!self::$clara) {
+            self::bootstrapOutput(new ConsoleOutput);
+        }
+        self::$clara->error($message);
+    }
+
     /**
      * Return a string representation of a route to output to the console eg [GET] /api/users
      * @param Route $route

diff --git a/src/Tools/ErrorHandlingUtils.php b/src/Tools/ErrorHandlingUtils.php
@@ -2,39 +2,28 @@
 
 namespace Knuckles\Scribe\Tools;
 
-use Amp\MultiReasonException;
 use Symfony\Component\Console\Output\ConsoleOutput;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ErrorHandlingUtils
 {
     public static function dumpExceptionIfVerbose(\Throwable $e, $completelySilent = false): void
     {
-        if (Flags::$shouldBeVerbose) {
-            if ($e instanceof MultiReasonException) {
-                self::dumpException($e->getReasons()[0]);
-            } else {
-                self::dumpException($e);
-            }
+        if (Globals::$shouldBeVerbose) {
+            self::dumpException($e);
         } else if (!$completelySilent) {
-            if ($e instanceof MultiReasonException) {
-                $message = join("\n", array_map(function (\Throwable $reason) {
-                    return get_class($reason).": ".$reason->getMessage();
-                }, $e->getReasons()));
-            } else {
-                [$firstFrame, $secondFrame] = $e->getTrace();
+            [$firstFrame, $secondFrame] = $e->getTrace();
 
-                try {
-                    ['file' => $file, 'line' => $line] = $firstFrame;
-                } catch (\Exception $_) {
-                    ['file' => $file, 'line' => $line] = $secondFrame;
-                }
-                $exceptionType = get_class($e);
-                $message = $e->getMessage();
-                $message = "$exceptionType in $file at line $line: $message";
+            try {
+                ['file' => $file, 'line' => $line] = $firstFrame;
+            } catch (\Exception $_) {
+                ['file' => $file, 'line' => $line] = $secondFrame;
             }
-            ConsoleOutputUtils::warn($message);
-            ConsoleOutputUtils::warn('Run this again with the --verbose flag to see the full stack trace.');
+            $exceptionType = get_class($e);
+            $message = $e->getMessage();
+            $message = "$exceptionType in $file at line $line: $message";
+            ConsoleOutputUtils::error($message);
+            ConsoleOutputUtils::error('Run this again with the --verbose flag to see the full stack trace.');
         }
 
     }

diff --git a/src/Tools/Flags.php b/src/Tools/Flags.php
diff --git a/src/Tools/Globals.php b/src/Tools/Globals.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Knuckles\Scribe\Tools;
+
+class Globals
+{
+    public const SCRIBE_VERSION = '2.4.0';
+
+    public static $shouldBeVerbose = false;
+
+    public static $excludedDbDrivers = null;
+}
diff --git a/src/Writing/Writer.php b/src/Writing/Writer.php
@@ -9,7 +9,7 @@
 use Knuckles\Pastel\Pastel;
 use Knuckles\Scribe\Tools\ConsoleOutputUtils;
 use Knuckles\Scribe\Tools\DocumentationConfig;
-use Knuckles\Scribe\Tools\Flags;
+use Knuckles\Scribe\Tools\Globals;
 use Knuckles\Scribe\Tools\Utils;
 use Symfony\Component\Yaml\Yaml;
 
@@ -289,7 +289,7 @@ public function writeHtmlDocs(): void
 
         $this->pastel->generate($this->sourceOutputPath . '/index.md', $this->staticTypeOutputPath);
         // Add our custom JS
-        copy(__DIR__.'/../../resources/js/tryitout.js', $this->staticTypeOutputPath . '/js/tryitout-'.Flags::SCRIBE_VERSION.'.js');
+        copy(__DIR__.'/../../resources/js/tryitout.js', $this->staticTypeOutputPath . '/js/tryitout-'.Globals::SCRIBE_VERSION.'.js');
 
         if (!$this->isStatic) {
             $this->performFinalTasksForLaravelType();