!!! FEATURE: update to doctrine/dbal version 3

[dependabot]

update to doctrine/dbal version 3

• doctrines json_array type is deprecated, therefore flow_json_array is based off of json type now
• We use PSR6 caches now instead of the deprecated doctrine cache implementation
• New Cache Flow_Persistence_Doctrine_Metadata for ORM class metadata
• Repository::findAllIterator directly returns an iterable, the Repository::iterate method is gone
• All doctrine migration commands have a new optional migration-folder argument that allows to overwrite the "platform name part" in migration resolving (e.g. "Mysql") as the resolving changed and we cannot be sure deducing it from the current connection will work long term for all cases. Currently MySQL/MariaDB (map to "Mysql"), PostgreSQL (maps to "Postgresql" and SQLite (maps to "Sqlite") all work fine automatically still.

Related Neos adjustments: neos/neos-development-collection#5161

Upgrade instructions

We require now version 3 of doctrine/dbal but still operate doctrine/orm in version 2.
In case you depend on DBAL directly you should have a look into their upgrade instructions: https://www.doctrine-project.org/2020/11/17/dbal-3.0.0.html

[kdambekalns]

It seems something is wrong with the (expected) PhpUnit version, and that ping() call missing an argument looks interesting, too…

[albe]

Ugh, okay, I need to continue this with a clear head at another time - ping() is a method of the AllowedObjectsListener and not the PersistenceManager...

[kdambekalns]

Any news, @albe?

[mhsdesign]

Rebased on 9.0 so this can be continued but i dont actually what our plan is with all this doctrine stuff

// cc @bwaidelich @kitsunet

[kitsunet]

Still many deprecations to tackle though

[kitsunet]

See #3379

[kitsunet]

There is still two open points.
Logging works differently now and I need to create a driver middleware to enable it without deprecation and

the whole Platform->getName() deprecation as we use the name currently to generate the folder name for migrations. We would now have to produce it from the classname which can be more specialised (e.g. \Doctrine\DBAL\Platforms\MariaDb1060Platform) Karsten and me talked about a Settings mapping of platform classes to folder name, alternative would be to try if exists and fallback the whole class hierarchy (ala MariaDb1060Platform -> MariaDb1052Platform -> MariaDb1043Platform -> MariaDb1027Platform -> MariaDBPlatform -> MySQLPlatform -> AbstractMySQLPlatform to show a worst case example) drawback is checking A LOT of folders potentially and also if doctrine changes the class hierarchy that might influence or migrations. So settings seems safer....

[bwaidelich]

the whole Platform->getName() deprecation as we use the name currently to generate the folder name for migrations
a Settings mapping of platform classes to folder name

Mh, another configuration option that might get outdated as new db versions are used.
Couldn't we just hard-code the common cases, and possibly allow the folder name to be specified as fallback:

       $folderName = match (true) {
           $platform instanceof MySQLPlatform => 'Mysql',
           $platform instanceof PostgreSQLPlatform => 'Posgresql',
           $platform instanceof SqlitePlatform => 'Sqlite',
           default => throw new \InvalidArgumentException(sprintf('Failed to determine migration folder name from platform "%s". Please specify --folder-name', get_debug_type($platform))),
       };

see https://3v4l.org/eDuvo#v8.2.7

[kitsunet]

Yep what you hardcoded woudl roughly how I thought about the setting, just those 3, but I would be fine to do it like this with the folder name option. Not that this whole migration system is hugely extensible / will be used elsewhere.

[kitsunet]

Let's go with that IMHO, pragmatic and should cover all cases... I like it, thanks @bwaidelich

[kitsunet]

Alright, migrations down, logging to go...

[kitsunet]

Aaaand logging covered.

[kitsunet]

Alright I call this done for now, technically I have 2 approvals, but stuff has changed since then....

[kdambekalns]

Alright I call this done for now, technically I have 2 approvals, but stuff has changed since then....

I had another look… sorry! 😇

[kitsunet]

Alright I call this done for now, technically I have 2 approvals, but stuff has changed since then....

I had another look… sorry! 😇

No very good, it was late! We definitely used too many different variants of these name comparisons between double and single quotes and strict and loose comparison ^^

[kdambekalns]

We definitely used too many different variants of these name comparisons

I think we all just used what Doctrine generated, and that changed over time… 🤷‍♂️

[mhsdesign]

btw no IterableResult here *g

but i was actually wondering .. what is the benefit here of using this vs findAll? isnt findAll an iterator as well??? I mean we can keep it for b/c ... but i dont know if we need to use it in neos?

[kitsunet]

That depends I guess worth a look at the source, but findAll would return a full array, woudln't it? While this here can be optimized at the driver level to stream the results as single chunks...