From 25f61141cb1d907ee6f318c3ee454da809dd9423 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Wed, 20 Mar 2024 11:49:57 +1300 Subject: [PATCH] Enhancements required for linkfield migration (#11171) * ENH Add lightweight test override for Environment::isCli() * NEW Allow JOIN with SQL UPDATE. --- src/Core/Environment.php | 10 +++++ src/ORM/Connect/DBQueryBuilder.php | 54 +++++++++++++++-------- tests/php/ORM/SQLUpdateTest.php | 25 ++++++++++- tests/php/ORM/SQLUpdateTest.yml | 4 ++ tests/php/ORM/SQLUpdateTest/TestOther.php | 16 +++++++ 5 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 tests/php/ORM/SQLUpdateTest/TestOther.php diff --git a/src/Core/Environment.php b/src/Core/Environment.php index a6b0fc7ef..aa39a0cc0 100644 --- a/src/Core/Environment.php +++ b/src/Core/Environment.php @@ -35,6 +35,13 @@ class Environment */ protected static $env = []; + /** + * Used by unit tests to override `isCli()` + * This is not config. Use reflection to change the value + * @internal + */ + private static ?bool $isCliOverride = null; + /** * Extract env vars prior to modification * @@ -251,6 +258,9 @@ class Environment */ public static function isCli() { + if (self::$isCliOverride !== null) { + return self::$isCliOverride; + } return in_array(strtolower(php_sapi_name() ?? ''), ['cli', 'phpdbg']); } } diff --git a/src/ORM/Connect/DBQueryBuilder.php b/src/ORM/Connect/DBQueryBuilder.php index 6a05b41d6..89bc4db79 100644 --- a/src/ORM/Connect/DBQueryBuilder.php +++ b/src/ORM/Connect/DBQueryBuilder.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\Connect; use InvalidArgumentException; +use LogicException; use SilverStripe\Control\Director; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Convert; @@ -400,8 +401,8 @@ class DBQueryBuilder */ public function buildUpdateFragment(SQLUpdate $query, array &$parameters) { - $table = $query->getTable(); - $text = "UPDATE $table"; + $nl = $this->getSeparator(); + $text = "{$nl}UPDATE " . $this->getTableWithJoins($query, $parameters); // Join SET components together, considering parameters $parts = []; @@ -427,26 +428,13 @@ class DBQueryBuilder */ public function buildFromFragment(SQLConditionalExpression $query, array &$parameters) { - $from = $query->getJoins($joinParameters); - $tables = []; - $joins = []; - - // E.g. a naive "Select 1" statement is valid SQL - if (empty($from)) { + $from = $this->getTableWithJoins($query, $parameters, true); + if ($from === '') { return ''; } - foreach ($from as $joinOrTable) { - if (preg_match(SQLConditionalExpression::getJoinRegex(), $joinOrTable)) { - $joins[] = $joinOrTable; - } else { - $tables[] = $joinOrTable; - } - } - - $parameters = array_merge($parameters, $joinParameters); $nl = $this->getSeparator(); - return "{$nl}FROM " . implode(', ', $tables) . ' ' . implode(' ', $joins); + return "{$nl}FROM " . $from; } /** @@ -601,4 +589,34 @@ class DBQueryBuilder } return $clause; } + + /** + * Get the name of the table (along with any join clauses) the query will operate on. + */ + private function getTableWithJoins(SQLConditionalExpression $query, array &$parameters, bool $allowEmpty = false): string + { + $from = $query->getJoins($joinParameters); + $tables = []; + $joins = []; + + // E.g. a naive "Select 1" statement is valid SQL + if (empty($from)) { + if ($allowEmpty) { + return ''; + } else { + throw new LogicException('Query have at least one table to operate on.'); + } + } + + foreach ($from as $joinOrTable) { + if (preg_match(SQLConditionalExpression::getJoinRegex(), $joinOrTable)) { + $joins[] = $joinOrTable; + } else { + $tables[] = $joinOrTable; + } + } + + $parameters = array_merge($parameters, $joinParameters); + return implode(', ', $tables) . ' ' . implode(' ', $joins); + } } diff --git a/tests/php/ORM/SQLUpdateTest.php b/tests/php/ORM/SQLUpdateTest.php index 69af48638..387e8facc 100644 --- a/tests/php/ORM/SQLUpdateTest.php +++ b/tests/php/ORM/SQLUpdateTest.php @@ -12,12 +12,12 @@ use SilverStripe\Dev\SapphireTest; */ class SQLUpdateTest extends SapphireTest { - public static $fixture_file = 'SQLUpdateTest.yml'; protected static $extra_dataobjects = [ SQLUpdateTest\TestBase::class, - SQLUpdateTest\TestChild::class + SQLUpdateTest\TestChild::class, + SQLUpdateTest\TestOther::class, ]; public function testEmptyQueryReturnsNothing() @@ -46,4 +46,25 @@ class SQLUpdateTest extends SapphireTest $item = DataObject::get_one(SQLUpdateTest\TestBase::class, ['"Title"' => 'Object 1']); $this->assertEquals('Description 1a', $item->Description); } + + public function testUpdateWithJoin() + { + $query = SQLUpdate::create() + ->setTable('"SQLUpdateTestBase"') + ->assign('"SQLUpdateTestBase"."Description"', 'Description 2a') + ->addInnerJoin('SQLUpdateTestOther', '"SQLUpdateTestOther"."Description" = "SQLUpdateTestBase"."Description"'); + $sql = $query->sql($parameters); + + // Check SQL + $this->assertSQLEquals('UPDATE "SQLUpdateTestBase" INNER JOIN "SQLUpdateTestOther" ON "SQLUpdateTestOther"."Description" = "SQLUpdateTestBase"."Description" SET "SQLUpdateTestBase"."Description" = ?', $sql); + $this->assertEquals(['Description 2a'], $parameters); + + // Check affected rows + $query->execute(); + $this->assertEquals(1, DB::affected_rows()); + + // Check item updated + $item = DataObject::get_one(SQLUpdateTest\TestBase::class, ['"Title"' => 'Object 2']); + $this->assertEquals('Description 2a', $item->Description); + } } diff --git a/tests/php/ORM/SQLUpdateTest.yml b/tests/php/ORM/SQLUpdateTest.yml index 34b006569..908449b41 100644 --- a/tests/php/ORM/SQLUpdateTest.yml +++ b/tests/php/ORM/SQLUpdateTest.yml @@ -10,3 +10,7 @@ SilverStripe\ORM\Tests\SQLUpdateTest\TestChild: Title: 'Object 3' Description: 'Description 3' Details: 'Details 3' +SilverStripe\ORM\Tests\SQLUpdateTest\TestOther: + test3: + Title: 'Object 2 mirror' + Description: 'Description 2' diff --git a/tests/php/ORM/SQLUpdateTest/TestOther.php b/tests/php/ORM/SQLUpdateTest/TestOther.php new file mode 100644 index 000000000..cf8167536 --- /dev/null +++ b/tests/php/ORM/SQLUpdateTest/TestOther.php @@ -0,0 +1,16 @@ + 'Varchar(255)', + 'Description' => 'Text' + ]; +}