From 59ed2d67ef1f06f7f5dfed681c8d3b7564330e49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 9 Dec 2024 12:31:18 +0100
Subject: [PATCH 1/2] Drop TransactionalMutex class

---
 README.md                              |  31 ----
 phpstan.neon.dist                      |   5 -
 src/Mutex/TransactionalMutex.php       | 146 -------------------
 tests/Mutex/MutexConcurrencyTest.php   |  58 --------
 tests/Mutex/MutexTest.php              |   8 -
 tests/Mutex/TransactionalMutexTest.php | 193 -------------------------
 6 files changed, 441 deletions(-)
 delete mode 100644 src/Mutex/TransactionalMutex.php
 delete mode 100644 tests/Mutex/TransactionalMutexTest.php

diff --git a/README.md b/README.md
index 6e24b376..f90252cb 100644
--- a/README.md
+++ b/README.md
@@ -167,7 +167,6 @@ implementations or create/extend your own implementation.
 - [`MemcachedMutex`](#memcachedmutex)
 - [`RedisMutex`](#redismutex)
 - [`SemaphoreMutex`](#semaphoremutex)
-- [`TransactionalMutex`](#transactionalmutex)
 - [`MySQLMutex`](#mysqlmutex)
 - [`PostgreSQLMutex`](#PostgreSQLMutex)
 
@@ -261,36 +260,6 @@ $mutex->synchronized(function () use ($bankAccount, $amount) {
 });
 ```
 
-#### TransactionalMutex
-
-The **TransactionalMutex**
-delegates the serialization to the database. The exclusive code is executed within
-a transaction. It's up to you to set the correct transaction isolation level.
-However if the transaction fails (i.e. a `PDOException` was thrown), the code
-will be executed again in a new transaction. Therefore the code must not have
-any side effects besides SQL statements. Also the isolation level should be
-conserved for the repeated transaction. If the code throws an exception,
-the transaction is rolled back and not replayed again.
-
-Example:
-```php
-$mutex = new TransactionalMutex($pdo);
-$mutex->synchronized(function () use ($pdo, $accountId, $amount) {
-    $select = $pdo->prepare(
-        'SELECT balance FROM account WHERE id = ? FOR UPDATE'
-    );
-    $select->execute([$accountId]);
-    $balance = $select->fetchColumn();
-
-    $balance -= $amount;
-    if ($balance < 0) {
-        throw new \DomainException('You have no credit');
-    }
-    $pdo->prepare('UPDATE account SET balance = ? WHERE id = ?')
-        ->execute([$balance, $accountId]);
-});
-```
-
 #### MySQLMutex
 
 The **MySQLMutex** uses MySQL's
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index 0b72517d..872f6261 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -16,11 +16,6 @@ parameters:
             identifier: if.condNotBoolean
             message: '~^Only booleans are allowed in an if condition, mixed given\.$~'
             count: 1
-        -
-            path: 'src/Mutex/TransactionalMutex.php'
-            identifier: if.condNotBoolean
-            message: '~^Only booleans are allowed in an if condition, mixed given\.$~'
-            count: 1
         -
             path: 'tests/Mutex/*Test.php'
             identifier: empty.notAllowed
diff --git a/src/Mutex/TransactionalMutex.php b/src/Mutex/TransactionalMutex.php
deleted file mode 100644
index ac46b508..00000000
--- a/src/Mutex/TransactionalMutex.php
+++ /dev/null
@@ -1,146 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Malkusch\Lock\Mutex;
-
-use Malkusch\Lock\Exception\LockAcquireException;
-use Malkusch\Lock\Util\Loop;
-
-/**
- * Serialization is delegated to the database.
- *
- * The critical code is executed within a transaction. The database will decide
- * which parts of that code need to be locked (if at all).
- *
- * A failing transaction will be replayed.
- */
-class TransactionalMutex extends AbstractMutex
-{
-    private \PDO $pdo;
-
-    /** In seconds */
-    private float $acquireTimeout;
-
-    /**
-     * The PDO object MUST be configured with PDO::ATTR_ERRMODE
-     * to throw exceptions on errors.
-     *
-     * As this implementation spans a transaction over a unit of work,
-     * PDO::ATTR_AUTOCOMMIT SHOULD not be enabled.
-     *
-     * @param float $acquireTimeout In seconds
-     */
-    public function __construct(\PDO $pdo, float $acquireTimeout = 3)
-    {
-        if ($pdo->getAttribute(\PDO::ATTR_ERRMODE) !== \PDO::ERRMODE_EXCEPTION) {
-            throw new \InvalidArgumentException('The pdo must have PDO::ERRMODE_EXCEPTION set');
-        }
-        self::checkAutocommit($pdo);
-
-        $this->pdo = $pdo;
-        $this->acquireTimeout = $acquireTimeout;
-    }
-
-    /**
-     * Checks that the AUTOCOMMIT mode is turned off.
-     */
-    private static function checkAutocommit(\PDO $pdo): void
-    {
-        $vendor = $pdo->getAttribute(\PDO::ATTR_DRIVER_NAME);
-
-        // MySQL turns autocommit off during a transaction.
-        if ($vendor === 'mysql') {
-            return;
-        }
-
-        try {
-            if ($pdo->getAttribute(\PDO::ATTR_AUTOCOMMIT)) {
-                throw new \InvalidArgumentException('PDO::ATTR_AUTOCOMMIT should be disabled');
-            }
-        } catch (\PDOException $e) {
-            /*
-             * Ignore this, as some drivers would throw an exception for an
-             * unsupported attribute (e.g. Postgres).
-             */
-        }
-    }
-
-    /**
-     * Executes the critical code within a transaction.
-     *
-     * It's up to the user to set the correct transaction isolation level.
-     * However if the transaction fails, the code will be executed again in a
-     * new transaction. Therefore the code must not have any side effects
-     * besides SQL statements. Also the isolation level should be conserved for
-     * the repeated transaction.
-     *
-     * A transaction is considered as failed if a PDOException or an exception
-     * which has a PDOException as any previous exception was raised.
-     *
-     * If the code throws any other exception, the transaction is rolled back
-     * and won't  be replayed.
-     */
-    #[\Override]
-    public function synchronized(callable $code)
-    {
-        $loop = new Loop();
-
-        return $loop->execute(function () use ($code, $loop) {
-            try {
-                // BEGIN
-                $this->pdo->beginTransaction();
-            } catch (\PDOException $e) {
-                throw new LockAcquireException('Could not begin transaction', 0, $e);
-            }
-
-            try {
-                // Unit of work
-                $result = $code();
-                $this->pdo->commit();
-                $loop->end();
-
-                return $result;
-            } catch (\Throwable $e) {
-                $this->rollBack($e);
-
-                if (self::hasPDOException($e)) {
-                    return null; // Replay
-                }
-
-                throw $e;
-            }
-        }, $this->acquireTimeout);
-    }
-
-    /**
-     * Checks if an exception or any of its previous exceptions is a \PDOException.
-     */
-    private static function hasPDOException(\Throwable $exception): bool
-    {
-        if ($exception instanceof \PDOException) {
-            return true;
-        }
-
-        return $exception->getPrevious() !== null
-            && self::hasPDOException($exception->getPrevious());
-    }
-
-    /**
-     * Rolls back a transaction.
-     *
-     * @throws LockAcquireException
-     */
-    private function rollBack(\Throwable $exception): void
-    {
-        try {
-            $this->pdo->rollBack();
-        } catch (\PDOException $e) {
-            throw new LockAcquireException(
-                'Could not roll back transaction: ' . $e->getMessage(),
-                0,
-                $exception
-            );
-        }
-    }
-}
diff --git a/tests/Mutex/MutexConcurrencyTest.php b/tests/Mutex/MutexConcurrencyTest.php
index f1214b8f..6ae0a563 100644
--- a/tests/Mutex/MutexConcurrencyTest.php
+++ b/tests/Mutex/MutexConcurrencyTest.php
@@ -12,7 +12,6 @@
 use Malkusch\Lock\Mutex\PostgreSQLMutex;
 use Malkusch\Lock\Mutex\RedisMutex;
 use Malkusch\Lock\Mutex\SemaphoreMutex;
-use Malkusch\Lock\Mutex\TransactionalMutex;
 use Malkusch\Lock\Util\LockUtil;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Constraint\IsType;
@@ -135,63 +134,6 @@ static function () use ($filename): void {
                 },
             ];
         }
-
-        $makePDOCase = static function (string $dsn, string $user, string $password, string $vendor) {
-            $pdo = self::getPDO($dsn, $user, $password);
-
-            $options = ['mysql' => 'engine=InnoDB'];
-            $option = $options[$vendor] ?? '';
-            $pdo->exec('CREATE TABLE IF NOT EXISTS counter(id INT PRIMARY KEY, counter INT) ' . $option);
-
-            self::$pdo = null;
-
-            return [
-                static function (int $increment) use ($dsn, $user, $password) {
-                    // This prevents using a closed connection from a child.
-                    if ($increment === 0) {
-                        self::$pdo = null;
-                    }
-                    $pdo = self::getPDO($dsn, $user, $password);
-                    $id = 1;
-                    $select = $pdo->prepare('SELECT counter FROM counter WHERE id = ? FOR UPDATE');
-                    $select->execute([$id]);
-                    $counter = $select->fetchColumn();
-
-                    $counter += $increment;
-
-                    $pdo->prepare('UPDATE counter SET counter = ? WHERE id = ?')
-                        ->execute([$counter, $id]);
-
-                    return $counter;
-                },
-                static function ($timeout) use ($dsn, $user, $password) {
-                    self::$pdo = null;
-                    $pdo = self::getPDO($dsn, $user, $password);
-
-                    return new TransactionalMutex($pdo, $timeout);
-                },
-                static function () use ($pdo): void {
-                    $pdo->beginTransaction();
-                    $pdo->exec('DELETE FROM counter');
-                    $pdo->exec('INSERT INTO counter VALUES (1, 0)');
-                    $pdo->commit();
-                },
-            ];
-        };
-
-        if (getenv('MYSQL_DSN')) {
-            $dsn = getenv('MYSQL_DSN');
-            $user = getenv('MYSQL_USER');
-            $password = getenv('MYSQL_PASSWORD');
-            yield 'mysql' => $makePDOCase($dsn, $user, $password, 'mysql');
-        }
-
-        if (getenv('PGSQL_DSN')) {
-            $dsn = getenv('PGSQL_DSN');
-            $user = getenv('PGSQL_USER');
-            $password = getenv('PGSQL_PASSWORD');
-            yield 'postgres' => $makePDOCase($dsn, $user, $password, 'postgres');
-        }
     }
 
     /**
diff --git a/tests/Mutex/MutexTest.php b/tests/Mutex/MutexTest.php
index 9ea6ee9c..42e82421 100644
--- a/tests/Mutex/MutexTest.php
+++ b/tests/Mutex/MutexTest.php
@@ -15,7 +15,6 @@
 use Malkusch\Lock\Mutex\PostgreSQLMutex;
 use Malkusch\Lock\Mutex\RedisMutex;
 use Malkusch\Lock\Mutex\SemaphoreMutex;
-use Malkusch\Lock\Mutex\TransactionalMutex;
 use org\bovigo\vfs\vfsStream;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\DoesNotPerformAssertions;
@@ -51,13 +50,6 @@ public static function provideMutexFactoriesCases(): iterable
             return new NoMutex();
         }];
 
-        yield 'TransactionalMutex' => [static function (): Mutex {
-            $pdo = new \PDO('sqlite::memory:');
-            $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
-
-            return new TransactionalMutex($pdo, self::TIMEOUT);
-        }];
-
         yield 'FlockMutex' => [static function (): Mutex {
             $file = fopen(vfsStream::url('test/lock'), 'w');
 
diff --git a/tests/Mutex/TransactionalMutexTest.php b/tests/Mutex/TransactionalMutexTest.php
deleted file mode 100644
index 0c39fa80..00000000
--- a/tests/Mutex/TransactionalMutexTest.php
+++ /dev/null
@@ -1,193 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Malkusch\Lock\Tests\Mutex;
-
-use Malkusch\Lock\Exception\LockAcquireException;
-use Malkusch\Lock\Mutex\TransactionalMutex;
-use PHPUnit\Framework\Attributes\DataProvider;
-use PHPUnit\Framework\TestCase;
-
-/**
- * Set the environment variables MYSQL_DSN, MYSQL_USER for this test.
- */
-class TransactionalMutexTest extends TestCase
-{
-    /**
-     * Tests building the mutex with an invalid error mode.
-     *
-     * @dataProvider provideInvalidErrorModeCases
-     */
-    #[DataProvider('provideInvalidErrorModeCases')]
-    public function testInvalidErrorMode(int $mode): void
-    {
-        $this->expectException(\InvalidArgumentException::class);
-
-        $pdo = new \PDO('sqlite::memory:');
-        $pdo->setAttribute(\PDO::ATTR_ERRMODE, $mode);
-        new TransactionalMutex($pdo);
-    }
-
-    /**
-     * @return iterable<list<mixed>>
-     */
-    public static function provideInvalidErrorModeCases(): iterable
-    {
-        yield [\PDO::ERRMODE_SILENT];
-        yield [\PDO::ERRMODE_WARNING];
-    }
-
-    /**
-     * Tests BEGIN fails.
-     */
-    public function testBeginFails(): void
-    {
-        $this->expectException(LockAcquireException::class);
-        $this->expectExceptionMessage('Could not begin transaction');
-
-        $pdo = $this->buildMySqlPdo();
-        $pdo->setAttribute(\PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, false);
-
-        $stmt = $pdo->prepare('SELECT 1 FROM DUAL');
-        $stmt->execute();
-
-        $mutex = new TransactionalMutex($pdo);
-        $mutex->synchronized(static function (): void {});
-    }
-
-    /**
-     * Tests that an exception in the critical code causes a ROLLBACK.
-     */
-    public function testExceptionRollsback(): void
-    {
-        $pdo = $this->buildMySqlPdo();
-        $mutex = new TransactionalMutex($pdo);
-
-        $pdo->exec('
-            CREATE TEMPORARY TABLE testExceptionRollsback(
-                id int primary key
-            ) engine=innodb
-        ');
-
-        try {
-            $mutex->synchronized(static function () use ($pdo): void {
-                $pdo->exec('INSERT INTO testExceptionRollsback VALUES(1)');
-
-                throw new \DomainException();
-            });
-        } catch (\DomainException $e) {
-            // expected
-        }
-
-        $count = $pdo->query('SELECT count(*) FROM testExceptionRollsback')->fetchColumn();
-        self::assertSame(0, \PHP_VERSION_ID < 8_01_00 ? (int) $count : $count);
-    }
-
-    /**
-     * Tests that a ROLLBACK caused by an exception fails.
-     */
-    public function testFailExceptionRollsback(): void
-    {
-        $pdo = $this->buildMySqlPdo();
-        $mutex = new TransactionalMutex($pdo);
-
-        $this->expectException(LockAcquireException::class);
-
-        $mutex->synchronized(static function () use ($pdo) {
-            // This will provoke the mutex' rollback to fail.
-            $pdo->rollBack();
-
-            throw new \DomainException();
-        });
-    }
-
-    /**
-     * Tests replaying the transaction.
-     *
-     * @dataProvider provideReplayTransactionCases
-     */
-    #[DataProvider('provideReplayTransactionCases')]
-    public function testReplayTransaction(\Exception $exception): void
-    {
-        $pdo = $this->buildMySqlPdo();
-        $mutex = new TransactionalMutex($pdo);
-
-        $pdo->exec('
-            CREATE TEMPORARY TABLE testExceptionRollsback(
-                id int primary key
-            ) engine=innodb
-        ');
-
-        $i = 0;
-        $mutex->synchronized(static function () use ($pdo, &$i, $exception) {
-            ++$i;
-
-            $count = $pdo->query('SELECT count(*) FROM testExceptionRollsback')->fetchColumn();
-            self::assertSame(0, \PHP_VERSION_ID < 8_01_00 ? (int) $count : $count);
-
-            $pdo->exec('INSERT INTO testExceptionRollsback VALUES(1)');
-
-            // this provokes the replay
-            if ($i < 5) {
-                throw $exception;
-            }
-        });
-
-        $count = $pdo->query('SELECT count(*) FROM testExceptionRollsback')->fetchColumn();
-        self::assertSame(1, \PHP_VERSION_ID < 8_01_00 ? (int) $count : $count);
-
-        self::assertSame(5, $i);
-    }
-
-    /**
-     * @return iterable<list<mixed>>
-     */
-    public static function provideReplayTransactionCases(): iterable
-    {
-        yield [new \PDOException()];
-        yield [new \Exception('', 0, new \PDOException())];
-    }
-
-    /**
-     * Tests failing a ROLLBACK after the failed COMMIT.
-     */
-    public function testRollbackAfterFailedCommitFails(): void
-    {
-        $this->expectException(LockAcquireException::class);
-        $this->expectExceptionMessage('Could not roll back transaction:');
-
-        $pdo = $this->buildMySqlPdo();
-        $mutex = new TransactionalMutex($pdo);
-
-        $mutex->synchronized(static function () use ($pdo) {
-            // This will provoke the mutex' commit and rollback to fail.
-            $pdo->rollBack();
-        });
-    }
-
-    /**
-     * Builds a MySQL PDO.
-     *
-     * Please provide these environment variables:
-     *
-     * - MYSQL_DSN
-     * - MYSQL_USER
-     * - MYSQL_PASSWORD
-     */
-    private function buildMySqlPdo(): \PDO
-    {
-        if (!getenv('MYSQL_DSN')) {
-            self::markTestSkipped('MySQL server is required');
-        }
-
-        $dsn = getenv('MYSQL_DSN');
-        $user = getenv('MYSQL_USER');
-        $password = getenv('MYSQL_PASSWORD');
-        $pdo = new \PDO($dsn, $user, $password);
-
-        $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
-
-        return $pdo;
-    }
-}

From c7262e9fd5e6b0ceef6dcf47b972a0d8f97f7b97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 9 Dec 2024 12:42:28 +0100
Subject: [PATCH 2/2] test cleanup

---
 tests/Mutex/MutexConcurrencyTest.php | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/tests/Mutex/MutexConcurrencyTest.php b/tests/Mutex/MutexConcurrencyTest.php
index 6ae0a563..cf31834c 100644
--- a/tests/Mutex/MutexConcurrencyTest.php
+++ b/tests/Mutex/MutexConcurrencyTest.php
@@ -31,8 +31,6 @@ class MutexConcurrencyTest extends TestCase
 {
     /** @var list<string> */
     protected static $temporaryFiles = [];
-    /** @var \PDO|null */
-    private static $pdo;
 
     #[\Override]
     public static function tearDownAfterClass(): void
@@ -42,24 +40,9 @@ public static function tearDownAfterClass(): void
         }
         self::$temporaryFiles = [];
 
-        self::$pdo = null;
-
         parent::tearDownAfterClass();
     }
 
-    /**
-     * Gets a PDO instance.
-     */
-    private static function getPDO(string $dsn, string $user, string $password): \PDO
-    {
-        if (self::$pdo === null) {
-            self::$pdo = new \PDO($dsn, $user, $password);
-            self::$pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
-        }
-
-        return self::$pdo;
-    }
-
     /**
      * Forks, runs code in the children and wait until all finished.
      *