From faab576eb6f744cc650ec9aa1ab5d41f1572b3a2 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@silverstripe.com>
Date: Mon, 18 Mar 2024 13:46:04 +1300
Subject: [PATCH] NEW Allow JOIN with SQL UPDATE.

---
 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 +++++++
 4 files changed, 79 insertions(+), 20 deletions(-)
 create mode 100644 tests/php/ORM/SQLUpdateTest/TestOther.php

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 @@
+<?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'
+    ];
+}