From 7a5157ac99f1cfdf7cbc73f622da8f0455da6c7b Mon Sep 17 00:00:00 2001 From: Alex Standiford Date: Mon, 27 Apr 2026 21:35:44 -0400 Subject: [PATCH] Resolve WordPress insert identity in transaction --- lib/Strategies/QueryStrategy.php | 21 ++++- lib/Traits/CanQueryWordPressDatabase.php | 90 +++++++++++++++---- tests/Unit/Strategies/QueryStrategyTest.php | 76 ++++++++++++++++ .../Traits/CanQueryWordPressDatabaseTest.php | 50 +++++++++++ 4 files changed, 217 insertions(+), 20 deletions(-) create mode 100644 tests/Unit/Strategies/QueryStrategyTest.php diff --git a/lib/Strategies/QueryStrategy.php b/lib/Strategies/QueryStrategy.php index 3aaa6d7..5f25af9 100644 --- a/lib/Strategies/QueryStrategy.php +++ b/lib/Strategies/QueryStrategy.php @@ -12,28 +12,47 @@ class QueryStrategy implements CoreQueryStrategy { use CanQueryWordPressDatabase; + /** + * Tracks whether this strategy has written during the current request. + * + * After a write, managed hosts with read replicas may route normal reads to a + * stale replica. This flag lets only post-write reads use the writer-consistent + * path, so normal reads stay cheap while read-after-write hydration can see + * the row that was just inserted or changed. + */ + protected bool $hasWritten = false; + /** @inheritDoc */ public function query(QueryBuilder $builder): array { + if ($this->hasWritten) { + return $this->wpdbGetResultsAfterWrite($builder); + } + return $this->wpdbGetResults($builder); } /** @inheritDoc */ public function insert(Table $table, array $data): array { - return $this->wpdbInsert($table, $data); + $ids = $this->wpdbInsert($table, $data); + $this->hasWritten = true; + + return $ids; } /** @inheritDoc */ public function delete(Table $table, array $ids): void { $this->wpdbDelete($table, $ids); + $this->hasWritten = true; } /** @inheritDoc */ public function update(Table $table, array $where, array $data): void { $this->wpdbUpdate($table, $data, $where); + $this->hasWritten = true; } /** @inheritDoc */ diff --git a/lib/Traits/CanQueryWordPressDatabase.php b/lib/Traits/CanQueryWordPressDatabase.php index 155188e..97450e2 100644 --- a/lib/Traits/CanQueryWordPressDatabase.php +++ b/lib/Traits/CanQueryWordPressDatabase.php @@ -10,6 +10,7 @@ use PHPNomad\Integrations\WordPress\Database\ClauseBuilder; use PHPNomad\Integrations\WordPress\Database\QueryBuilder as WordPressQueryBuilder; use PHPNomad\Utils\Helpers\Arr; +use Throwable; trait CanQueryWordPressDatabase { @@ -42,6 +43,43 @@ protected function wpdbGetResults(QueryBuilder $queryBuilder): array return $result; } + /** + * Gets a batch of rows using a writer-consistent read path after a write. + * + * @param QueryBuilder $queryBuilder + * @return array[]|array + * @throws DatastoreErrorException + * @throws RecordNotFoundException + */ + protected function wpdbGetResultsAfterWrite(QueryBuilder $queryBuilder): array + { + global $wpdb; + + if (method_exists($wpdb, 'send_reads_to_masters')) { + $wpdb->send_reads_to_masters(); + + return $this->wpdbGetResults($queryBuilder); + } + + if (false === $wpdb->query('START TRANSACTION')) { + throw new DatastoreErrorException('Consistent read failed - could not start transaction: ' . $wpdb->last_error); + } + + try { + $result = $this->wpdbGetResults($queryBuilder); + + if (false === $wpdb->query('COMMIT')) { + throw new DatastoreErrorException('Consistent read failed - could not commit transaction: ' . $wpdb->last_error); + } + + return $result; + } catch (Throwable $e) { + $wpdb->query('ROLLBACK'); + + throw $e; + } + } + /** * Gets a single row using wpdb. * @return array @@ -81,32 +119,46 @@ protected function wpdbInsert(Table $table, array $data): array { global $wpdb; - if (empty($data)) { - $inserted = $wpdb->query('INSERT INTO ' . $table->getName() . '() VALUES ();'); - } else { - $inserted = $wpdb->insert($table->getName(), $data, $this->getFormats($data)); + if (false === $wpdb->query('START TRANSACTION')) { + throw new DatastoreErrorException('Insert failed - could not start transaction: ' . $wpdb->last_error); } - if (false === $inserted) { - throw new DatastoreErrorException('Insert failed - ' . $wpdb->last_error); - } + try { + if (empty($data)) { + $inserted = $wpdb->query('INSERT INTO ' . $table->getName() . '() VALUES ();'); + } else { + $inserted = $wpdb->insert($table->getName(), $data, $this->getFormats($data)); + } + + if (false === $inserted) { + throw new DatastoreErrorException('Insert failed - ' . $wpdb->last_error); + } - $fields = $table->getFieldsForIdentity(); - $ids = Arr::process($fields) - ->reduce(function ($acc, $field) use ($data) { - if (isset($data[$field])) { - $acc[$field] = $data[$field]; - } + $fields = $table->getFieldsForIdentity(); + $ids = Arr::process($fields) + ->reduce(function ($acc, $field) use ($data) { + if (isset($data[$field])) { + $acc[$field] = $data[$field]; + } - return $acc; - }, []) - ->toArray(); + return $acc; + }, []) + ->toArray(); + + if (count($ids) !== count($fields)) { + $ids = ['id' => $wpdb->insert_id]; + } + + if (false === $wpdb->query('COMMIT')) { + throw new DatastoreErrorException('Insert failed - could not commit transaction: ' . $wpdb->last_error); + } - if (count($ids) === count($fields)) { return $ids; - } + } catch (Throwable $e) { + $wpdb->query('ROLLBACK'); - return ['id' => $wpdb->insert_id]; + throw $e; + } } /** diff --git a/tests/Unit/Strategies/QueryStrategyTest.php b/tests/Unit/Strategies/QueryStrategyTest.php new file mode 100644 index 0000000..7f324a7 --- /dev/null +++ b/tests/Unit/Strategies/QueryStrategyTest.php @@ -0,0 +1,76 @@ +queries[] = $query; + + if ($query === 'START TRANSACTION') { + $this->inTransaction = true; + } + + if ($query === 'COMMIT' || $query === 'ROLLBACK') { + $this->inTransaction = false; + } + + return true; + } + + public function insert(string $table, array $data, array $formats): int + { + return 1; + } + + public function get_results(string $query, string $output): array + { + return $this->inTransaction ? [['id' => 123, 'name' => 'Example']] : []; + } + }; + + $table = $this->createMock(Table::class); + $table->method('getName')->willReturn('wp_test_records'); + $table->method('getFieldsForIdentity')->willReturn(['id']); + + $queryBuilder = $this->createMock(QueryBuilder::class); + $queryBuilder->expects($this->once()) + ->method('build') + ->willReturn('SELECT * FROM wp_test_records WHERE id = 123'); + + $strategy = new QueryStrategy(); + $strategy->insert($table, ['name' => 'Example']); + + $this->assertSame([['id' => 123, 'name' => 'Example']], $strategy->query($queryBuilder)); + $this->assertSame(['START TRANSACTION', 'COMMIT', 'START TRANSACTION', 'COMMIT'], $GLOBALS['wpdb']->queries); + } +} diff --git a/tests/Unit/Traits/CanQueryWordPressDatabaseTest.php b/tests/Unit/Traits/CanQueryWordPressDatabaseTest.php index 9307878..f1ffe49 100644 --- a/tests/Unit/Traits/CanQueryWordPressDatabaseTest.php +++ b/tests/Unit/Traits/CanQueryWordPressDatabaseTest.php @@ -58,6 +58,56 @@ public function getResults(QueryBuilder $queryBuilder): array $subject->getResults($queryBuilder); } + public function testWpdbInsertResolvesInsertIdInsideTransaction(): void + { + $table = $this->createMock(Table::class); + $table->method('getName')->willReturn('wp_test_records'); + $table->method('getFieldsForIdentity')->willReturn(['id']); + + $GLOBALS['wpdb'] = new class { + public int $insert_id = 123; + public string $last_error = ''; + public array $queries = []; + public bool $insertedInTransaction = false; + private bool $inTransaction = false; + + public function query(string $query): bool + { + $this->queries[] = $query; + + if ($query === 'START TRANSACTION') { + $this->inTransaction = true; + } + + if ($query === 'COMMIT' || $query === 'ROLLBACK') { + $this->inTransaction = false; + } + + return true; + } + + public function insert(string $table, array $data, array $formats): int + { + $this->insertedInTransaction = $this->inTransaction; + + return 1; + } + }; + + $subject = new class { + use CanQueryWordPressDatabase; + + public function insertRecord(Table $table, array $data): array + { + return $this->wpdbInsert($table, $data); + } + }; + + $this->assertSame(['id' => 123], $subject->insertRecord($table, ['name' => 'Example'])); + $this->assertTrue($GLOBALS['wpdb']->insertedInTransaction); + $this->assertSame(['START TRANSACTION', 'COMMIT'], $GLOBALS['wpdb']->queries); + } + public function testWpdbUpdateIncludesTableIdentityAndPayloadWhenRecordIsMissing(): void { $table = $this->createMock(Table::class);