Merge pull request #7787 from kenjis/fix-Model-updateBatch

fix: [Model] updateBatch() may generate invalid SQL statement

Author: kenjis

system/BaseModel.php

@@ -979,7 +979,9 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
                 // properties representing the collection elements, we need to grab
                 // them as an array.
                 if (is_object($row) && ! $row instanceof stdClass) {
-                    $row = $this->objectToArray($row, true, true);
+                    // For updates the index field is needed even if it is not changed.
+                    // So set $onlyChanged to false.
+                    $row = $this->objectToArray($row, false, true);
                 }
 
                 // If it's still a stdClass, go ahead and convert to
@@ -997,6 +999,13 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
                 // Save updateIndex for later
                 $updateIndex = $row[$index] ?? null;
 
+                if ($updateIndex === null) {
+                    throw new InvalidArgumentException(
+                        'The index ("' . $index . '") for updateBatch() is missing in the data: '
+                        . json_encode($row)
+                    );
+                }
+
                 // Must be called first so we don't
                 // strip out updated_at values.
                 $row = $this->doProtectFields($row);

tests/system/Models/UpdateModelTest.php

@@ -147,6 +147,27 @@ public function testUpdateBatchSuccess(): void
         ]);
     }
 
+    public function testUpdateBatchInvalidIndex(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'The index ("not_exist") for updateBatch() is missing in the data: {"name":"Derek Jones","country":"Greece"}'
+        );
+
+        $data = [
+            [
+                'name'    => 'Derek Jones',
+                'country' => 'Greece',
+            ],
+            [
+                'name'    => 'Ahmadinejad',
+                'country' => 'Greece',
+            ],
+        ];
+
+        $this->createModel(UserModel::class)->updateBatch($data, 'not_exist');
+    }
+
     public function testUpdateBatchValidationFail(): void
     {
         $data = [
@@ -208,11 +229,16 @@ public function testUpdateBatchWithEntity(): void
         $entity1->name    = 'Jones Martin';
         $entity1->country = 'India';
         $entity1->deleted = 0;
+        $entity1->syncOriginal();
+        // Update the entity.
+        $entity1->country = 'China';
 
+        // This entity is not updated.
         $entity2->id      = 4;
         $entity2->name    = 'Jones Martin';
         $entity2->country = 'India';
         $entity2->deleted = 0;
+        $entity2->syncOriginal();
 
         $this->assertSame(2, $this->createModel(UserModel::class)->updateBatch([$entity1, $entity2], 'id'));
     }