EZP-26885: As a Developer I want to future proof my Field Types by using Doctrine [in external storage]

[alongosz]

Implements EZP-26885
Status: Ready for a review

We want to be able to use \Doctrine\DBAL\Connection for FieldTypes external storage directly.

To achieve this, FieldType external storage needs to be injected with DoctrineStorage gateway which uses Doctrine instead of Legacy Connection Handler.
PR also consists of TMP commits presenting sample implementations, for Url and RichText FieldTypes.

The main architecture issue related to this implementation is the fact that \eZ\Publish\Core\Persistence\Legacy\Content\FieldHandler uses, common for all FieldTypes, Legacy StorageHandler. This means that it's not enough to just register Doctrine gateway for a given FieldType but also FieldHandler must use proper StorageHandler.

To workaround this issue Doctrine StorageHandler is injected directly into DoctrineStorage (GatewayBasedStorage) implementation for a specific field. If GatewayBasedStorage got explicitly injected with StorageHandler, then this one is used instead of the one passed by FieldHandler to get a proper gateway.

However, sometimes gateway is injected directly setting incorrect Connection.
To avoid this, DoctrineStorage Gateway for a FieldType gets injected directly with proper Connection and setConnection is ignored.

This solution is far from perfect (hence the "workaround" keyword appears), so let's discuss improvements.

TODO:

• Create Doctrine StorageHandler for FieldTypes.
• Refactor \eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry to \eZ\Publish\Core\Persistence\Content\StorageRegistry to reflect this is common for storage engines.
• Refactor \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler to \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler to extract common part for all storage engines.
• Override \Doctrine\DBAL\Connection with custom wrapper extending it.
• Discuss solution.
• Move TMP commit to other PR if/when this one is accepted.

[alongosz]

One possible alternative solution to this is to inject into FieldHandler some registry being able to get proper StorageHandler for the given field type (to be used here).

[andrerom]

For the tmp commit can you replace the existing storage gateway while at it so we see the diff? (I assume we don't need the old one anymore in this case as it is part of kernel still)

Or maybe you assume we will support both engines over time as they diverge? Mention that as I didn't quote get why we would move the storage registery out of the storage engine, unless what I wrote in beginning of this paragraph.

[alongosz]

@andrerom

Or maybe you assume we will support both engines over time as they diverge?

Yes, I actually see Doctrine as something on the same level as Legacy, but if that's not the intention I can move it back to existing legacy Yamls.

[alongosz]

I've found out reason for failure on Travis. I guess it must be a different version of pgsql driver used by Doctrine.
What was missing was sequence name for lastInsertId: https://github.com/alongosz/ezpublish-kernel/blob/6ea7c48a218db271d144c8f4bb3897ef8a833146/eZ/Publish/Core/FieldType/Url/UrlStorage/Gateway/DoctrineStorage.php#L137

I've kinda fallen into my own trap there, because on purpose I tried to avoid it, remembering that it worked fine without it (using last sequence changed).
It turns out that it works fine sometimes, by accident, due to driver. PDO doc says sequence name is required for pgsql, so - my bad... :)

This causes further challenges, because we have getSequenceName in eZ\Publish\Core\Persistence\Doctrine\ConnectionHandler and now we want to use Doctrine\DBAL\Connection directly.

I see two possible solutions (the first one partially done):

1. Replace Doctrine\DBAL\Connection with e.g. eZ\Publish\Core\Persistence\Doctrine\Connection and add getSequenceName method.
   I already did it but it requires further refactorings (AppVeyor fail is no accident).
   Sequence naming also needs to be adjusted when merged into 7.0 as I did it in [PostgreSQL] Change sequences naming to conform to SERIAL type #1906 - which makes this solution redundant.
2. Inject into our new DoctrineStorage gateways Legacy DbHandler (eZ\Publish\Core\Persistence\Doctrine\ConnectionHandler) and access Doctrine\DBAL\Connection inside gateway. Then, we can reuse eZ\Publish\Core\Persistence\Doctrine\ConnectionHandler::getSequenceName.

[andrerom]

Preference for 1 here, as the stuff inside Doctrine implements eZ\Publish\Core\Persistence\Database which is an abstraction of Zeta Database which we here aim to start to remove the need for.

[alongosz]

This is sort of ready for a review, but I'm working on a better way to set proper Storage Handler avoiding this workaround. I think it will be better to make another PR than overwrite this one.

The main concept in the alternative approach is to get in FieldHandler (e.g. here) the proper Ext. Storage Handler via FieldTypeRegistry instead of injecting the Legacy one.

In this solution, for now, I'm keeping TMP commits with actual Storage implementations for specific Field Types.
If you prefer squashed fixups, let me know.

// ping @andrerom

[andrerom]

Can we type hint against the DBAL class? (same for use statements, and for Url below)
Afaik our own here is implementation detail, as it looks like we can optionally remove it once we are back to standard pgsql sequences right?

[alongosz]

Closing in favor of #1911