Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

patchEntity -> isDirty always true for mysql/sqlite json type #18054

Open
kolorafa opened this issue Dec 3, 2024 · 2 comments
Open

patchEntity -> isDirty always true for mysql/sqlite json type #18054

kolorafa opened this issue Dec 3, 2024 · 2 comments

Comments

@kolorafa
Copy link
Contributor

kolorafa commented Dec 3, 2024

Description

Related to #17886

patchEntity -> isDirty always true for mysql/sqlite with json type fields

What I have

Table with jsonTypeField field of type json that is automatically converted to array on read.

    echo $this->Form->control('jsonTypeField.value1');
    echo $this->Form->control('jsonTypeField.value2');
 
    $table->patchEntity(
          $entity,
          $this->request->getData(),
     );

What happens

When doing patchEntity, the value is correctly updated/saved when changed, but jsonTypeField is always marked as dirty.
So even if you don't do any updates, the $entity->isDirty('jsonTypeField') is always true.

What I expected to happen

The patchEntity do a deep array comparison to detect changes because the field value is array and/or because the field type is json.

The expected behavior might need tweaking, especially due to double nature of json tyle field, as you might want to edit each field sep.

What I have done to mitigate this issue locally

Externally override table using trait:

use Cake\Datasource\EntityInterface;

trait HandleArrayWithPatchEntityTrait
{
    /**
     * @param mixed $actual
     * @param mixed $expected
     * @return bool
     */
    private function arrayEqualRecursive(mixed $actual, mixed $expected): bool
    {
        if (!is_array($expected) || !is_array($actual)) {
            return $actual === $expected;
        }
        foreach ($expected as $key => $value) {
            if (!array_key_exists($key, $actual)) {
                return false;
            }
            if (!self::arrayEqualRecursive($value, $actual[$key])) {
                return false;
            }
        }
        foreach ($actual as $key => $value) {
            if (!array_key_exists($key, $expected)) {
                return false;
            }
            if (!self::arrayEqualRecursive($value, $expected[$key])) {
                return false;
            }
        }

        return true;
    }

    /**
     * @param \Cake\Datasource\EntityInterface $entity
     * @param array $data
     * @param array $options
     * @return \Cake\Datasource\EntityInterface
     */
    public function patchEntity(EntityInterface $entity, array $data, array $options = []): EntityInterface
    {
        foreach ($data as $key => $value) {
            // handle recursive comparing arrays, to avoid marking as dirty if equals
            if (is_array($value) && is_array($entity->get($key))) {
                if ($this->arrayEqualRecursive($value, $entity->get($key))) {
                    unset($data[$key]);
                }
            }
        }

        return parent::patchEntity($entity, $data, $options);
    }
}

I probably going to open PR later, but first I would like to open discussion especially what to check for, as I know that we also have relations to other models that that are also represented as arrays. Also that array could be modified directly without patchEntity that in turn would potentially not flag those changes as dirty and not save them.

CakePHP Version

5.1.2

PHP Version

8.3.14

@markstory
Copy link
Member

When doing patchEntity, the value is correctly updated/saved when changed, but jsonTypeField is always marked as dirty.
So even if you don't do any updates, the $entity->isDirty('jsonTypeField') is always true.

Does the request data contain the jsonTypeField data each time? If so it is 'changing' and the ORM is not currently able to look inside arbitrary structured data and locate differences.

I've moved this issue to enhancment as deep field difference detection isn't a current feature.

@kolorafa
Copy link
Contributor Author

kolorafa commented Dec 4, 2024

Does the request data contain the jsonTypeField data each time?

The data is structured mostly the same between rows, but is different in different tables/fields.
Some are 2 level deep (yes, EAV or shadow table strategy could be used on those), some are 10 level deep.

So I was thinking of making it decoded as array for "ease of access" from code, but if used with helpers ten to just encode the whole array as json on frontend. As the json handling is a nice feature to have it automatically json_decoded, it is now a pain to use in forms where you want to show/forward/handle raw json.

I've moved this issue to enhancement as deep field difference detection isn't a current feature

Totally agreed, json type field is already an edge-case in itself :) My thinking was more about - if there would be a room to merge such feature if provided?

Something to brainstorm if people would be interested and discuss of potential caveats :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants