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

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

Closed

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Feb 14, 2017

Alternative approach to #1908
Implements EZP-26885

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.

Note: we actually use eZ\Publish\Core\Persistence\Doctrine\DoctrineConnection which extends Doctrine\DBAL\Connection.

This approach makes FieldHandler and ContentUpdater determine proper persistence Storage Handler for the specific FieldType from the FieldTypeRegistry. Relevant changes are presented in 12ab214.

The difference between this PR and #1908 is that injecting proper persistence Storage Handler into GatewayBasedStorage for each FieldType is no longer required. Sample implementation for ezurl is shown in c176270.

To avoid adding too much responsibility to objects, StorageHandlerRegistry is introduced (along with proper compiler pass) to store actual persistence Storage Handlers (objects implementing \eZ\Publish\SPI\Persistence\Content\StorageHandler).
FieldTypeRegistry gets them "asking" StorageHandlerRegistry for e.g. LegacyStorage or DoctrineStorage.

Note: "Storage handler" refers both to e.g eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler and also to implementations which are obtained by it (like eZ\Publish\Core\FieldType\Url\UrlStorage). In this description I added "persistence" keyword to distinguish the first one.

TODO:

  • Replace usages of Doctrine\DBAL\Connection with eZ\Publish\Core\Persistence\Doctrine\DoctrineConnection to be able to reuse getting Db sequence name if needed and supported by DBMS.
  • Add StorageHandlerRegistry which can register persistence Storage Handlers (and register LegacyStorage to it).
  • Refactor FieldHandler and ContentUpdater to get StorageHandler from FieldTypeRegistry.
  • Move StorageRegistry to eZ\Publish\Core\Persistence\Content\StorageRegistry namespace to reflect it's more common.
  • Add Doctrine persistence StorageHandler (registered to StorageHandlerRegistry).
  • Move TMP commits with sample implementations/usages to other PR if/when this one is accepted.

@alongosz alongosz force-pushed the ezp-26885-v2-ft-doctrine-ext-storage branch from a364c6f to 7e10bd1 Compare April 7, 2017 12:56
@alongosz alongosz changed the title [v2] EZP-26885: As a Developer I want to future proof my Field Types by using Doctrine [in external storage] EZP-26885: As a Developer I want to future proof my Field Types by using Doctrine [in external storage] Apr 7, 2017
@ezrobot
Copy link
Contributor

ezrobot commented Apr 7, 2017

Tool version : PHP CS Fixer 2.1.2 by Fabien Potencier and Dariusz Ruminski
Command executed php-cs-fixer --dry-run --diff -v fix
This Pull Request does not respect PSR-2 Coding Standards, please, see the suggested diff below:

You are running php-cs-fixer with xdebug enabled. This has a major impact on runtime performance.
Loaded config default from "/jenkins/jenkins.std/jobs/PAPI-Pull-Request-code-sniffer/workspace/.php_cs".
Using cache file ".php_cs.cache".
SSSSSS.SSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSS.SSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS..SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSS.SSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSS.SSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSS.SSSS.SSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSS..SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SS.SSSSSSSSSSSSSSSSSSSSSS.S.S.S.SSS.SSSSSS.SSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSS.SSSSSS..SSSSSSSSSSSSSSSSSSSSSSSSSS.S.SSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSS..SSSS..SSSSSSSSSSSSSS.SSSSSSSS.SSSSSSSSSSS.S.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSS..S.SSSSSSSSSS...SSSSSSSSSSS.SSSSS..SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS....S.S.........................SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSFSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.S..SSSSSSSS.SSSSSSSS.S.SSSSSSSS...SSSSSSSSSSSSSSS...S...S..SSSSSSSSSSSSSS.SSSSSS.SSS.SSSSSS.SSSSSSSS.SSSSSSSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS..SSS.SSSSSSSSSSSSSSSS....S.....SS..S...........SSS.SSSSSSSSSSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSS.SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.SSSSSSS
Legend: ?-unknown, I-invalid file syntax, file ignored, S-Skipped, .-no changes, F-fixed, E-error
   1) eZ/Publish/Core/FieldType/Url/UrlStorage/Gateway/DoctrineStorage.php (method_separation)
      ---------- begin diff ----------
--- Original
+++ New
@@ @@
     }
+
     /**
      * Removes link to URL for $fieldId in $versionNo and cleans up possibly orphaned URLs.
      *
      * @param int|string $fieldId
      * @param int $versionNo
      */
     public function unlinkUrl($fieldId, $versionNo)
     {
         $deleteQuery = $this->connection->createQueryBuilder();

         $deleteQuery->delete(
             $this->connection->quoteIdentifier(self::URL_LINK_TABLE)
         )->where(
             $deleteQuery->expr()->andX(
                 $deleteQuery->expr()->in('contentobject_attribute_id', ':contentobject_attribute_id'),
                 $deleteQuery->expr()->in('contentobject_attribute_version', ':contentobject_attribute_version')
             )
         )
             ->setParameter(':contentobject_attribute_id', $fieldId, PDO::PARAM_INT)
             ->setParameter(':contentobject_attribute_version', $versionNo, PDO::PARAM_INT)
         ;

         $deleteQuery->execute();

         $this->deleteOrphanedUrls();
     }

     /**
      * Deletes all orphaned URLs.
      *
      * @todo using two queries because zeta Database does not support joins in delete query.
      * That could be avoided if the feature is implemented there.
      *
      * URL is orphaned if it is not linked to a content attribute through ezurl_object_link table.
      */
     private function deleteOrphanedUrls()
     {
         $query = $this->connection->createQueryBuilder();

         $query
             ->select('url.id')
             ->from($this->connection->quoteIdentifier(self::URL_TABLE), 'url')
             ->leftJoin('url', $this->connection->quoteIdentifier(self(...)

NOTICE : Report truncated to 5Kb

@alongosz
Copy link
Member Author

Closing in favor of #1985

@alongosz alongosz closed this May 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants