DEBUG Merge branch 'fix_unique_for_fk' into feature/join-foreign-id-f…

…ield-not-id
atk4 · Aug 20, 2023 · d3933ca · d3933ca
2 parents dedc16b + 94eb209
commit d3933ca
Show file tree

Hide file tree

Showing 23 changed files with 502 additions and 205 deletions.
diff --git a/bootstrap-types.php b/bootstrap-types.php
@@ -7,7 +7,12 @@
 use Atk4\Data\Type\LocalObjectType;
 use Atk4\Data\Type\MoneyType;
 use Atk4\Data\Type\Types;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Types as DbalTypes;
 
+// force Doctrine\DBAL\Platforms\SQLitePlatform class load as in DBAL 3.x it is named with a different case
+// remove once DBAL 3.x support is dropped
+new SqlitePlatform();
+
 DbalTypes\Type::addType(Types::LOCAL_OBJECT, LocalObjectType::class);
 DbalTypes\Type::addType(Types::MONEY, MoneyType::class);
diff --git a/composer.json b/composer.json
@@ -35,13 +35,13 @@
     "require": {
         "php": ">=7.4 <8.3",
         "atk4/core": "dev-develop",
-        "doctrine/dbal": "~3.4.5 || ~3.5.1 || ~3.6.0",
+        "doctrine/dbal": "~3.5.1 || ~3.6.0",
         "mvorisek/atk4-hintable": "~1.9.0"
     },
     "require-release": {
         "php": ">=7.4 <8.3",
         "atk4/core": "~5.0.0",
-        "doctrine/dbal": "~3.4.5 || ~3.5.1 || ~3.6.0",
+        "doctrine/dbal": "~3.5.1 || ~3.6.0",
         "mvorisek/atk4-hintable": "~1.9.0"
     },
     "require-dev": {

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -17,9 +17,9 @@ parameters:
 
         # fix https://github.com/phpstan/phpstan-deprecation-rules/issues/52 and https://github.com/phpstan/phpstan/issues/6444
         -
-            message: '~^Call to method (getVarcharTypeDeclarationSQL|getClobTypeDeclarationSQL|getCreateTableSQL|getCurrentDatabaseExpression|initializeDoctrineTypeMappings)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQLPlatform|SQLServerPlatform|AbstractPlatform):\nUse.+instead\.$~'
+            message: '~^Call to method (getVarcharTypeDeclarationSQL|getClobTypeDeclarationSQL|getCreateIndexSQL|getCreateTableSQL|getCurrentDatabaseExpression|initializeDoctrineTypeMappings)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQLPlatform|SQLServerPlatform|AbstractPlatform):\nUse.+instead\.$~'
             path: '*'
-            count: 5
+            count: 6
         # https://github.com/phpstan/phpstan-deprecation-rules/issues/75
         -
             message: '~^Call to deprecated method getVarcharTypeDeclarationSQL\(\) of class AnonymousClass\w+:\nUse \{@link getStringTypeDeclarationSQL\(\)\} instead\.$~'
@@ -36,15 +36,11 @@ parameters:
             path: '*'
             count: 3
 
-        # TODO for DBAL 4.0 upgrade
+        # FC for DBAL 4.0, remove once DBAL 3.x support is dropped
         - 
-            message: '~^Instantiation of deprecated class Doctrine\\DBAL\\Event\\Listeners\\OracleSessionInit:\nUse \{@see \\Doctrine\\DBAL\\Driver\\OCI8\\Middleware\\InitializeSession\} instead\.$~'
-            path: 'src/Persistence/Sql/Oracle/Connection.php'
-            count: 1
-        - 
-            message: '~^(Fetching deprecated class constant postConnect of class Doctrine\\DBAL\\Events\.|Fetching class constant postConnect of deprecated class Doctrine\\DBAL\\Events\.|Parameter \$args of method postConnect\(\) in anonymous class has typehint with deprecated class Doctrine\\DBAL\\Event\\ConnectionEventArgs\.)$~'
-            path: 'src/Persistence/Sql/Sqlite/Connection.php'
-            count: 3
+            message: '~^Class Doctrine\\DBAL\\Platforms\\SqlitePlatform referenced with incorrect case: Doctrine\\DBAL\\Platforms\\SQLitePlatform\.$~'
+            path: '*'
+            count: 33
 
         # TODO these rules are generated, this ignores should be fixed in the code
         # for src/Schema/TestCase.php

diff --git a/src/Model/UserAction.php b/src/Model/UserAction.php
@@ -32,15 +32,15 @@ class UserAction
     public const APPLIES_TO_MULTIPLE_RECORDS = 'multiple'; // e.g. delete
     public const APPLIES_TO_ALL_RECORDS = 'all'; // e.g. truncate
 
-    /** @var string by default action is for a single record */
-    public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;
-
     /** Defining action modifier */
     public const MODIFIER_CREATE = 'create'; // create new record(s)
     public const MODIFIER_UPDATE = 'update'; // update existing record(s)
     public const MODIFIER_DELETE = 'delete'; // delete record(s)
     public const MODIFIER_READ = 'read'; // just read, does not modify record(s)
 
+    /** @var string by default action is for a single record */
+    public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;
+
     /** @var string How this action interact with record */
     public $modifier;
 
@@ -74,26 +74,28 @@ class UserAction
     /** @var bool Atomic action will automatically begin transaction before and commit it after completing. */
     public $atomic = true;
 
+    private function _getOwner(): Model
+    {
+        return $this->getOwner(); // @phpstan-ignore-line;
+    }
+
     public function isOwnerEntity(): bool
     {
-        /** @var Model */
-        $owner = $this->getOwner(); // @phpstan-ignore-line
+        $owner = $this->_getOwner();
 
         return $owner->isEntity();
     }
 
     public function getModel(): Model
     {
-        /** @var Model */
-        $owner = $this->getOwner(); // @phpstan-ignore-line
+        $owner = $this->_getOwner();
 
         return $owner->getModel(true);
     }
 
     public function getEntity(): Model
     {
-        /** @var Model */
-        $owner = $this->getOwner(); // @phpstan-ignore-line
+        $owner = $this->_getOwner();
         $owner->assertIsEntity();
 
         return $owner;
@@ -104,8 +106,7 @@ public function getEntity(): Model
      */
     public function getActionForEntity(Model $entity): self
     {
-        /** @var Model */
-        $owner = $this->getOwner(); // @phpstan-ignore-line
+        $owner = $this->_getOwner();
 
         $entity->assertIsEntity($owner);
         foreach ($owner->getUserActions() as $name => $action) {
@@ -126,12 +127,13 @@ public function getActionForEntity(Model $entity): self
      */
     public function execute(...$args)
     {
+        $passOwner = false;
         if ($this->callback === null) {
-            $fx = \Closure::fromCallable([$this->getEntity(), $this->shortName]);
+            $fx = \Closure::fromCallable([$this->_getOwner(), $this->shortName]);
         } elseif (is_string($this->callback)) {
-            $fx = \Closure::fromCallable([$this->getEntity(), $this->callback]);
+            $fx = \Closure::fromCallable([$this->_getOwner(), $this->callback]);
         } else {
-            array_unshift($args, $this->getEntity());
+            $passOwner = true;
             $fx = $this->callback;
         }
 
@@ -140,9 +142,13 @@ public function execute(...$args)
         try {
             $this->validateBeforeExecute();
 
+            if ($passOwner) {
+                array_unshift($args, $this->_getOwner());
+            }
+
             return $this->atomic === false
                 ? $fx(...$args)
-                : $this->getModel()->atomic(static fn () => $fx(...$args));
+                : $this->_getOwner()->atomic(static fn () => $fx(...$args));
         } catch (CoreException $e) {
             $e->addMoreInfo('action', $this);
 
@@ -152,39 +158,39 @@ public function execute(...$args)
 
     protected function validateBeforeExecute(): void
     {
-        if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->getEntity()) === false)) {
-            throw new Exception('This action is disabled');
+        if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->_getOwner()) === false)) {
+            throw new Exception('User action is disabled');
         }
 
-        // Verify that model fields wouldn't be too dirty
-        if (is_array($this->fields)) {
-            $tooDirty = array_diff(array_keys($this->getEntity()->getDirtyRef()), $this->fields);
+        if (!is_bool($this->fields) && $this->isOwnerEntity()) {
+            $dirtyFields = array_keys($this->getEntity()->getDirtyRef());
+            $tooDirtyFields = array_diff($dirtyFields, $this->fields);
 
-            if ($tooDirty) {
-                throw (new Exception('Calling user action on a Model with dirty fields that are not allowed by this action'))
-                    ->addMoreInfo('too_dirty', $tooDirty)
-                    ->addMoreInfo('dirty', array_keys($this->getEntity()->getDirtyRef()))
-                    ->addMoreInfo('permitted', $this->fields);
+            if ($tooDirtyFields !== []) {
+                throw (new Exception('User action cannot be executed when unrelated fields are dirty'))
+                    ->addMoreInfo('tooDirtyFields', $tooDirtyFields)
+                    ->addMoreInfo('otherDirtyFields', array_diff($dirtyFields, $tooDirtyFields));
             }
-        } elseif (!is_bool($this->fields)) { // @phpstan-ignore-line
-            throw (new Exception('Argument `fields` for the user action must be either array or boolean'))
-                ->addMoreInfo('fields', $this->fields);
         }
 
-        // Verify some records scope cases
         switch ($this->appliesTo) {
             case self::APPLIES_TO_NO_RECORDS:
                 if ($this->getEntity()->isLoaded()) {
-                    throw (new Exception('This user action can be executed on non-existing record only'))
+                    throw (new Exception('User action can be executed on new entity only'))
                         ->addMoreInfo('id', $this->getEntity()->getId());
                 }
 
                 break;
             case self::APPLIES_TO_SINGLE_RECORD:
                 if (!$this->getEntity()->isLoaded()) {
-                    throw new Exception('This user action requires you to load existing record first');
+                    throw new Exception('User action can be executed on loaded entity only');
                 }
 
+                break;
+            case self::APPLIES_TO_MULTIPLE_RECORDS:
+            case self::APPLIES_TO_ALL_RECORDS:
+                $this->_getOwner()->assertIsModel();
+
                 break;
         }
     }
@@ -198,14 +204,27 @@ protected function validateBeforeExecute(): void
      */
     public function preview(...$args)
     {
+        $passOwner = false;
         if (is_string($this->preview)) {
-            $fx = \Closure::fromCallable([$this->getEntity(), $this->preview]);
+            $fx = \Closure::fromCallable([$this->_getOwner(), $this->preview]);
         } else {
-            array_unshift($args, $this->getEntity());
+            $passOwner = true;
             $fx = $this->preview;
         }
 
-        return $fx(...$args);
+        try {
+            $this->validateBeforeExecute();
+
+            if ($passOwner) {
+                array_unshift($args, $this->_getOwner());
+            }
+
+            return $fx(...$args);
+        } catch (CoreException $e) {
+            $e->addMoreInfo('action', $this);
+
+            throw $e;
+        }
     }
 
     /**
@@ -232,7 +251,7 @@ public function getConfirmation()
         } elseif ($this->confirmation === true) {
             $confirmation = 'Are you sure you wish to execute '
                 . $this->getCaption()
-                . ($this->getEntity()->getTitle() ? ' using ' . $this->getEntity()->getTitle() : '')
+                . ($this->isOwnerEntity() && $this->getEntity()->getTitle() ? ' using ' . $this->getEntity()->getTitle() : '')
                 . '?';
 
             return $confirmation;

diff --git a/src/Model/UserActionsTrait.php b/src/Model/UserActionsTrait.php
@@ -155,7 +155,7 @@ protected function initUserActions(): void
         ]);
 
         $this->addUserAction('validate', [
-            // 'appliesTo' => any!
+            // 'appliesTo' => any entity!
             'description' => 'Provided with modified values will validate them but will not save',
             'modifier' => UserAction::MODIFIER_READ,
             'fields' => true,

diff --git a/src/Persistence/Sql/Connection.php b/src/Persistence/Sql/Connection.php
@@ -5,8 +5,7 @@
 namespace Atk4\Data\Persistence\Sql;
 
 use Atk4\Core\DiContainerTrait;
-use Doctrine\Common\EventManager;
-use Doctrine\DBAL\Configuration as DbalConfiguration;
+use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Connection as DbalConnection;
 use Doctrine\DBAL\ConnectionException as DbalConnectionException;
 use Doctrine\DBAL\Driver as DbalDriver;
@@ -226,10 +225,10 @@ private static function getDriverNameFromDbalDriverConnection(DbalDriverConnecti
         return null; // @phpstan-ignore-line
     }
 
-    protected static function createDbalConfiguration(): DbalConfiguration
+    protected static function createDbalConfiguration(): Configuration
     {
-        $dbalConfiguration = new DbalConfiguration();
-        $dbalConfiguration->setMiddlewares([
+        $configuration = new Configuration();
+        $configuration->setMiddlewares([
             new class() implements DbalMiddleware {
                 public function wrap(DbalDriver $driver): DbalDriver
                 {
@@ -238,12 +237,7 @@ public function wrap(DbalDriver $driver): DbalDriver
             },
         ]);
 
-        return $dbalConfiguration;
-    }
-
-    protected static function createDbalEventManager(): EventManager
-    {
-        return new EventManager();
+        return $configuration;
     }
 
     /**
@@ -260,8 +254,7 @@ protected static function connectFromDsn(array $dsn): DbalDriverConnection
 
         $dbalConnection = DriverManager::getConnection(
             $dsn, // @phpstan-ignore-line
-            (static::class)::createDbalConfiguration(),
-            (static::class)::createDbalEventManager()
+            (static::class)::createDbalConfiguration()
         );
 
         return $dbalConnection->getWrappedConnection(); // @phpstan-ignore-line https://github.com/doctrine/dbal/issues/5199
@@ -271,8 +264,7 @@ protected static function connectFromDbalDriverConnection(DbalDriverConnection $
     {
         $dbalConnection = DriverManager::getConnection(
             ['driver' => self::getDriverNameFromDbalDriverConnection($dbalDriverConnection)],
-            (static::class)::createDbalConfiguration(),
-            (static::class)::createDbalEventManager()
+            (static::class)::createDbalConfiguration()
         );
         \Closure::bind(function () use ($dbalConnection, $dbalDriverConnection): void {
             $dbalConnection->_conn = $dbalDriverConnection;

diff --git a/src/Persistence/Sql/DbalDriverMiddleware.php b/src/Persistence/Sql/DbalDriverMiddleware.php
@@ -6,18 +6,16 @@
 
 use Doctrine\DBAL\Connection as DbalConnection;
 use Doctrine\DBAL\Driver\API\ExceptionConverter;
-use Doctrine\DBAL\Driver\API\SQLite\ExceptionConverter as SqliteExceptionConverter;
 use Doctrine\DBAL\Driver\API\SQLSrv\ExceptionConverter as SQLServerExceptionConverter;
 use Doctrine\DBAL\Driver\Exception as DbalDriverException;
 use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
 use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
 use Doctrine\DBAL\Exception\DriverException as DbalDriverConvertedException;
-use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
 use Doctrine\DBAL\Exception\TableNotFoundException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
-use Doctrine\DBAL\Platforms\SqlitePlatform;
+use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Query as DbalQuery;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
@@ -28,8 +26,8 @@ class DbalDriverMiddleware extends AbstractDriverMiddleware
 {
     protected function replaceDatabasePlatform(AbstractPlatform $platform): AbstractPlatform
     {
-        if ($platform instanceof SqlitePlatform) {
-            $platform = new class() extends SqlitePlatform {
+        if ($platform instanceof SQLitePlatform) {
+            $platform = new class() extends SQLitePlatform {
                 use Sqlite\PlatformTrait;
             };
         } elseif ($platform instanceof PostgreSQLPlatform) {
@@ -64,7 +62,7 @@ public function createDatabasePlatformForVersion($version): AbstractPlatform
      */
     public function getSchemaManager(DbalConnection $connection, AbstractPlatform $platform): AbstractSchemaManager
     {
-        if ($platform instanceof SqlitePlatform) {
+        if ($platform instanceof SQLitePlatform) {
             return new class($connection, $platform) extends SqliteSchemaManager { // @phpstan-ignore-line
                 use Sqlite\SchemaManagerTrait;
             };
@@ -116,22 +114,7 @@ final protected static function getUnconvertedException(DbalDriverConvertedExcep
     public function getExceptionConverter(): ExceptionConverter
     {
         $exceptionConverter = parent::getExceptionConverter();
-        if ($exceptionConverter instanceof SqliteExceptionConverter) {
-            $exceptionConverter = $this->createExceptionConvertorMiddleware(
-                $exceptionConverter,
-                function (DbalDriverConvertedException $convertedException, ?DbalQuery $query): DbalDriverConvertedException {
-                    // fix FK violation exception conversion
-                    // https://github.com/doctrine/dbal/issues/5496
-                    $exception = self::getUnconvertedException($convertedException);
-                    $exceptionMessageLc = strtolower($exception->getMessage());
-                    if (str_contains($exceptionMessageLc, 'integrity constraint violation')) {
-                        return new ForeignKeyConstraintViolationException($exception, $query);
-                    }
-
-                    return $convertedException;
-                }
-            );
-        } elseif ($exceptionConverter instanceof SQLServerExceptionConverter) {
+        if ($exceptionConverter instanceof SQLServerExceptionConverter) {
             $exceptionConverter = $this->createExceptionConvertorMiddleware(
                 $exceptionConverter,
                 function (DbalDriverConvertedException $convertedException, ?DbalQuery $query): DbalDriverConvertedException {
@@ -140,7 +123,7 @@ function (DbalDriverConvertedException $convertedException, ?DbalQuery $query):
                     if ($convertedException instanceof DatabaseObjectNotFoundException) {
                         $exception = self::getUnconvertedException($convertedException);
                         $exceptionMessageLc = strtolower($exception->getMessage());
-                        if (str_contains($exceptionMessageLc, 'cannot drop the table')) {
+                        if (str_contains($exceptionMessageLc, 'cannot drop the table') && !$convertedException instanceof TableNotFoundException) {
                             return new TableNotFoundException($exception, $query);
                         }
                     }