[!!!][FEATURE] Use DTO for record representation and identity (#77)

This internal change does not affect any external APIs, but improves the way record data and identities are handled. This fixes and issue that made multilingual operations impossible, as well as making it easier to work with record data and identities. * `\Pixelant\Interest\Domain\Model\Dto\RecordRepresentation` represents a record state, and includes data as well as a `RecordInstanceIdentifier` object. * `\Pixelant\Interest\Domain\Model\Dto\RecordInstanceIdentifier` represents a record's identity: table, UID, language, workspace, and remote ID. It can output both the remote ID as it is seen from the outside (which is only unique when combined with language and workspace) and the internal, so-called "aspected", remote ID, which is unique for each language and workspace combination. Also fixes the condition for group relations. Was only accepting `internal_type` as "db", but `allowed` can also indicate a database relation. Co-authored-by: Mats Svensson <[email protected]>
FriendsOfTYPO3 · Sep 23, 2022 · 44a99b4 · 44a99b4
1 parent ba19bfc
commit 44a99b4
Show file tree

Hide file tree

Showing 31 changed files with 1,239 additions and 348 deletions.
diff --git a/.ddev/config.yaml b/.ddev/config.yaml
@@ -15,6 +15,7 @@ mutagen_enabled: false
 hooks:
   post-start:
   - exec: composer install --no-progress
+  - exec: export PHP_IDE_CONFIG="serverName=interest.ddev.site"
 omit_containers: [dba, ddev-ssh-agent]
 webimage_extra_packages: [parallel]
 use_dns_when_possible: true

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -152,7 +152,7 @@ jobs:
             composer-dependencies: lowest
   functional-tests:
     name: "Functional tests"
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-20.04
     needs: php-lint
     steps:
       - name: "Checkout"

diff --git a/.github/workflows/codecoverage.yml b/.github/workflows/codecoverage.yml
@@ -8,7 +8,7 @@ on:
 jobs:
   code-coverage:
     name: "Calculate code coverage"
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-20.04
     steps:
       - name: "Checkout"
         uses: actions/checkout@v3

diff --git a/Classes/Command/CreateCommandController.php b/Classes/Command/CreateCommandController.php
@@ -8,6 +8,8 @@
 use Pixelant\Interest\DataHandling\Operation\Event\Exception\StopRecordOperationException;
 use Pixelant\Interest\DataHandling\Operation\Exception\IdentityConflictException;
 use Pixelant\Interest\DataHandling\Operation\UpdateRecordOperation;
+use Pixelant\Interest\Domain\Model\Dto\RecordInstanceIdentifier;
+use Pixelant\Interest\Domain\Model\Dto\RecordRepresentation;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -48,11 +50,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
         foreach ($input->getOption('data') as $remoteId => $data) {
             try {
                 (new CreateRecordOperation(
-                    $data,
-                    $input->getArgument('endpoint'),
-                    $remoteId,
-                    $input->getArgument('language'),
-                    $input->getArgument('workspace'),
+                    new RecordRepresentation(
+                        $data,
+                        new RecordInstanceIdentifier(
+                            $input->getArgument('endpoint'),
+                            $remoteId,
+                            (string)$input->getArgument('language'),
+                            (string)$input->getArgument('workspace'),
+                        )
+                    ),
                     $input->getOption('metaData')
                 ))();
             } catch (StopRecordOperationException $exception) {
@@ -66,11 +72,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
                 try {
                     (new UpdateRecordOperation(
-                        $data,
-                        $input->getArgument('endpoint'),
-                        $remoteId,
-                        $input->getArgument('language'),
-                        $input->getArgument('workspace'),
+                        new RecordRepresentation(
+                            $data,
+                            new RecordInstanceIdentifier(
+                                $input->getArgument('endpoint'),
+                                $remoteId,
+                                (string)$input->getArgument('language'),
+                                (string)$input->getArgument('workspace'),
+                            )
+                        ),
                         $input->getOption('metaData')
                     ))();
                 } catch (StopRecordOperationException $exception) {

diff --git a/Classes/Command/DeleteCommandController.php b/Classes/Command/DeleteCommandController.php
@@ -8,6 +8,9 @@
 use Pixelant\Interest\Database\RelationHandlerWithoutReferenceIndex;
 use Pixelant\Interest\DataHandling\Operation\DeleteRecordOperation;
 use Pixelant\Interest\DataHandling\Operation\Event\Exception\StopRecordOperationException;
+use Pixelant\Interest\Domain\Model\Dto\RecordInstanceIdentifier;
+use Pixelant\Interest\Domain\Model\Dto\RecordRepresentation;
+use Pixelant\Interest\Domain\Repository\RemoteIdMappingRepository;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -75,12 +78,22 @@ protected function execute(InputInterface $input, OutputInterface $output)
     {
         $exceptions = [];
 
+        $mappingRepository = GeneralUtility::makeInstance(RemoteIdMappingRepository::class);
+
         foreach (GeneralUtility::trimExplode(',', $input->getArgument('remoteId'), true) as $remoteId) {
+            $table = $mappingRepository->table($remoteId);
+
             try {
                 (new DeleteRecordOperation(
-                    $remoteId,
-                    $input->getArgument('language'),
-                    $input->getArgument('workspace')
+                    new RecordRepresentation(
+                        [],
+                        new RecordInstanceIdentifier(
+                            $table,
+                            $remoteId,
+                            (string)$input->getArgument('language'),
+                            (string)$input->getArgument('workspace'),
+                        )
+                    )
                 ))();
             } catch (StopRecordOperationException $exception) {
                 $output->writeln($exception->getMessage(), OutputInterface::VERBOSITY_VERY_VERBOSE);

diff --git a/Classes/Command/UpdateCommandController.php b/Classes/Command/UpdateCommandController.php
@@ -8,6 +8,8 @@
 use Pixelant\Interest\DataHandling\Operation\Event\Exception\StopRecordOperationException;
 use Pixelant\Interest\DataHandling\Operation\Exception\NotFoundException;
 use Pixelant\Interest\DataHandling\Operation\UpdateRecordOperation;
+use Pixelant\Interest\Domain\Model\Dto\RecordInstanceIdentifier;
+use Pixelant\Interest\Domain\Model\Dto\RecordRepresentation;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -48,11 +50,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
         foreach ($input->getOption('data') as $remoteId => $data) {
             try {
                 (new UpdateRecordOperation(
-                    $data,
-                    $input->getArgument('endpoint'),
-                    $remoteId,
-                    $input->getArgument('language'),
-                    $input->getArgument('workspace'),
+                    new RecordRepresentation(
+                        $data,
+                        new RecordInstanceIdentifier(
+                            $input->getArgument('endpoint'),
+                            $remoteId,
+                            (string)$input->getArgument('language'),
+                            (string)$input->getArgument('workspace'),
+                        )
+                    ),
                     $input->getOption('metaData')
                 ))();
             } catch (StopRecordOperationException $exception) {
@@ -66,11 +72,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
                 try {
                     (new CreateRecordOperation(
-                        $data,
-                        $input->getArgument('endpoint'),
-                        $remoteId,
-                        $input->getArgument('language'),
-                        $input->getArgument('workspace'),
+                        new RecordRepresentation(
+                            $data,
+                            new RecordInstanceIdentifier(
+                                $input->getArgument('endpoint'),
+                                $remoteId,
+                                (string)$input->getArgument('language'),
+                                (string)$input->getArgument('workspace'),
+                            )
+                        ),
                         $input->getOption('metaData')
                     ))();
                 } catch (StopRecordOperationException $exception) {

diff --git a/Classes/DataHandling/Operation/AbstractRecordOperation.php b/Classes/DataHandling/Operation/AbstractRecordOperation.php
@@ -13,6 +13,7 @@
 use Pixelant\Interest\DataHandling\Operation\Exception\DataHandlerErrorException;
 use Pixelant\Interest\DataHandling\Operation\Exception\InvalidArgumentException;
 use Pixelant\Interest\DataHandling\Operation\Exception\NotFoundException;
+use Pixelant\Interest\Domain\Model\Dto\RecordRepresentation;
 use Pixelant\Interest\Domain\Repository\PendingRelationsRepository;
 use Pixelant\Interest\Domain\Repository\RemoteIdMappingRepository;
 use Pixelant\Interest\Utility\CompatibilityUtility;
@@ -21,7 +22,6 @@
 use Pixelant\Interest\Utility\TcaUtility;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
-use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
@@ -126,36 +126,34 @@ abstract class AbstractRecordOperation
     protected string $hash;
 
     /**
-     * @param array $data
-     * @param string $table
-     * @param string $remoteId
-     * @param string|null $language as RFC 1766/3066 string, e.g. nb or sv-SE.
-     * @param string|null $workspace workspace represented with a remote ID.
+     * @var RecordRepresentation
+     */
+    protected RecordRepresentation $recordRepresentation;
+
+    /**
+     * @param RecordRepresentation $recordRepresentation to perform the operation on.
      * @param array|null $metaData any additional data items not to be persisted but used in processing.
      *
      * @throws StopRecordOperationException is re-thrown from BeforeRecordOperationEvent handlers
      */
     public function __construct(
-        array $data,
-        string $table,
-        string $remoteId,
-        ?string $language = null,
-        ?string $workspace = null,
+        RecordRepresentation $recordRepresentation,
         ?array $metaData = []
     ) {
-        $this->table = strtolower($table);
-        $this->remoteId = $remoteId;
-        $this->data = $data;
+        $this->recordRepresentation = $recordRepresentation;
+        $this->table = strtolower($this->recordRepresentation->getRecordInstanceIdentifier()->getTable());
+        $this->remoteId = $this->recordRepresentation->getRecordInstanceIdentifier()->getRemoteIdWithAspects();
+        $this->data = $this->recordRepresentation->getData();
         $this->metaData = $metaData ?? [];
-        $this->workspace = $workspace;
+        $this->workspace = $this->recordRepresentation->getRecordInstanceIdentifier()->getWorkspace();
 
         $this->configurationProvider = GeneralUtility::makeInstance(ConfigurationProvider::class);
 
         $this->mappingRepository = GeneralUtility::makeInstance(RemoteIdMappingRepository::class);
 
         $this->pendingRelationsRepository = GeneralUtility::makeInstance(PendingRelationsRepository::class);
 
-        $this->language = $this->resolveLanguage((string)$language);
+        $this->language = $this->recordRepresentation->getRecordInstanceIdentifier()->getLanguage();
 
         $this->createTranslationFields();
 
@@ -189,7 +187,7 @@ public function __construct(
         $this->dataHandler = GeneralUtility::makeInstance(DataHandler::class);
         $this->dataHandler->start([], []);
 
-        if (!isset($this->getData()['pid']) && $this instanceof ContentObjectRenderer) {
+        if (!isset($this->getData()['pid']) && $this instanceof CreateRecordOperation) {
             $this->data['pid'] = $this->storagePid;
         }
     }
@@ -272,7 +270,7 @@ public function getArguments(): array
      */
     private function validateFieldNames(): void
     {
-        $fieldsNotInTca = array_diff_key($this->getData(), $GLOBALS['TCA'][$this->getTable()]['columns']) ?? [];
+        $fieldsNotInTca = array_diff_key($this->getData(), $GLOBALS['TCA'][$this->getTable()]['columns'] ?? []);
 
         if (count(array_diff(array_keys($fieldsNotInTca), ['pid'])) > 0) {
             throw new ConflictException(
@@ -313,6 +311,10 @@ private function resolveStoragePid(): int
             $settings['persistence.']['storagePid.'] ?? []
         );
 
+        if (empty($pid)) {
+            $pid = 0;
+        }
+
         if (!MathUtility::canBeInterpretedAsInteger($pid)) {
             throw new InvalidArgumentException(
                 'The PID "' . $pid . '" is invalid and must be an integer.',
@@ -323,63 +325,6 @@ private function resolveStoragePid(): int
         return (int)$pid;
     }
 
-    /**
-     * Resolves a site language. If no language is defined, the sites's default language will be returned. If the
-     * storagePid has no site, null will be returned.
-     *
-     * @param string|null $language
-     * @return SiteLanguage|null
-     * @throws InvalidArgumentException
-     */
-    protected function resolveLanguage(?string $language): ?SiteLanguage
-    {
-        if (!TcaUtility::isLocalizable($this->getTable()) || empty($language)) {
-            return null;
-        }
-
-        /** @var SiteFinder $siteFinder */
-        $siteFinder = GeneralUtility::makeInstance(SiteFinder::class);
-
-        $sites = $siteFinder->getAllSites();
-
-        $siteLanguages = [];
-
-        foreach ($sites as $site) {
-            $siteLanguages = array_merge($siteLanguages, $site->getAllLanguages());
-        }
-
-        // This is the equivalent of running array_unique, but supports objects.
-        $siteLanguages = array_reduce($siteLanguages, function (array $uniqueSiteLanguages, SiteLanguage $item) {
-            /** @var SiteLanguage $siteLanguage */
-            foreach ($uniqueSiteLanguages as $siteLanguage) {
-                if ($siteLanguage->getLanguageId() === $item->getLanguageId()) {
-                    return $uniqueSiteLanguages;
-                }
-            }
-
-            $uniqueSiteLanguages[] = $item;
-
-            return $uniqueSiteLanguages;
-        }, []);
-
-        foreach ($siteLanguages as $siteLanguage) {
-            $hreflang = $siteLanguage->getHreflang();
-
-            // In case this is the short form, e.g. "nb" or "sv", not "nb-NO" or "sv-SE".
-            if (strlen($language) === 2) {
-                $hreflang = substr($hreflang, 0, 2);
-            }
-
-            if (strtolower($hreflang) === strtolower($language)) {
-                return $siteLanguage;
-            }
-        }
-
-        throw new InvalidArgumentException(
-            'The language "' . $language . '" is not defined in this TYPO3 instance.'
-        );
-    }
-
     /**
      * Resolves the UID for the remote ID.
      *
@@ -564,7 +509,10 @@ protected function isRelationalField(string $field): bool
 
         return (
             $tca['type'] === 'group'
-            && $tca['internal_type'] === 'db'
+            && (
+                ($tca['internal_type'] ?? null) === 'db'
+                || isset($tca['allowed'])
+            )
         )
         || (
             in_array($tca['type'], ['inline', 'select'], true)
@@ -598,7 +546,7 @@ protected function isSingleRelationField(string $field): bool
 
         $tca = $this->getTcaFieldConfigurationAndRespectColumnsOverrides($field);
 
-        return $tca['maxitems'] === 1 && empty($tca['foreign_table']);
+        return ($tca['maxitems'] ?? 0) === 1 && empty($tca['foreign_table']);
     }
 
     /**
@@ -847,6 +795,18 @@ protected function reduceFieldSingleValueArrayToScalar(string $fieldName): void
     {
         foreach ($this->data as $fieldName => $fieldValue) {
             if (is_array($fieldValue) && count($fieldValue) <= 1) {
+                if (
+                    $fieldValue === []
+                    && (
+                        $fieldName === TcaUtility::getTranslationSourceField($this->getTable())
+                        || $fieldName === TcaUtility::getTransOrigPointerField($this->getTable())
+                    )
+                ) {
+                    $this->data[$fieldName] = 0;
+
+                    continue;
+                }
+
                 $this->data[$fieldName] = $fieldValue[array_key_first($fieldValue)];
 
                 // Unset empty single-relation fields (1:n) in new records.