Merge branch 'dba-transactions' into 'dev'

include/dba: Make Dba driver transaction aware.

See merge request hubzilla/core!2108

4 files changed, 321 insertions, 32 deletions

diff --git a/include/dba/dba_transaction.php b/include/dba/dba_transaction.php
new file mode 100644
index 000000000..02e9945ca
--- /dev/null
+++ b/include/dba/dba_transaction.php
@@ -0,0 +1,64 @@
+<?php
+/**
+ * Class to represent a database transaction.
+ *
+ * A database transaction is initiated upon construction of an object of this
+ * class. The transaction will be automatically rolled back upon destruction
+ * unless it has been explicitly committed by calling the `commit` method.
+ *
+ * Wrapping multiple database operation within a transaction ensures that all
+ * (or none) of the operations are successfully completed at the same time.
+ *
+ * If a transaction is already active when constructing an object of this
+ * class, it will _not_ try to initiate a transaction, but constructs an object
+ * that will in practice be a stub. This prevents that "nested" transactions
+ * will cause problems with the existing active transaction.
+ *
+ * It also means that any rollbacks or commits perfomed on the "nested"
+ * transaction will be ignored, and postponed to the outer transaction is
+ * committed or rolled back.
+ *
+ * Also note that any modification to the database schema will implicitly
+ * commit active transactions in most cases, so be careful about relying on
+ * transactions in those cases.
+ *
+ * @Note This class assumes the actual underlying database driver is PDO.
+ */
+class DbaTransaction {
+	private bool $committed = false;
+	private bool $active = false;
+
+	/**
+	 * Creates a database transaction object.
+	 *
+	 * If a transaction is already active for this db connection,
+	 * no transaction is initiated, and the constructed object will
+	 * not perform any commit or rollback actions.
+	 */
+	public function __construct(private dba_driver $dba) {
+		if (! $this->dba->db->inTransaction()) {
+			$this->active = $this->dba->db->beginTransaction();
+		}
+	}
+
+	/**
+	 * Roll back the transaction if it is active and not already committed.
+	 */
+	public function __destruct() {
+		if ($this->active && ! $this->committed) {
+			$this->dba->db->rollBack();
+		}
+	}
+
+	/**
+	 * Commit the transaction if active.
+	 *
+	 * This will also mark the transaction as committed, preventing it from
+	 * being attempted rolled back on destruction.
+	 */
+	public function commit(): void {
+		if ($this->active && ! $this->committed) {
+			$this->committed = $this->dba->db->commit();
+		}
+	}
+}
diff --git a/tests/fakes/fake_dba.php b/tests/fakes/fake_dba.php
new file mode 100644
index 000000000..2289f5c80
--- /dev/null
+++ b/tests/fakes/fake_dba.php
@@ -0,0 +1,18 @@
+<?php
+namespace Zotlabs\Tests\Fakes;
+
+require_once 'include/dba/dba_pdo.php';
+
+/**
+ * Fake dba_driver implementation.
+ *
+ * This is a subclass of the dba_pdo class, that essentially lets us inject a
+ * stub for the PDO class that is the actual database driver.
+ */
+class FakeDba extends \dba_pdo {
+	public function __construct($stub) {
+		$this->db = $stub;
+		$this->connected = true;
+	}
+}
+
diff --git a/tests/unit/UnitTestCase.php b/tests/unit/UnitTestCase.php
index 0bf7b547a..18467d91e 100644
--- a/tests/unit/UnitTestCase.php
+++ b/tests/unit/UnitTestCase.php
@@ -23,6 +23,7 @@
 namespace Zotlabs\Tests\Unit;
 
 use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\TestResult;
 
 /*
  * Make sure global constants and the global App object is available to the
@@ -41,10 +42,39 @@ require_once 'include/dba/dba_driver.php' ;
  * @author Klaus Weidenbach
  */
 class UnitTestCase extends TestCase {
-	private bool $in_transaction = false;
 	protected array $fixtures = array();
 
-	public static function setUpBeforeClass() : void {
+	/**
+	 * Override the PHPUnit\Framework\TestCase::run method, so we can
+	 * wrap it in a database transaction.
+	 *
+	 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+	 */
+	public function run(TestResult $result = null): TestResult {
+		// $myclass = get_class($this);
+		// logger("[*] Running test: {$myclass}::{$this->getName(true)}", LOGGER_DEBUG);
+
+		if (! \DBA::$dba) {
+			//logger('[*] Connecting to test db...');
+			$this->connect_to_test_db();
+		}
+
+		// The $transactuion variable is needed to hold the transaction until the
+		// function returns.
+		$transaction = new \DbaTransaction(\DBA::$dba);
+
+		$this->loadFixtures();
+
+		// Make sure app config is reset and loaded from fixtures
+		\App::$config = array();
+		\Zotlabs\Lib\Config::Load('system');
+
+		$result = parent::run($result);
+
+		return $result;
+	}
+
+	protected function connect_to_test_db() : void {
 		if ( !\DBA::$dba ) {
 			\DBA::dba_factory(
 				getenv('HZ_TEST_DB_HOST') ?: 'db',
@@ -71,36 +101,6 @@ class UnitTestCase extends TestCase {
 		}
 	}
 
-	protected function setUp() : void {
-		$myclass = get_class($this);
-		logger("[*] Running test: {$myclass}::{$this->getName(true)}", LOGGER_DEBUG);
-		if ( \DBA::$dba->connected ) {
-			// Create a transaction, so that any actions taken by the
-			// tests does not change the actual contents of the database.
-			$this->in_transaction = \DBA::$dba->db->beginTransaction();
-
-			$this->loadFixtures();
-		}
-
-		// Make sure app config is reset and loaded from fixtures
-		\App::$config = array();
-		\Zotlabs\Lib\Config::Load('system');
-	}
-
-	protected function tearDown() : void {
-		if ( \DBA::$dba->connected && $this->in_transaction ) {
-			// Roll back the transaction, restoring the db to the
-			// state it was before the test was run.
-			if ( \DBA::$dba->db->rollBack() ) {
-				$this->in_transaction = false;
-			} else {
-				throw new \Exception(
-					"Transaction rollback failed! Error is: "
-					. \DBA::$dba->db->errorInfo());
-			}
-		}
-	}
-
 	private static function dbtype(string $type): int {
 		if (trim(strtolower($type)) === 'postgres') {
 			return DBTYPE_POSTGRES;
diff --git a/tests/unit/includes/dba/TransactionTest.php b/tests/unit/includes/dba/TransactionTest.php
new file mode 100644
index 000000000..99e3f459d
--- /dev/null
+++ b/tests/unit/includes/dba/TransactionTest.php
@@ -0,0 +1,207 @@
+<?php
+/*
+ * Copyright (c) 2024 Hubzilla
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+require_once 'tests/fakes/fake_dba.php';
+require_once 'include/dba/dba_transaction.php';
+
+use \PHPUnit\Framework\TestCase;
+use \Zotlabs\Tests\Fakes\FakeDba;
+
+/**
+ * Test database transactions.
+ *
+ * This class subclass the base PHPUnit TestCase class, since we do _not_
+ * want a real database connection for these tests. We're testing functionality
+ * of the database adapter itself, so we choose to stub the underlying db driver
+ * to be able to assert that the adapter behaves as it should.
+ */
+class DbaTransactionTest extends TestCase {
+	private $pdo_stub;
+
+	public function setUp(): void {
+		$this->pdo_stub = $this->createStub(PDO::class);
+	}
+
+
+	/**
+	 * Test that creating a DbaTransaction object initiates a database transaction.
+	 *
+	 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+	 */
+	public function test_transaction_initialized_on_construction(): void {
+		// Stub PDO::inTransaction()
+		// Expect that it's called once, and return false to simulate that no
+		// transactions are active.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('inTransaction')
+			->willReturn(false);
+
+		// Stub PDO::beginTransaction to ensure that it is being called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('beginTransaction')
+			->willReturn(true);
+
+		$dba = new FakeDba($this->pdo_stub);
+
+		$transaction = new DbaTransaction($dba);
+	}
+
+	/**
+	 * Test that a transaction is rolled back when the DbaTransaction object
+	 * is destroyed.
+	 *
+	 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+	 */
+	public function test_uncommitted_transaction_is_rolled_back_on_destruction(): void {
+		// Stub PDO::inTransaction()
+		// Expect that it's called once, and return false to simulate that no
+		// transactions are active.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('inTransaction')
+			->willReturn(false);
+
+		// Stub PDO::beginTransaction to ensure that it is being called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('beginTransaction')
+			->willReturn(true);
+
+		// Stub PDO::rollBack to make sure we test it is being called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('rollBack')
+			->willReturn(true);
+
+		$dba = new FakeDba($this->pdo_stub);
+
+		$transaction = new DbaTransaction($dba);
+	}
+
+	/**
+	 * Test that a committed transaction is not rolled back when the
+	 * DbaTransaction object goes out of scope.
+	 */
+	public function test_committed_transaction_is_not_rolled_back(): void {
+		// Stub PDO::inTransaction()
+		// Return false to simulate that no transaction is active when called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('inTransaction')
+			->willReturn(false);
+
+		// Stub PDO::beginTransaction to ensure that it is being called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('beginTransaction')
+			->willReturn(true);
+
+		// Stub PDO::rollBack to ensure it is _not_ called
+		$this->pdo_stub
+			->expects($this->never())
+			->method('rollBack');
+
+		// Stub PDO::commit to make the test check that it is being called
+		$this->pdo_stub
+			->expects($this->once())
+			->method('commit')
+			->willReturn(true);
+
+		$dba = new FakeDba($this->pdo_stub);
+
+		$transaction = new DbaTransaction($dba);
+		$transaction->commit();
+	}
+
+	/**
+	 * Test that commiting a transaction more than once is a no-op.
+	 */
+	public function test_that_committing_an_already_committed_transaction_does_nothing(): void {
+		// Stub PDO::inTransaction()
+		// Return false to simulate that no transaction is active when called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('inTransaction')
+			->willReturn(false);
+
+		// Stub PDO::beginTransaction to ensure that it is being called.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('beginTransaction')
+			->willReturn(true);
+
+		// Stub PDO::rollBack to ensure it is _not_ called
+		$this->pdo_stub
+			->expects($this->never())
+			->method('rollBack');
+
+		// Stub PDO::commit to make the test check that it is being called
+		$this->pdo_stub
+			->expects($this->once())
+			->method('commit')
+			->willReturn(true);
+
+		$dba = new FakeDba($this->pdo_stub);
+
+		$transaction = new DbaTransaction($dba);
+		$transaction->commit();
+		$transaction->commit();
+	}
+
+	/**
+	 * Test simulating constructing a DbaTransaction object when a transaction
+	 * is already active.
+	 *
+	 * This should _not_ initiate an actual DB transaction, not call the rollBack
+	 * method on destruction.
+	 *
+	 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+	 */
+	public function test_that_nesting_a_transaction_does_not_create_a_new_transaction_in_db(): void {
+		// Stub PDO::inTransaction()
+		// We simulate that a transaction is already active, by returning true from
+		// this method.
+		$this->pdo_stub
+			->expects($this->once())
+			->method('inTransaction')
+			->willReturn(true);
+
+		// Stub PDO::beginTransaction
+		// Since a transaction is already active, we should _not_ initiate
+		// a new transaction when the DbaTransaction object is constructed.
+		$this->pdo_stub
+			->expects($this->never())
+			->method('beginTransaction');
+
+		// Stub PDO::rollBack to ensure it is _not_ called
+		$this->pdo_stub
+			->expects($this->never())
+			->method('rollBack');
+
+		$dba = new FakeDba($this->pdo_stub);
+
+		$transaction = new DbaTransaction($dba);
+	}
+}