Validation in patchEntity happens for fields excluded for proessing with 'fields' option.

[menshutin]

Description

According to documentation we can skip some properties from being assigned with the help of 'fields' option.

But Marshaller:merge() is calling validate for all data posted, including attributes that are supposed to be excluded. If this excluded attributes hold the wrong data (for some reason), than validation will result in error and data will not be saved into database.

Instead, we should validate only required (not excluded) fields, or, alternatively, suppress validation errors for excluded fields.

CakePHP Version

5.0

PHP Version

8.1

[dereuromark]

I agree with you. Did you manage to track down the issue? And maybe provide a fix as PR?

[dereuromark]

The main issue seems to be that the marshaller first validates on the whole dataset, then looks into fields config

        $entity->setErrors($errors);
        if (!isset($options['fields'])) {

I would also expect the dataset to be reduced to the fields given, if extra fields are needed for custom validations, they need to be also included in that list IMO.

To keep things BC, maybe we could add a flag to enable this strictness - or add "onlyFields" config key for now?

[dereuromark]

One option could be

Index: src/ORM/Marshaller.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php
--- a/src/ORM/Marshaller.php	(revision 4b8915cf32949cac5c798af2ff65f688c4d76d21)
+++ b/src/ORM/Marshaller.php	(date 1744566492371)
@@ -286,13 +286,21 @@
      */
     protected function _prepareDataAndOptions(array $data, array $options): array
     {
-        $options += ['validate' => true];
+        $options += ['validate' => true, 'onlyFields' => false];
 
         $tableName = $this->_table->getAlias();
         if (isset($data[$tableName]) && is_array($data[$tableName])) {
             $data += $data[$tableName];
             unset($data[$tableName]);
         }
+        if ($options['onlyFields'] && !empty($options['fields'])) {
+            $resultData = [];
+            foreach ((array)$options['fields'] as $field) {
+                $resultData[$field] = $data[$field] ?? null;
+            }
+
+            $data = $resultData;
+        }
 
         $data = new ArrayObject($data);
         $options = new ArrayObject($options);
Index: tests/TestCase/ORM/MarshallerTest.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php
--- a/tests/TestCase/ORM/MarshallerTest.php	(revision 4b8915cf32949cac5c798af2ff65f688c4d76d21)
+++ b/tests/TestCase/ORM/MarshallerTest.php	(date 1744566381720)
@@ -2668,6 +2668,61 @@
         $this->assertFalse($entity->isAccessible('*'));
     }
 
+    /**
+     * Tests that it is possible to pass a strict fields option to the merge method
+     */
+    public function testMergeWithFieldsStrict(): void
+    {
+        $this->articles->getValidator()
+            ->requirePresence('title')
+            ->notEmptyString('title');
+
+        $data = [
+            'title' => null,
+            'body' => 'My body',
+            'author_id' => 1,
+        ];
+        $marshall = new Marshaller($this->articles);
+
+        $entity = new Entity([
+            'title' => 'Foo',
+            'body' => 'My content',
+            'author_id' => 2,
+        ]);
+
+        $entity->setAccess('*', false);
+        $entity->setNew(false);
+        $entity->clean();
+        $result = $marshall->merge($entity, $data, ['fields' => ['body']]);
+
+        $expected = [
+            'title' => 'Foo',
+            'body' => 'My body',
+            'author_id' => 2,
+        ];
+
+        $this->assertSame($entity, $result);
+        $this->assertEquals($expected, $result->toArray());
+        $this->assertFalse($entity->isAccessible('*'));
+        $this->assertNotEmpty($entity->getErrors());
+
+        $entity = new Entity([
+            'title' => 'Foo',
+            'body' => 'My content',
+            'author_id' => 2,
+        ]);
+
+        $entity->setAccess('*', false);
+        $entity->setNew(false);
+        $entity->clean();
+        $result = $marshall->merge($entity, $data, ['fields' => ['body'], 'onlyFields' => true]);
+
+        $this->assertSame($entity, $result);
+        $this->assertEquals($expected, $result->toArray());
+        $this->assertFalse($entity->isAccessible('*'));
+        $this->assertEmpty($entity->getErrors());
+    }
+
     /**
      * Test that many() also receives a fields option
      */

But the test reveals that requirePresence is so strong it would then still fire. We would need to stop using requirePresence completely for validation rules here.

Instead, I think, we should ignore all validation errors to fields that are not in the list. This way we dont need to mess with the data array itself I assume.

[dereuromark]

cc @menshutin