Clarify how associative arrays as document values should be updated.

[internalsystemerror]

Assuming the following:

        $this->documentStore->updateDoc(
            $collection,
            $event->get(Payload::USER_ID),
            [
                Payload::USER_VALIDATIONS   => [
                    $event->get(Payload::USER_EMAIL_ADDRESS) => $event->createdAt(),
                ],
            ],
        );

The InMemoryDocumentStore would merge the Payload::USER_VALIDATIONS arrays recursively, adding additional values. This provides no way of removing a nested key. There is even a test to enforce this behaviour: https://github.com/event-engine/php-document-store/blob/1e980c8e8e3f7b48428740e109692440dffa3a2b/tests/InMemoryDocumentStoreTest.php#L88-112

Further, the event-engine/php-postgres-document-store handles this differently, where merges are NOT recursive: https://github.com/event-engine/php-postgres-document-store/blob/e9721e21f42a648a79eb9865b5656c6949bf1c42/src/PostgresDocumentStore.php#L366

For more on how the || operator is used: https://www.postgresql.org/docs/13/functions-json.html

I have submitted the following PRs for changes:

• Add tests to event-engine/php-postgres-document-store to confirm that associative arrays are not merged recursively (tests: confirm that (and how) associative array values are handled in updates php-postgres-document-store#15).
• Modify the tests on this repository, and the behaviour of InMemoryDocumentStore to conform to the behaviour of event-engine/php-postgres-document-store (fix: Mimic how postgres-document-store works, where documents are not merged recursively. #18).

[codeliner]

@sandrokeil @martin-schilling @heiglandreas @jsor @sblum I consider this a bugfix and would release a v0.6.1 on next Tuesday. What do you think? It might effect some test cases, but that's even a good thing, because it won't work with Postgres anyway.

[codeliner]

Thank you for investigation and the PRs @internalsystemerror 👏

[martin-schilling]

Just for completeness sake, I think I still remember the reason arrayReplaceRecursiveAssocOnly was added. In a previous commit, array_merge had been replaced by array_replace_recursive which led to changed behavior and failing tests. The arrayReplaceRecursiveAssocOnly introduced here was a compromise between the two methods. Working with PHP arrays can really be a mess.
Anyway, I'm not entirely sure why we didn't notice that Postgres behaves differently here but I feel like the InMemoryDocumentStore should behave similarly to the PostgresDocumentStore.

Just out of curiosity though, how do other DBs behave? It might be worth checking how this works in other popular DBs like MongoDB to make sure that the most suitable procedure can be chosen. Afterwards this behavior should probably be defined and documented so that we don't have this problem in the future anymore.

Thanks for the PR!

[codeliner]

Good points @martin-schilling . I ask @sandrokeil tomorrow regarding MongoDB.

[codeliner]

MongoDB has a $set operator and it behaves similar to Postgres. I'm going to merge the PRs now.

[codeliner]

Fixed with #18