Enhancements required for linkfield migration (silverstripe#11171)

* ENH Add lightweight test override for Environment::isCli() * NEW Allow JOIN with SQL UPDATE.
beerbohmdo · Apr 18, 2024 · 560ec04 · 560ec04
1 parent b9b8c6b
commit 560ec04
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 20 deletions.
diff --git 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 @@ public static function hasEnv(string $name): bool
      */
     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
@@ -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 @@ public function buildDeleteFragment(SQLDelete $query, array &$parameters)
      */
     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 @@ public function buildUpdateFragment(SQLUpdate $query, array &$parameters)
      */
     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 @@ public function buildLimitFragment(SQLSelect $query, array &$parameters)
         }
         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
@@ -12,12 +12,12 @@
  */
 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 @@ public function testBasicUpdate()
         $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
@@ -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
@@ -0,0 +1,16 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\SQLUpdateTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class TestOther extends DataObject implements TestOnly
+{
+    private static $table_name = 'SQLUpdateTestOther';
+
+    private static $db = [
+        'Title' => 'Varchar(255)',
+        'Description' => 'Text'
+    ];
+}