Skip to content

Feature: nested transactions#26

Merged
defunctl merged 3 commits intomainfrom
feature/nested-transactions
Feb 25, 2026
Merged

Feature: nested transactions#26
defunctl merged 3 commits intomainfrom
feature/nested-transactions

Conversation

@defunctl
Copy link
Copy Markdown
Contributor

@defunctl defunctl commented Feb 24, 2026

Main Changes

Adds support for nested database transactions. Using DB::beginTransaction() with other existing transactions causes MySQL/MariaDB to commit all existing transactions when it encounters a START TRANSACTION.

Instead, we now properly create savepoints and restore them when utilizing more than 1 transaction. This is similar to what Laravel does for their lockForTransaction functionality.

As wp-browser starts transactions as well, this allows us to start and rollback and initial transaction in tests, so we're creating savepoints instead of committing all of wp-browsers previous transactions in tests:

<?php declare( strict_types=1 );

namespace StellarWP\Licensing\Test\Classes;

use lucatume\WPBrowser\TestCase\WPTestCase;
use StellarWP\Licensing\StellarWP\DB\DB;

/**
 * @mixin \Codeception\Test\Unit
 * @mixin \PHPUnit\Framework\TestCase
 */
class TestCase extends WPTestCase {

	protected function setUp(): void {
		parent::setUp();

		/*
		 * The WP test framework starts a transaction via raw $wpdb->query('START TRANSACTION'),
		 * which the DB facade's depth counter doesn't see. Re-start through the facade so that
		 * any code calling DB::beginTransaction() creates a savepoint instead of implicitly
		 * committing the framework's transaction.
		 */
		DB::beginTransaction();
	}

	protected function tearDown(): void {
		DB::rollback();

		parent::tearDown();
	}
}

@defunctl defunctl added this to the 1.3.0 milestone Feb 24, 2026
@defunctl defunctl self-assigned this Feb 24, 2026
@defunctl defunctl marked this pull request as draft February 25, 2026 00:07
@defunctl
Copy link
Copy Markdown
Contributor Author

defunctl commented Feb 25, 2026

Edit: hold on, still testing some things.

Edit again: It's good.

@defunctl defunctl marked this pull request as ready for review February 25, 2026 00:17
Copy link
Copy Markdown
Contributor

@shvlv shvlv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really afraid of transactions in the WordPress context because of 100+ different plugins doing something in the DB, but we have it already, and it's a nice improvement. It should be a consumer's responsibility to use this carefully.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This filename is odd. I think transaction is a single word 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call

Copy link
Copy Markdown

@johnhooks johnhooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @shvlv, user beware.

I approve.

I was playing with something similar. I had trouble with WP-Browser and testing logic that used transactions because of the transaction that WP-Browser wraps around the test.

I accounted for it (or another plugin using transactions) with:

SELECT @@in_transaction;

But I knew I was running a newer database and it was available.

@defunctl defunctl merged commit 34d6c66 into main Feb 25, 2026
2 checks passed
@defunctl defunctl deleted the feature/nested-transactions branch February 25, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants