From cd4a0e0d00a32cebbdd8c83440667aec0addb2c1 Mon Sep 17 00:00:00 2001 From: Krystian Szymukowicz Date: Mon, 2 Nov 2026 21:51:51 +0100 Subject: [PATCH 1/3] [!!!][TASK] Restrict assignment of existing records of inline relations --- .../Handler/AbstractDomainObjectHandler.php | 25 ++- .../Handler/FileReferenceHandler.php | 24 ++- Classes/Service/InlineRelationService.php | 128 ++++++++++++++ Configuration/Services.yaml | 3 + .../HandlingCascadePersistence/Index.rst | 12 ++ Documentation/HandlingFileUpload/Index.rst | 4 +- .../Miscellaneous/Changelog/Index.rst | 1 + Resources/Private/Language/locallang.xlf | 3 + ...InlineRelationAssignmentDispatcherTest.php | 158 ++++++++++++++++++ .../Classes/Domain/Model/FileReference.php | 10 ++ .../Classes/Domain/Model/Recipe.php | 121 ++++++++++++++ .../Classes/Domain/Model/RecipeStep.php | 22 +++ .../Extbase/Persistence/Classes.php | 4 + .../tx_functionaltest_domain_model_recipe.php | 31 ++++ ...functionaltest_domain_model_recipestep.php | 21 +++ .../Serializer/Domain.Model.Recipe.yml | 10 ++ .../t3api_functional_test/ext_tables.sql | 11 ++ 17 files changed, 585 insertions(+), 3 deletions(-) create mode 100644 Classes/Service/InlineRelationService.php create mode 100644 Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/FileReference.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Recipe.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/RecipeStep.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipestep.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml diff --git a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php index c2db52d5..70b3a9fd 100644 --- a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php +++ b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php @@ -9,8 +9,10 @@ use JMS\Serializer\Visitor\DeserializationVisitorInterface; use SourceBroker\T3api\Annotation\ORM\Cascade; use SourceBroker\T3api\Exception\BadRequestException; +use SourceBroker\T3api\Service\InlineRelationService; use SourceBroker\T3api\Service\PropertyInfoService; use SourceBroker\T3api\Service\SerializerService; +use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject; use TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager; use TYPO3\CMS\Extbase\Persistence\ObjectStorage; @@ -28,6 +30,22 @@ public function __construct( protected readonly SerializerService $serializerService ) {} + protected ?InlineRelationService $inlineRelationService = null; + + /** + * Injected by method (not constructor) so that adding new dependency does not break handlers which extend this + * class and override the constructor. + */ + public function injectInlineRelationService(InlineRelationService $inlineRelationService): void + { + $this->inlineRelationService = $inlineRelationService; + } + + protected function getInlineRelationService(): InlineRelationService + { + return $this->inlineRelationService ??= GeneralUtility::makeInstance(InlineRelationService::class); + } + /** * @var string[] */ @@ -51,11 +69,16 @@ public function deserialize( $targetObjectType = $type['params']['targetType']; if (is_numeric($data)) { - return $this->persistenceManager->getObjectByIdentifier( + $object = $this->persistenceManager->getObjectByIdentifier( (int)$data, $targetObjectType, false ); + if ($object instanceof AbstractDomainObject) { + $this->getInlineRelationService()->assertAssignable($object, $context); + } + + return $object; } if (is_array($data)) { diff --git a/Classes/Serializer/Handler/FileReferenceHandler.php b/Classes/Serializer/Handler/FileReferenceHandler.php index ca1fed66..dac8eb10 100644 --- a/Classes/Serializer/Handler/FileReferenceHandler.php +++ b/Classes/Serializer/Handler/FileReferenceHandler.php @@ -11,6 +11,7 @@ use JMS\Serializer\Visitor\SerializationVisitorInterface; use SourceBroker\T3api\Exception\ValidationException; use SourceBroker\T3api\Service\FileReferenceService; +use SourceBroker\T3api\Service\InlineRelationService; use SourceBroker\T3api\Service\SerializerService; use SourceBroker\T3api\Service\UrlService; use TYPO3\CMS\Core\Resource\FileReference as Typo3FileReference; @@ -48,6 +49,22 @@ public function __construct( protected readonly ContentObjectRenderer $contentObjectRenderer ) {} + protected ?InlineRelationService $inlineRelationService = null; + + /** + * Injected by method (not constructor) so that adding new dependency does not break handlers which extend this + * class and override the constructor. + */ + public function injectInlineRelationService(InlineRelationService $inlineRelationService): void + { + $this->inlineRelationService = $inlineRelationService; + } + + protected function getInlineRelationService(): InlineRelationService + { + return $this->inlineRelationService ??= GeneralUtility::makeInstance(InlineRelationService::class); + } + /** * @param ExtbaseFileReference|Typo3FileReference $fileReference * @@ -155,11 +172,16 @@ public function deserialize( $uid = (int)(is_numeric($data) ? $data : $data['uid']); if ($uid > 0) { - return $this->persistenceManager->getObjectByIdentifier( + $fileReference = $this->persistenceManager->getObjectByIdentifier( $uid, ExtbaseFileReference::class, false ); + if ($fileReference instanceof AbstractDomainObject) { + $this->getInlineRelationService()->assertAssignable($fileReference, $context); + } + + return $fileReference; } return null; } diff --git a/Classes/Service/InlineRelationService.php b/Classes/Service/InlineRelationService.php new file mode 100644 index 00000000..ca1dafdd --- /dev/null +++ b/Classes/Service/InlineRelationService.php @@ -0,0 +1,128 @@ +getVisitor(); + $parent = method_exists($visitor, 'getCurrentObject') ? $visitor->getCurrentObject() : null; + $metadataStack = $context->getMetadataStack(); + $propertyMetadata = $metadataStack->isEmpty() ? null : $metadataStack->top(); + + if ( + $parent instanceof AbstractDomainObject + && $propertyMetadata instanceof PropertyMetadata + && !$this->isAssignable($parent, $propertyMetadata->name, $record) + ) { + throw new BadRequestException( + 'relation_not_assignable', + [(string)$record->getUid(), implode('.', $context->getCurrentPath())], + 1793660001 + ); + } + } + + public function isAssignable(AbstractDomainObject $parent, string $propertyName, AbstractDomainObject $record): bool + { + $columnMap = $this->dataMapper->getDataMap(get_class($parent))->getColumnMap($propertyName); + if ($columnMap === null || !$this->isStoredInRelatedTable($columnMap)) { + return true; + } + + $parentKeyField = (string)$columnMap->getParentKeyFieldName(); + $parentTableField = (string)$columnMap->getParentTableFieldName(); + $matchFields = $columnMap->getRelationTableMatchFields() ?? []; + + $row = $this->fetchParentFields( + (string)$columnMap->getChildTableName(), + (int)$record->getUid(), + array_filter([$parentKeyField, $parentTableField, ...array_keys($matchFields)]) + ); + if ($row === null) { + return false; + } + + if ((int)$row[$parentKeyField] === 0) { + return true; + } + + // Same value as written by Extbase persistence - a translated parent is stored with its localized UID. + $parentUid = (int)($parent->_getProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID) ?: $parent->getUid()); + if ($parentUid === 0 || (int)$row[$parentKeyField] !== $parentUid) { + return false; + } + + if ( + $parentTableField !== '' + && (string)$row[$parentTableField] !== $this->dataMapper->getDataMap(get_class($parent))->getTableName() + ) { + return false; + } + + foreach ($matchFields as $field => $value) { + if ((string)$row[$field] !== (string)$value) { + return false; + } + } + + return true; + } + + private function isStoredInRelatedTable(ColumnMap $columnMap): bool + { + return in_array($columnMap->getTypeOfRelation(), [Relation::HAS_ONE, Relation::HAS_MANY], true) + && (string)$columnMap->getParentKeyFieldName() !== '' + && (string)$columnMap->getChildTableName() !== ''; + } + + /** + * @param string[] $fields + * @return array|null + */ + private function fetchParentFields(string $table, int $uid, array $fields): ?array + { + $queryBuilder = $this->connectionPool->getQueryBuilderForTable($table); + // Hidden records and records not visible in the current context still belong to their parent. + $queryBuilder->getRestrictions()->removeAll()->add(new DeletedRestriction()); + + $row = $queryBuilder + ->select(...$fields) + ->from($table) + ->where($queryBuilder->expr()->eq('uid', $queryBuilder->createNamedParameter($uid, Connection::PARAM_INT))) + ->executeQuery() + ->fetchAssociative(); + + return $row === false ? null : $row; + } +} diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml index 0ea4a2eb..3d88aa1b 100644 --- a/Configuration/Services.yaml +++ b/Configuration/Services.yaml @@ -72,6 +72,9 @@ services: SourceBroker\T3api\Serializer\Construction\ExtbaseObjectConstructor: public: true + SourceBroker\T3api\Service\InlineRelationService: + public: true + SourceBroker\T3api\Serializer\Handler\AbstractDomainObjectHandler: public: true diff --git a/Documentation/HandlingCascadePersistence/Index.rst b/Documentation/HandlingCascadePersistence/Index.rst index df26a1bb..d0db1c6c 100644 --- a/Documentation/HandlingCascadePersistence/Index.rst +++ b/Documentation/HandlingCascadePersistence/Index.rst @@ -27,6 +27,18 @@ In similar way it works also for `One-To-Many` and `Many-To-Many` relations for ] } +.. _handling_cascade_persistence_inline_relations: + +Relations stored in the related table +-------------------------------------- + +Records of relations stored in the related table - TCA ``inline`` with ``foreign_field`` and TCA ``file`` +(file references) - keep their parent record in their own row, so they can belong to one parent only. +An existing record of such relation can be passed by ``uid`` only when it does not belong to any record yet, +or when it already belongs to the same property of the same record (e.g. sending the current items again in +``PATCH`` or ``PUT``). Otherwise the response is ``400 Bad Request``. To add new items, create them using +cascade persistence described below. + Cascade persistence - creating related entity in single request ================================================================ diff --git a/Documentation/HandlingFileUpload/Index.rst b/Documentation/HandlingFileUpload/Index.rst index 4a945d71..00f1b971 100644 --- a/Documentation/HandlingFileUpload/Index.rst +++ b/Documentation/HandlingFileUpload/Index.rst @@ -141,7 +141,9 @@ Save reference to new file .. important:: It is not (yet) possible to update existing file reference within T3api request - it is possible only to create - new reference. + new reference. An existing file reference can be passed by ``uid`` only when it already belongs to the same + property of the same record or does not belong to any record, see + :ref:`handling_cascade_persistence_inline_relations`. .. code-block:: json diff --git a/Documentation/Miscellaneous/Changelog/Index.rst b/Documentation/Miscellaneous/Changelog/Index.rst index dd91d2ec..6416584b 100644 --- a/Documentation/Miscellaneous/Changelog/Index.rst +++ b/Documentation/Miscellaneous/Changelog/Index.rst @@ -6,6 +6,7 @@ Changelog 5.0.0 ===== +- [!!!] Existing records of relations stored in the related table (TCA ``inline`` with ``foreign_field``, TCA ``file``) can be assigned by ``uid`` only when they do not belong to any record or already belong to the same property of the same record, otherwise ``400 Bad Request`` is returned. See :ref:`handling_cascade_persistence_inline_relations`. - Keep properties missing in the request body when an existing record is updated (``PATCH``, cascade update of a related record). Newer versions of JMS serializer set the declared default value of such properties (e.g. ``protected string $title = ''``), so all properties which were not sent were reset. - Fix response cache identifier for ``identifierExpressions`` returning an array (e.g. ``frontend.user.userGroupIds``) - array items are part of the identifier now. Tag expressions returning an array add a tag per item. See :ref:`response-cache`. - [!!!] Validate origin and content type of requests changing data: requests from another origin not allowed by CORS ``allowOrigin`` return ``403 Forbidden``, JSON body without JSON ``Content-Type`` returns ``415 Unsupported Media Type``. Add origins of frontend applications to ``allowOrigin`` and send ``Content-Type: application/json``. See :ref:`cors_write_requests`. diff --git a/Resources/Private/Language/locallang.xlf b/Resources/Private/Language/locallang.xlf index 3f94b3b6..63670e14 100644 --- a/Resources/Private/Language/locallang.xlf +++ b/Resources/Private/Language/locallang.xlf @@ -60,6 +60,9 @@ Value of type `%s` in path `%s` has to be an array + + Record `%s` in path `%s` belongs to another record and can not be assigned + Unknown order direction `%s`, expected `asc` or `desc` diff --git a/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php b/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php new file mode 100644 index 00000000..15c0c086 --- /dev/null +++ b/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php @@ -0,0 +1,158 @@ +getConnectionPool()->getConnectionForTable(self::RECIPE_TABLE); + $recipes->insert(self::RECIPE_TABLE, ['uid' => 1, 'pid' => 0, 'title' => 'First recipe', 'steps' => 1, 'images' => 1]); + $recipes->insert(self::RECIPE_TABLE, ['uid' => 2, 'pid' => 0, 'title' => 'Second recipe', 'steps' => 1, 'images' => 1]); + + $steps = $this->getConnectionPool()->getConnectionForTable(self::STEP_TABLE); + $steps->insert(self::STEP_TABLE, ['uid' => 1, 'pid' => 0, 'title' => 'Step of first recipe', 'recipe' => 1]); + $steps->insert(self::STEP_TABLE, ['uid' => 2, 'pid' => 0, 'title' => 'Step of second recipe', 'recipe' => 2]); + $steps->insert(self::STEP_TABLE, ['uid' => 3, 'pid' => 0, 'title' => 'Step without recipe', 'recipe' => 0]); + + $references = $this->getConnectionPool()->getConnectionForTable('sys_file_reference'); + foreach ([ + [11, 1, self::RECIPE_TABLE, 'images'], + [12, 2, self::RECIPE_TABLE, 'images'], + [13, 1, 'tt_content', 'image'], + [14, 1, self::RECIPE_TABLE, 'other'], + [15, 0, '', ''], + ] as [$uid, $uidForeign, $tableNames, $fieldName]) { + $references->insert('sys_file_reference', [ + 'uid' => $uid, + 'pid' => 0, + 'uid_local' => 1, + 'uid_foreign' => $uidForeign, + 'tablenames' => $tableNames, + 'fieldname' => $fieldName, + ]); + } + } + + #[Test] + #[DataProvider('assignableRecordProvider')] + public function recordWithoutParentOrOfSameParentPropertyIsAssigned(string $property, int $uid, string $table, string $parentField): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', [$property => [$uid]]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(1, $this->fetchField($table, $uid, $parentField)); + } + + public static function assignableRecordProvider(): array + { + return [ + 'inline record of same parent' => ['steps', 1, self::STEP_TABLE, 'recipe'], + 'inline record without parent' => ['steps', 3, self::STEP_TABLE, 'recipe'], + 'file reference of same parent property' => ['images', 11, 'sys_file_reference', 'uid_foreign'], + 'file reference without parent' => ['images', 15, 'sys_file_reference', 'uid_foreign'], + ]; + } + + #[Test] + #[DataProvider('notAssignableRecordProvider')] + public function recordOfOtherParentIsRejected(string $property, int $uid, string $table, string $parentField, int $parentUid): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', [$property => [$uid]]); + + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + self::assertSame(400, $response->getStatusCode(), $body); + self::assertSame( + sprintf('Record `%d` in path `%s` belongs to another record and can not be assigned', $uid, $property), + $json['hydra:description'] + ); + self::assertSame($parentUid, $this->fetchField($table, $uid, $parentField)); + } + + public static function notAssignableRecordProvider(): array + { + return [ + 'inline record of other parent' => ['steps', 2, self::STEP_TABLE, 'recipe', 2], + 'file reference of other parent' => ['images', 12, 'sys_file_reference', 'uid_foreign', 2], + 'file reference of record with same UID in other table' => ['images', 13, 'sys_file_reference', 'uid_foreign', 1], + 'file reference of other property of same parent' => ['images', 14, 'sys_file_reference', 'uid_foreign', 1], + ]; + } + + #[Test] + public function putKeepsInlineRecordsOfSameParent(): void + { + $response = $this->dispatch('PUT', '/_api/recipes/1', ['title' => 'Replaced', 'steps' => [1]]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(1, $this->fetchField(self::STEP_TABLE, 1, 'recipe')); + } + + #[Test] + public function newRecordAcceptsOnlyInlineRecordsWithoutParent(): void + { + $rejected = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [1]]); + + self::assertSame(400, $rejected->getStatusCode(), (string)$rejected->getBody()); + self::assertSame(1, $this->fetchField(self::STEP_TABLE, 1, 'recipe')); + + $created = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [3]]); + $json = json_decode((string)$created->getBody(), true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(201, $created->getStatusCode(), (string)$created->getBody()); + self::assertSame($json['uid'], $this->fetchField(self::STEP_TABLE, 3, 'recipe')); + self::assertSame(3, $this->getConnectionPool()->getConnectionForTable(self::RECIPE_TABLE)->count('*', self::RECIPE_TABLE, [])); + } + + private function fetchField(string $table, int $uid, string $field): int + { + return (int)$this->getConnectionPool()->getConnectionForTable($table) + ->select([$field], $table, ['uid' => $uid])->fetchOne(); + } + + private function dispatch(string $method, string $path, array $body): ResponseInterface + { + $stream = new Stream('php://temp', 'rw'); + $stream->write(json_encode($body, JSON_THROW_ON_ERROR)); + $stream->rewind(); + + $request = (new ServerRequest('https://example.com' . $path, $method, $stream)) + ->withHeader('Content-Type', 'application/json'); + + $dispatcher = new Bootstrap( + $this->get(SerializerService::class), + $this->get(ApiResourceRepository::class), + $this->get(SerializationContextBuilder::class), + $this->get(DeserializationContextBuilder::class), + $this->get(EventDispatcherInterface::class), + $this->get(OperationResponseCache::class) + ); + + return $dispatcher->process($request); + } +} diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/FileReference.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/FileReference.php new file mode 100644 index 00000000..95b323d9 --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/FileReference.php @@ -0,0 +1,10 @@ + + */ + protected ObjectStorage $steps; + + /** + * @var ObjectStorage + */ + protected ObjectStorage $images; + + public function __construct() + { + $this->steps = new ObjectStorage(); + $this->images = new ObjectStorage(); + } + + public function getTitle(): string + { + return $this->title; + } + + public function setTitle(string $title): void + { + $this->title = $title; + } + + /** + * @return ObjectStorage + */ + public function getSteps(): ObjectStorage + { + return $this->steps; + } + + /** + * @param ObjectStorage $steps + */ + public function setSteps(ObjectStorage $steps): void + { + $this->steps = $steps; + } + + /** + * @return ObjectStorage + */ + public function getImages(): ObjectStorage + { + return $this->images; + } + + /** + * @param ObjectStorage $images + */ + public function setImages(ObjectStorage $images): void + { + $this->images = $images; + } +} diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/RecipeStep.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/RecipeStep.php new file mode 100644 index 00000000..9dd18bbb --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/RecipeStep.php @@ -0,0 +1,22 @@ +title; + } + + public function setTitle(string $title): void + { + $this->title = $title; + } +} diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/Extbase/Persistence/Classes.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/Extbase/Persistence/Classes.php index b2fb85dd..1a3dde64 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/Extbase/Persistence/Classes.php +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/Extbase/Persistence/Classes.php @@ -3,6 +3,7 @@ declare(strict_types=1); use T3apiTests\FunctionalTest\Domain\Model\CategoryResource; +use T3apiTests\FunctionalTest\Domain\Model\FileReference; use T3apiTests\FunctionalTest\Domain\Model\TagResource; return [ @@ -12,4 +13,7 @@ TagResource::class => [ 'tableName' => 'tx_functionaltest_domain_model_tag', ], + FileReference::class => [ + 'tableName' => 'sys_file_reference', + ], ]; diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php new file mode 100644 index 00000000..f00a2c06 --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php @@ -0,0 +1,31 @@ + [ + 'label' => 'Title', + 'config' => [ + 'type' => 'input', + ], + ], + 'steps' => [ + 'label' => 'Steps', + 'config' => [ + 'type' => 'inline', + 'foreign_table' => 'tx_functionaltest_domain_model_recipestep', + 'foreign_field' => 'recipe', + ], + ], + 'images' => [ + 'label' => 'Images', + 'config' => [ + 'type' => 'file', + 'allowed' => 'common-image-types', + ], + ], + ], + 'title, steps, images' +); diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipestep.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipestep.php new file mode 100644 index 00000000..be570564 --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipestep.php @@ -0,0 +1,21 @@ + [ + 'label' => 'Title', + 'config' => [ + 'type' => 'input', + ], + ], + 'recipe' => [ + 'config' => [ + 'type' => 'passthrough', + ], + ], + ], + 'title' +); diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml new file mode 100644 index 00000000..b4bef49a --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml @@ -0,0 +1,10 @@ +T3apiTests\FunctionalTest\Domain\Model\Recipe: + properties: + uid: + groups: [api_recipe] + title: + groups: [api_recipe, api_recipe_write] + steps: + groups: [api_recipe_write] + images: + groups: [api_recipe_write] diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql index 6817b949..4ae5d296 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql @@ -34,3 +34,14 @@ CREATE TABLE tx_functionaltest_product_tag_mm ( CREATE TABLE tx_functionaltest_domain_model_article ( title varchar(255) DEFAULT '' NOT NULL ); + +CREATE TABLE tx_functionaltest_domain_model_recipe ( + title varchar(255) DEFAULT '' NOT NULL, + steps int(11) unsigned DEFAULT '0' NOT NULL, + images int(11) unsigned DEFAULT '0' NOT NULL +); + +CREATE TABLE tx_functionaltest_domain_model_recipestep ( + title varchar(255) DEFAULT '' NOT NULL, + recipe int(11) unsigned DEFAULT '0' NOT NULL +); From 9d3e48a2f092fafce821c7832b3b4f278179b374 Mon Sep 17 00:00:00 2001 From: Krystian Szymukowicz Date: Tue, 3 Nov 2026 11:13:41 +0100 Subject: [PATCH 2/3] [TASK] Accept only records of the same parent in inline relations --- .../Handler/AbstractDomainObjectHandler.php | 100 +++---- .../Handler/FileReferenceHandler.php | 36 ++- .../Handler/InlineRelationServiceTrait.php | 30 ++ .../Handler/ObjectStorageHandler.php | 8 + Classes/Service/InlineRelationService.php | 111 ++++--- .../HandlingCascadePersistence/Index.rst | 54 +++- Documentation/HandlingFileUpload/Index.rst | 3 +- .../Miscellaneous/Changelog/Index.rst | 2 +- Resources/Private/Language/locallang.xlf | 8 +- ...InlineRelationAssignmentDispatcherTest.php | 270 +++++++++++++++++- .../Classes/Domain/Model/Category.php | 5 + .../Classes/Domain/Model/Recipe.php | 52 ++++ .../Classes/Domain/Model/Tag.php | 5 + .../tx_functionaltest_domain_model_recipe.php | 28 +- .../Serializer/Domain.Model.Category.yml | 4 + .../Serializer/Domain.Model.Recipe.yml | 8 + .../Serializer/Domain.Model.RecipeStep.yml | 4 + .../Private/Serializer/Domain.Model.Tag.yml | 4 + .../t3api_functional_test/ext_tables.sql | 13 + .../Service/InlineRelationServiceTest.php | 93 ++++++ 20 files changed, 712 insertions(+), 126 deletions(-) create mode 100644 Classes/Serializer/Handler/InlineRelationServiceTrait.php create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Category.yml create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.RecipeStep.yml create mode 100644 Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Tag.yml create mode 100644 Tests/Functional/Service/InlineRelationServiceTest.php diff --git a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php index 70b3a9fd..b180dbf7 100644 --- a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php +++ b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php @@ -9,17 +9,17 @@ use JMS\Serializer\Visitor\DeserializationVisitorInterface; use SourceBroker\T3api\Annotation\ORM\Cascade; use SourceBroker\T3api\Exception\BadRequestException; -use SourceBroker\T3api\Service\InlineRelationService; use SourceBroker\T3api\Service\PropertyInfoService; use SourceBroker\T3api\Service\SerializerService; -use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject; +use TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy; use TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager; use TYPO3\CMS\Extbase\Persistence\ObjectStorage; -use TYPO3\CMS\Extbase\Reflection\ObjectAccess; class AbstractDomainObjectHandler extends AbstractHandler implements DeserializeHandlerInterface { + use InlineRelationServiceTrait; + /** * @var string */ @@ -30,22 +30,6 @@ public function __construct( protected readonly SerializerService $serializerService ) {} - protected ?InlineRelationService $inlineRelationService = null; - - /** - * Injected by method (not constructor) so that adding new dependency does not break handlers which extend this - * class and override the constructor. - */ - public function injectInlineRelationService(InlineRelationService $inlineRelationService): void - { - $this->inlineRelationService = $inlineRelationService; - } - - protected function getInlineRelationService(): InlineRelationService - { - return $this->inlineRelationService ??= GeneralUtility::makeInstance(InlineRelationService::class); - } - /** * @var string[] */ @@ -69,14 +53,24 @@ public function deserialize( $targetObjectType = $type['params']['targetType']; if (is_numeric($data)) { + // `0` clears a single relation (in a collection it is rejected by `ObjectStorageHandler`). + if ((int)$data === 0) { + return null; + } + $object = $this->persistenceManager->getObjectByIdentifier( (int)$data, $targetObjectType, false ); - if ($object instanceof AbstractDomainObject) { - $this->getInlineRelationService()->assertAssignable($object, $context); + if (!$object instanceof AbstractDomainObject) { + throw new BadRequestException( + 'relation_not_found', + [(string)$data, implode('.', $context->getCurrentPath())], + 1793670001 + ); } + $this->getInlineRelationService()->assertAssignable($object, $context, 1793660001); return $object; } @@ -126,7 +120,8 @@ protected function processCascadePersistence( ); } - if (isset($data['uid'])) { + // UID `0` is a new record as well, same as in `FileReferenceHandler`. + if (!empty($data['uid'])) { return $this->processCascadeUpdate( (int)$data['uid'], $targetObjectType, @@ -164,24 +159,18 @@ private function processCascadeUpdate( ) { $object = $this->persistenceManager->getObjectByIdentifier($uid, $targetObjectType, false); - if ($object === null) { - throw new \InvalidArgumentException( - sprintf( - 'Path `%s`: Entity of type `%s` with UID `%s` could not be found', - implode('.', $context->getCurrentPath()), - $targetObjectType, - $uid - ), + if (!$object instanceof AbstractDomainObject) { + throw new BadRequestException( + 'relation_not_found', + [(string)$uid, implode('.', $context->getCurrentPath())], 1588278656829 ); } if (!$this->isObjectInContextScope($context, $propertyMetadata, $object)) { - throw new \InvalidArgumentException( - sprintf( - 'Object in path `%s` is out of scope. You can not update objects which are not related already. To create new object pass it without `uid` property.', - implode('.', $context->getCurrentPath()) - ), + throw new BadRequestException( + 'relation_not_assignable', + [(string)$uid, implode('.', $context->getCurrentPath())], 1589811402076 ); } @@ -195,33 +184,44 @@ private function processCascadeUpdate( ); } + /** + * Relations stored in the related table are checked by `InlineRelationService`. Other relations are compared with + * the state loaded from the database (clean state) - the current value may be already reset (PUT). + */ private function isObjectInContextScope( DeserializationContext $context, PropertyMetadata $propertyMetadata, - $object + AbstractDomainObject $object ): bool { - $parentObject = $context->getAttribute('target'); + $parentObject = $this->getInlineRelationService()->getContextParent($context); + if ($parentObject === null) { + return false; + } - if (empty($parentObject)) { - throw new \RuntimeException( - 'It is not possible to check if object is in context scope without parent object. This message should never be thrown', - 1589811502043 - ); + $assignable = $this->getInlineRelationService()->checkAssignable($parentObject, $propertyMetadata->name, $object); + if ($assignable !== null) { + return $assignable; } - $property = ObjectAccess::getProperty($parentObject, $propertyMetadata->name); + $property = $parentObject->_getCleanProperty($propertyMetadata->name); - if ( - $propertyMetadata->type['name'] === ObjectStorage::class - || is_subclass_of($propertyMetadata->type['name'], ObjectStorage::class) - ) { - return $property->contains($object); + // Compared by UID - the related object may be an instance of another class (e.g. an API resource class + // declared as `type` in YAML metadata) than the one in the relation. + if ($property instanceof ObjectStorage) { + foreach ($property as $relatedObject) { + if ($relatedObject instanceof AbstractDomainObject && $relatedObject->getUid() === $object->getUid()) { + return true; + } + } + + return false; } - if ($property instanceof AbstractDomainObject) { + // `LazyLoadingProxy::getUid()` does not load the related object. + if ($property instanceof AbstractDomainObject || $property instanceof LazyLoadingProxy) { return $object->getUid() === $property->getUid(); } - throw new \RuntimeException('Unsupported object type', 1589811431286); + return false; } } diff --git a/Classes/Serializer/Handler/FileReferenceHandler.php b/Classes/Serializer/Handler/FileReferenceHandler.php index dac8eb10..32695d9d 100644 --- a/Classes/Serializer/Handler/FileReferenceHandler.php +++ b/Classes/Serializer/Handler/FileReferenceHandler.php @@ -9,9 +9,9 @@ use JMS\Serializer\SerializationContext; use JMS\Serializer\Visitor\DeserializationVisitorInterface; use JMS\Serializer\Visitor\SerializationVisitorInterface; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\Exception\ValidationException; use SourceBroker\T3api\Service\FileReferenceService; -use SourceBroker\T3api\Service\InlineRelationService; use SourceBroker\T3api\Service\SerializerService; use SourceBroker\T3api\Service\UrlService; use TYPO3\CMS\Core\Resource\FileReference as Typo3FileReference; @@ -31,6 +31,8 @@ class FileReferenceHandler extends AbstractHandler implements SerializeHandlerInterface, DeserializeHandlerInterface { + use InlineRelationServiceTrait; + /** * @var string */ @@ -49,22 +51,6 @@ public function __construct( protected readonly ContentObjectRenderer $contentObjectRenderer ) {} - protected ?InlineRelationService $inlineRelationService = null; - - /** - * Injected by method (not constructor) so that adding new dependency does not break handlers which extend this - * class and override the constructor. - */ - public function injectInlineRelationService(InlineRelationService $inlineRelationService): void - { - $this->inlineRelationService = $inlineRelationService; - } - - protected function getInlineRelationService(): InlineRelationService - { - return $this->inlineRelationService ??= GeneralUtility::makeInstance(InlineRelationService::class); - } - /** * @param ExtbaseFileReference|Typo3FileReference $fileReference * @@ -157,7 +143,8 @@ public function deserialize( ); } - if ($data === 0) { + // `0` (also as string) removes the reference of a single file property + if (is_numeric($data) && (int)$data === 0) { $this->removeExistingFileReference($context); return null; @@ -177,9 +164,18 @@ public function deserialize( ExtbaseFileReference::class, false ); - if ($fileReference instanceof AbstractDomainObject) { - $this->getInlineRelationService()->assertAssignable($fileReference, $context); + // @todo `File` and `Folder` properties are looked up as file references as well - kept unchanged here + if (!is_subclass_of($type['params']['targetType'], FileReference::class)) { + return $fileReference; + } + if (!$fileReference instanceof AbstractDomainObject) { + throw new BadRequestException( + 'relation_not_found', + [(string)$uid, implode('.', $context->getCurrentPath())], + 1793670002 + ); } + $this->getInlineRelationService()->assertAssignable($fileReference, $context, 1793660002); return $fileReference; } diff --git a/Classes/Serializer/Handler/InlineRelationServiceTrait.php b/Classes/Serializer/Handler/InlineRelationServiceTrait.php new file mode 100644 index 00000000..4c4bc9a3 --- /dev/null +++ b/Classes/Serializer/Handler/InlineRelationServiceTrait.php @@ -0,0 +1,30 @@ +inlineRelationService = $inlineRelationService; + } + + protected function getInlineRelationService(): InlineRelationService + { + return $this->inlineRelationService ??= GeneralUtility::makeInstance(InlineRelationService::class); + } +} diff --git a/Classes/Serializer/Handler/ObjectStorageHandler.php b/Classes/Serializer/Handler/ObjectStorageHandler.php index 054e6803..58917d7a 100644 --- a/Classes/Serializer/Handler/ObjectStorageHandler.php +++ b/Classes/Serializer/Handler/ObjectStorageHandler.php @@ -68,6 +68,14 @@ public function deserialize( $items = $visitor->visitArray($data, $type); foreach ($items as $item) { + // `null` or `0` (which clears a single relation) can not be an item of a collection + if ($item === null) { + throw new BadRequestException( + 'empty_collection_item', + [implode('.', $context->getCurrentPath())], + 1793680001 + ); + } $objectStorage->attach($item); } diff --git a/Classes/Service/InlineRelationService.php b/Classes/Service/InlineRelationService.php index ca1dafdd..ab583cf7 100644 --- a/Classes/Service/InlineRelationService.php +++ b/Classes/Service/InlineRelationService.php @@ -13,17 +13,19 @@ use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject; use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap; use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap\Relation; -use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper; +use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMap; +use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapFactory; /** * Records of relations stored in the related table (TCA `inline` with `foreign_field`, TCA `file`) keep the parent - * record in their own row. Assigning such an existing record to a property moves it to the new parent, so it is - * allowed only for records without parent and for records which already belong to the same parent property. + * record in their own row and belong to this parent only. An existing record of such relation can be assigned + * (or updated by cascade persistence) only by the parent property it already belongs to. New records are created + * without `uid`. */ class InlineRelationService { public function __construct( - private readonly DataMapper $dataMapper, + private readonly DataMapFactory $dataMapFactory, private readonly ConnectionPool $connectionPool ) {} @@ -33,60 +35,110 @@ public function __construct( * * @throws BadRequestException */ - public function assertAssignable(AbstractDomainObject $record, DeserializationContext $context): void + public function assertAssignable(AbstractDomainObject $record, DeserializationContext $context, int $code): void { - $visitor = $context->getVisitor(); - $parent = method_exists($visitor, 'getCurrentObject') ? $visitor->getCurrentObject() : null; + $parent = $this->getContextParent($context); $metadataStack = $context->getMetadataStack(); $propertyMetadata = $metadataStack->isEmpty() ? null : $metadataStack->top(); if ( - $parent instanceof AbstractDomainObject + $parent !== null && $propertyMetadata instanceof PropertyMetadata - && !$this->isAssignable($parent, $propertyMetadata->name, $record) + && $this->checkAssignable($parent, $propertyMetadata->name, $record) === false ) { throw new BadRequestException( 'relation_not_assignable', [(string)$record->getUid(), implode('.', $context->getCurrentPath())], - 1793660001 + $code ); } } + /** + * The object which is currently deserialized - the parent of the relation which is deserialized. + */ + public function getContextParent(DeserializationContext $context): ?AbstractDomainObject + { + $visitor = $context->getVisitor(); + $parent = method_exists($visitor, 'getCurrentObject') ? $visitor->getCurrentObject() : null; + + return $parent instanceof AbstractDomainObject ? $parent : null; + } + + /** + * Whether `$record` already belongs to property `$propertyName` of `$parent` - checked in the database, as the + * property of `$parent` may be reset before deserialization (PUT). Null when the relation is not stored in the + * related table. + */ + public function checkAssignable(AbstractDomainObject $parent, string $propertyName, AbstractDomainObject $record): ?bool + { + $parentDataMap = $this->dataMapFactory->buildDataMap(get_class($parent)); + $columnMap = $this->getRelatedTableColumnMap($parentDataMap, $propertyName); + + return $columnMap === null + ? null + : $this->belongsToParent($parent, $parentDataMap->getTableName(), $columnMap, $record); + } + + public function isStoredInRelatedTable(AbstractDomainObject $parent, string $propertyName): bool + { + return $this->getRelatedTableColumnMap( + $this->dataMapFactory->buildDataMap(get_class($parent)), + $propertyName + ) !== null; + } + public function isAssignable(AbstractDomainObject $parent, string $propertyName, AbstractDomainObject $record): bool { - $columnMap = $this->dataMapper->getDataMap(get_class($parent))->getColumnMap($propertyName); - if ($columnMap === null || !$this->isStoredInRelatedTable($columnMap)) { - return true; - } + return $this->checkAssignable($parent, $propertyName, $record) === true; + } + + private function getRelatedTableColumnMap(DataMap $parentDataMap, string $propertyName): ?ColumnMap + { + $columnMap = $parentDataMap->getColumnMap($propertyName); + + return $columnMap !== null + && in_array($columnMap->getTypeOfRelation(), [Relation::HAS_ONE, Relation::HAS_MANY], true) + && (string)$columnMap->getParentKeyFieldName() !== '' + && (string)$columnMap->getChildTableName() !== '' + ? $columnMap + : null; + } + private function belongsToParent( + AbstractDomainObject $parent, + string $parentTable, + ColumnMap $columnMap, + AbstractDomainObject $record + ): bool { $parentKeyField = (string)$columnMap->getParentKeyFieldName(); $parentTableField = (string)$columnMap->getParentTableFieldName(); $matchFields = $columnMap->getRelationTableMatchFields() ?? []; + // The row Extbase updates when persisting the record: the translation for a translated record. + $recordUid = (int)($record->_getProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID) ?: $record->getUid()); $row = $this->fetchParentFields( (string)$columnMap->getChildTableName(), - (int)$record->getUid(), + $recordUid, array_filter([$parentKeyField, $parentTableField, ...array_keys($matchFields)]) ); if ($row === null) { return false; } - if ((int)$row[$parentKeyField] === 0) { - return true; - } - - // Same value as written by Extbase persistence - a translated parent is stored with its localized UID. - $parentUid = (int)($parent->_getProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID) ?: $parent->getUid()); - if ($parentUid === 0 || (int)$row[$parentKeyField] !== $parentUid) { + // The parent key Extbase writes: for records of a collection the localized UID of a translated parent, so + // a record stored with another value would be moved. A record of a single relation keeps its parent key, + // which is the default language UID of the parent when the record was inserted. + $localizedParentUid = (int)($parent->_getProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID) ?: $parent->getUid()); + $parentUids = $columnMap->getTypeOfRelation() === Relation::HAS_MANY + ? [$localizedParentUid] + : [$localizedParentUid, (int)$parent->getUid()]; + $parentUids = array_filter($parentUids); + if (!in_array((int)$row[$parentKeyField], $parentUids, true)) { return false; } - if ( - $parentTableField !== '' - && (string)$row[$parentTableField] !== $this->dataMapper->getDataMap(get_class($parent))->getTableName() - ) { + if ($parentTableField !== '' && (string)$row[$parentTableField] !== $parentTable) { return false; } @@ -99,13 +151,6 @@ public function isAssignable(AbstractDomainObject $parent, string $propertyName, return true; } - private function isStoredInRelatedTable(ColumnMap $columnMap): bool - { - return in_array($columnMap->getTypeOfRelation(), [Relation::HAS_ONE, Relation::HAS_MANY], true) - && (string)$columnMap->getParentKeyFieldName() !== '' - && (string)$columnMap->getChildTableName() !== ''; - } - /** * @param string[] $fields * @return array|null diff --git a/Documentation/HandlingCascadePersistence/Index.rst b/Documentation/HandlingCascadePersistence/Index.rst index d0db1c6c..da628071 100644 --- a/Documentation/HandlingCascadePersistence/Index.rst +++ b/Documentation/HandlingCascadePersistence/Index.rst @@ -33,11 +33,55 @@ Relations stored in the related table -------------------------------------- Records of relations stored in the related table - TCA ``inline`` with ``foreign_field`` and TCA ``file`` -(file references) - keep their parent record in their own row, so they can belong to one parent only. -An existing record of such relation can be passed by ``uid`` only when it does not belong to any record yet, -or when it already belongs to the same property of the same record (e.g. sending the current items again in -``PATCH`` or ``PUT``). Otherwise the response is ``400 Bad Request``. To add new items, create them using -cascade persistence described below. +(file references) - keep their parent record in their own row and belong to this parent only. An existing record +of such relation can be passed by ``uid`` (also to update it, see below) only when it already belongs to the same +property of the same record, e.g. when sending the current items again in ``PATCH`` or ``PUT``. Otherwise the +response is ``400 Bad Request``. New items are created in the same request - pass them without ``uid`` using +cascade persistence described below, file references with ``uidLocal`` (see :ref:`handling_file_upload`). + +.. code-block:: json + + { + "steps": [ + 1, + {"uid": 2, "title": "Changed title of existing step"}, + {"title": "New step"} + ] + } + +The collection sent in the request replaces the current one. Extbase does not delete an item removed from the +collection, it only clears its relation to the parent. To delete removed items, add the Extbase ``Cascade`` +attribute to the property. TYPO3 14 reads Extbase annotations only from PHP attributes, not from doc comments. +The form below works in TYPO3 12, 13 and 14 (in TYPO3 14 you can use +``#[\TYPO3\CMS\Extbase\Attribute\ORM\Cascade('remove')]`` instead): + +.. code-block:: php + + use SourceBroker\T3api\Annotation as T3api; + use TYPO3\CMS\Extbase\Annotation\ORM\Cascade; + + /** + * @var \TYPO3\CMS\Extbase\Persistence\ObjectStorage<\Vendor\Ext\Domain\Model\Step> + * @T3api\ORM\Cascade("persist") + */ + #[Cascade(['value' => 'remove'])] + protected ObjectStorage $steps; + +An existing record which can not be found (also a hidden one) returns ``400 Bad Request``. A record passed with +``"uid": 0`` is created as a new one. Value ``0`` clears a single relation; in a collection it returns +``400 Bad Request``. + +.. note:: + For a translated record Extbase stores the translated record as the parent of items of such relation. An item + of the default language shown for the translation (language fallback) is therefore rejected when sent again - + saving it would move it from the default language record to the translation. + + Do not expose the field which stores the parent (``foreign_field``, e.g. ``recipe`` of a recipe step) in the + denormalization groups - otherwise a record can be moved to another parent by updating it. + +If records of your relations are owned by users and should be checked by their owner, add a rule for it in +``security_post_denormalize`` of the operation, e.g. a method of the model which checks all items, see +:ref:`security`. Cascade persistence - creating related entity in single request ================================================================ diff --git a/Documentation/HandlingFileUpload/Index.rst b/Documentation/HandlingFileUpload/Index.rst index 00f1b971..5a4488e6 100644 --- a/Documentation/HandlingFileUpload/Index.rst +++ b/Documentation/HandlingFileUpload/Index.rst @@ -142,8 +142,7 @@ Save reference to new file .. important:: It is not (yet) possible to update existing file reference within T3api request - it is possible only to create new reference. An existing file reference can be passed by ``uid`` only when it already belongs to the same - property of the same record or does not belong to any record, see - :ref:`handling_cascade_persistence_inline_relations`. + property of the same record, see :ref:`handling_cascade_persistence_inline_relations`. .. code-block:: json diff --git a/Documentation/Miscellaneous/Changelog/Index.rst b/Documentation/Miscellaneous/Changelog/Index.rst index 6416584b..bf621520 100644 --- a/Documentation/Miscellaneous/Changelog/Index.rst +++ b/Documentation/Miscellaneous/Changelog/Index.rst @@ -6,7 +6,7 @@ Changelog 5.0.0 ===== -- [!!!] Existing records of relations stored in the related table (TCA ``inline`` with ``foreign_field``, TCA ``file``) can be assigned by ``uid`` only when they do not belong to any record or already belong to the same property of the same record, otherwise ``400 Bad Request`` is returned. See :ref:`handling_cascade_persistence_inline_relations`. +- [!!!] Existing records of relations stored in the related table (TCA ``inline`` with ``foreign_field``, TCA ``file``) can be assigned or updated by ``uid`` only when they already belong to the same property of the same record, otherwise ``400 Bad Request`` is returned. New records are passed without ``uid``. Fixes cascade update in ``PUT`` requests (all relation types). A not existing record passed by ``uid`` returns ``400 Bad Request`` instead of HTTP 500. See :ref:`handling_cascade_persistence_inline_relations`. - Keep properties missing in the request body when an existing record is updated (``PATCH``, cascade update of a related record). Newer versions of JMS serializer set the declared default value of such properties (e.g. ``protected string $title = ''``), so all properties which were not sent were reset. - Fix response cache identifier for ``identifierExpressions`` returning an array (e.g. ``frontend.user.userGroupIds``) - array items are part of the identifier now. Tag expressions returning an array add a tag per item. See :ref:`response-cache`. - [!!!] Validate origin and content type of requests changing data: requests from another origin not allowed by CORS ``allowOrigin`` return ``403 Forbidden``, JSON body without JSON ``Content-Type`` returns ``415 Unsupported Media Type``. Add origins of frontend applications to ``allowOrigin`` and send ``Content-Type: application/json``. See :ref:`cors_write_requests`. diff --git a/Resources/Private/Language/locallang.xlf b/Resources/Private/Language/locallang.xlf index 63670e14..a03ea9e0 100644 --- a/Resources/Private/Language/locallang.xlf +++ b/Resources/Private/Language/locallang.xlf @@ -60,8 +60,14 @@ Value of type `%s` in path `%s` has to be an array + + Collection in path `%s` can not contain an empty value + + + Record `%s` in path `%s` not found + - Record `%s` in path `%s` belongs to another record and can not be assigned + Record `%s` in path `%s` does not belong to this record. Pass new records without `uid` Unknown order direction `%s`, expected `asc` or `desc` diff --git a/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php b/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php index 15c0c086..46f4892c 100644 --- a/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php +++ b/Tests/Functional/Dispatcher/InlineRelationAssignmentDispatcherTest.php @@ -19,21 +19,33 @@ /** * Existing records of relations stored in the related table (inline `steps` with `foreign_field`, `images` of TCA - * type `file`) can be assigned by UID only when they have no parent or already belong to the same parent property. + * type `file`) can be assigned or updated by UID only when they already belong to the same parent property. New + * records are created without UID. Records of other relations (`category`, MM `tags`) can be updated only when + * they are related already - also in PUT, which resets the properties before deserialization. */ class InlineRelationAssignmentDispatcherTest extends AbstractDispatcherTestCase { private const RECIPE_TABLE = 'tx_functionaltest_domain_model_recipe'; private const STEP_TABLE = 'tx_functionaltest_domain_model_recipestep'; + private const CATEGORY_TABLE = 'tx_functionaltest_domain_model_category'; + private const TAG_TABLE = 'tx_functionaltest_domain_model_tag'; protected function setUp(): void { parent::setUp(); $recipes = $this->getConnectionPool()->getConnectionForTable(self::RECIPE_TABLE); - $recipes->insert(self::RECIPE_TABLE, ['uid' => 1, 'pid' => 0, 'title' => 'First recipe', 'steps' => 1, 'images' => 1]); + $recipes->insert(self::RECIPE_TABLE, ['uid' => 1, 'pid' => 0, 'title' => 'First recipe', 'steps' => 1, 'images' => 1, 'image' => 1, 'category' => 1, 'tags' => 1]); $recipes->insert(self::RECIPE_TABLE, ['uid' => 2, 'pid' => 0, 'title' => 'Second recipe', 'steps' => 1, 'images' => 1]); + foreach ([self::CATEGORY_TABLE, self::TAG_TABLE] as $table) { + $connection = $this->getConnectionPool()->getConnectionForTable($table); + $connection->insert($table, ['uid' => 1, 'pid' => 0, 'title' => 'Related']); + $connection->insert($table, ['uid' => 2, 'pid' => 0, 'title' => 'Not related']); + } + $this->getConnectionPool()->getConnectionForTable('tx_functionaltest_recipe_tag_mm') + ->insert('tx_functionaltest_recipe_tag_mm', ['uid_local' => 1, 'uid_foreign' => 1, 'sorting' => 1]); + $steps = $this->getConnectionPool()->getConnectionForTable(self::STEP_TABLE); $steps->insert(self::STEP_TABLE, ['uid' => 1, 'pid' => 0, 'title' => 'Step of first recipe', 'recipe' => 1]); $steps->insert(self::STEP_TABLE, ['uid' => 2, 'pid' => 0, 'title' => 'Step of second recipe', 'recipe' => 2]); @@ -46,6 +58,7 @@ protected function setUp(): void [13, 1, 'tt_content', 'image'], [14, 1, self::RECIPE_TABLE, 'other'], [15, 0, '', ''], + [16, 1, self::RECIPE_TABLE, 'image'], ] as [$uid, $uidForeign, $tableNames, $fieldName]) { $references->insert('sys_file_reference', [ 'uid' => $uid, @@ -60,7 +73,7 @@ protected function setUp(): void #[Test] #[DataProvider('assignableRecordProvider')] - public function recordWithoutParentOrOfSameParentPropertyIsAssigned(string $property, int $uid, string $table, string $parentField): void + public function recordOfSameParentPropertyIsAssigned(string $property, int $uid, string $table, string $parentField): void { $response = $this->dispatch('PATCH', '/_api/recipes/1', [$property => [$uid]]); @@ -72,9 +85,7 @@ public static function assignableRecordProvider(): array { return [ 'inline record of same parent' => ['steps', 1, self::STEP_TABLE, 'recipe'], - 'inline record without parent' => ['steps', 3, self::STEP_TABLE, 'recipe'], 'file reference of same parent property' => ['images', 11, 'sys_file_reference', 'uid_foreign'], - 'file reference without parent' => ['images', 15, 'sys_file_reference', 'uid_foreign'], ]; } @@ -88,7 +99,7 @@ public function recordOfOtherParentIsRejected(string $property, int $uid, string $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); self::assertSame(400, $response->getStatusCode(), $body); self::assertSame( - sprintf('Record `%d` in path `%s` belongs to another record and can not be assigned', $uid, $property), + sprintf('Record `%d` in path `%s` does not belong to this record. Pass new records without `uid`', $uid, $property), $json['hydra:description'] ); self::assertSame($parentUid, $this->fetchField($table, $uid, $parentField)); @@ -101,6 +112,8 @@ public static function notAssignableRecordProvider(): array 'file reference of other parent' => ['images', 12, 'sys_file_reference', 'uid_foreign', 2], 'file reference of record with same UID in other table' => ['images', 13, 'sys_file_reference', 'uid_foreign', 1], 'file reference of other property of same parent' => ['images', 14, 'sys_file_reference', 'uid_foreign', 1], + 'inline record without parent' => ['steps', 3, self::STEP_TABLE, 'recipe', 0], + 'file reference without parent' => ['images', 15, 'sys_file_reference', 'uid_foreign', 0], ]; } @@ -114,19 +127,250 @@ public function putKeepsInlineRecordsOfSameParent(): void } #[Test] - public function newRecordAcceptsOnlyInlineRecordsWithoutParent(): void + public function newRecordAcceptsOnlyNewInlineRecords(): void { - $rejected = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [1]]); - - self::assertSame(400, $rejected->getStatusCode(), (string)$rejected->getBody()); - self::assertSame(1, $this->fetchField(self::STEP_TABLE, 1, 'recipe')); + foreach ([1, 3] as $existingUid) { + $rejected = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [$existingUid]]); + self::assertSame(400, $rejected->getStatusCode(), (string)$rejected->getBody()); + } - $created = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [3]]); + $created = $this->dispatch('POST', '/_api/recipes', ['title' => 'New recipe', 'steps' => [['title' => 'New step']]]); $json = json_decode((string)$created->getBody(), true, 512, JSON_THROW_ON_ERROR); self::assertSame(201, $created->getStatusCode(), (string)$created->getBody()); - self::assertSame($json['uid'], $this->fetchField(self::STEP_TABLE, 3, 'recipe')); self::assertSame(3, $this->getConnectionPool()->getConnectionForTable(self::RECIPE_TABLE)->count('*', self::RECIPE_TABLE, [])); + self::assertSame( + $json['uid'], + (int)$this->getConnectionPool()->getConnectionForTable(self::STEP_TABLE) + ->select(['recipe'], self::STEP_TABLE, ['title' => 'New step'])->fetchOne() + ); + self::assertSame(1, $this->fetchField(self::STEP_TABLE, 1, 'recipe')); + self::assertSame(0, $this->fetchField(self::STEP_TABLE, 3, 'recipe')); + } + + /** + * Forms upload a file first (`sys_file`) and send it as a new reference by `uidLocal` together with the existing + * references of the record. + */ + #[Test] + public function existingReferenceIsKeptAndNewReferenceIsCreatedFromUploadedFile(): void + { + // The test request is a backend request, so FAL evaluates storage permissions of a backend user. + $this->getConnectionPool()->getConnectionForTable('be_users')->insert('be_users', [ + 'uid' => 1, + 'pid' => 0, + 'username' => 'admin', + 'admin' => 1, + ]); + $this->setUpBackendUser(1); + $this->getConnectionPool()->getConnectionForTable('sys_file_storage')->insert('sys_file_storage', [ + 'uid' => 1, + 'pid' => 0, + 'name' => 'fileadmin', + 'driver' => 'Local', + 'configuration' => 'fileadmin/relative1', + 'is_browsable' => 1, + 'is_public' => 1, + 'is_writable' => 1, + 'is_online' => 1, + ]); + $this->getConnectionPool()->getConnectionForTable('sys_file')->insert('sys_file', [ + 'uid' => 21, + 'pid' => 0, + 'storage' => 1, + 'type' => 2, + 'identifier' => '/user_upload/uploaded.jpg', + 'identifier_hash' => sha1('/user_upload/uploaded.jpg'), + 'folder_hash' => sha1('/user_upload/'), + 'extension' => 'jpg', + 'mime_type' => 'image/jpeg', + 'name' => 'uploaded.jpg', + ]); + + $response = $this->dispatch('PATCH', '/_api/recipes/1', ['images' => [11, ['uidLocal' => 21]]]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(1, $this->fetchField('sys_file_reference', 11, 'uid_foreign')); + $newReference = $this->getConnectionPool()->getConnectionForTable('sys_file_reference') + ->select(['uid_foreign', 'tablenames', 'fieldname'], 'sys_file_reference', ['uid_local' => 21]) + ->fetchAssociative(); + self::assertSame( + ['uid_foreign' => 1, 'tablenames' => self::RECIPE_TABLE, 'fieldname' => 'images'], + ['uid_foreign' => (int)$newReference['uid_foreign'], 'tablenames' => $newReference['tablenames'], 'fieldname' => $newReference['fieldname']] + ); + } + + #[Test] + #[DataProvider('writeMethodProvider')] + public function inlineRecordOfSameParentIsUpdated(string $method): void + { + $response = $this->dispatch($method, '/_api/recipes/1', [ + 'title' => 'First recipe', + 'steps' => [['uid' => 1, 'title' => 'Changed step']], + ]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame('Changed step', $this->fetchTitle(self::STEP_TABLE, 1)); + self::assertSame(1, $this->fetchField(self::STEP_TABLE, 1, 'recipe')); + } + + #[Test] + #[DataProvider('writeMethodProvider')] + public function inlineRecordOfOtherParentIsNotUpdated(string $method): void + { + $response = $this->dispatch($method, '/_api/recipes/1', [ + 'title' => 'First recipe', + 'steps' => [['uid' => 2, 'title' => 'Changed step']], + ]); + + self::assertSame(400, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame('Step of second recipe', $this->fetchTitle(self::STEP_TABLE, 2)); + self::assertSame(2, $this->fetchField(self::STEP_TABLE, 2, 'recipe')); + } + + #[Test] + #[DataProvider('writeMethodProvider')] + public function relatedRecordOfOtherRelationIsUpdated(string $method): void + { + $response = $this->dispatch($method, '/_api/recipes/1', [ + 'title' => 'First recipe', + 'category' => ['uid' => 1, 'title' => 'Changed category'], + 'tags' => [['uid' => 1, 'title' => 'Changed tag']], + ]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame('Changed category', $this->fetchTitle(self::CATEGORY_TABLE, 1)); + self::assertSame('Changed tag', $this->fetchTitle(self::TAG_TABLE, 1)); + self::assertSame(1, $this->fetchField(self::RECIPE_TABLE, 1, 'category')); + } + + #[Test] + #[DataProvider('notRelatedRecordProvider')] + public function notRelatedRecordOfOtherRelationIsNotUpdated(string $method, int $recipeUid, array $body, string $table): void + { + $response = $this->dispatch($method, '/_api/recipes/' . $recipeUid, ['title' => 'Recipe'] + $body); + + self::assertSame(400, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame('Not related', $this->fetchTitle($table, 2)); + } + + public static function notRelatedRecordProvider(): array + { + $cases = []; + foreach (['PATCH', 'PUT'] as $method) { + $cases[$method . ' single relation'] = [$method, 1, ['category' => ['uid' => 2, 'title' => 'Changed']], self::CATEGORY_TABLE]; + $cases[$method . ' MM relation'] = [$method, 1, ['tags' => [['uid' => 2, 'title' => 'Changed']]], self::TAG_TABLE]; + $cases[$method . ' empty single relation'] = [$method, 2, ['category' => ['uid' => 2, 'title' => 'Changed']], self::CATEGORY_TABLE]; + } + + return $cases; + } + + #[Test] + #[DataProvider('notExistingRecordProvider')] + public function notExistingRecordIsRejected(array $body, string $path): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', $body); + + $json = json_decode((string)$response->getBody(), true, 512, JSON_THROW_ON_ERROR); + self::assertSame(400, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(sprintf('Record `999` in path `%s` not found', $path), $json['hydra:description']); + } + + public static function notExistingRecordProvider(): array + { + return [ + 'inline record' => [['steps' => [999]], 'steps'], + 'file reference' => [['images' => [999]], 'images'], + 'cascade update' => [['steps' => [['uid' => 999, 'title' => 'Changed']]], 'steps'], + 'MM relation' => [['tags' => [999]], 'tags'], + ]; + } + + #[Test] + #[DataProvider('zeroValueProvider')] + public function zeroClearsSingleRelation(int|string $value): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', ['category' => $value]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(0, $this->fetchField(self::RECIPE_TABLE, 1, 'category')); + } + + public static function zeroValueProvider(): array + { + return [ + 'integer' => [0], + 'string' => ['0'], + ]; + } + + #[Test] + #[DataProvider('emptyCollectionItemProvider')] + public function emptyCollectionItemIsRejected(array $body, string $path): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', $body); + + $json = json_decode((string)$response->getBody(), true, 512, JSON_THROW_ON_ERROR); + self::assertSame(400, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame(sprintf('Collection in path `%s` can not contain an empty value', $path), $json['hydra:description']); + } + + public static function emptyCollectionItemProvider(): array + { + return [ + 'zero in MM relation' => [['tags' => [0]], 'tags'], + 'zero string in inline relation' => [['steps' => [1, '0']], 'steps'], + 'zero in file references' => [['images' => [0]], 'images'], + ]; + } + + #[Test] + public function nullCollectionItemIsRejected(): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', ['steps' => [null]]); + + self::assertSame(400, $response->getStatusCode(), (string)$response->getBody()); + } + + #[Test] + #[DataProvider('zeroValueProvider')] + public function zeroRemovesReferenceOfSingleFileProperty(int|string $value): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', ['image' => $value]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + $queryBuilder = $this->getConnectionPool()->getQueryBuilderForTable('sys_file_reference'); + $queryBuilder->getRestrictions()->removeAll(); + self::assertSame(1, (int)$queryBuilder->select('deleted')->from('sys_file_reference') + ->where($queryBuilder->expr()->eq('uid', 16))->executeQuery()->fetchOne()); + } + + #[Test] + public function recordWithZeroUidIsCreated(): void + { + $response = $this->dispatch('PATCH', '/_api/recipes/1', ['steps' => [1, ['uid' => 0, 'title' => 'New step']]]); + + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame( + 1, + (int)$this->getConnectionPool()->getConnectionForTable(self::STEP_TABLE) + ->select(['recipe'], self::STEP_TABLE, ['title' => 'New step'])->fetchOne() + ); + } + + public static function writeMethodProvider(): array + { + return [ + 'PATCH' => ['PATCH'], + 'PUT' => ['PUT'], + ]; + } + + private function fetchTitle(string $table, int $uid): string + { + return (string)$this->getConnectionPool()->getConnectionForTable($table) + ->select(['title'], $table, ['uid' => $uid])->fetchOne(); } private function fetchField(string $table, int $uid, string $field): int diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Category.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Category.php index 350ecab5..6c89cf5e 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Category.php +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Category.php @@ -14,4 +14,9 @@ public function getTitle(): string { return $this->title; } + + public function setTitle(string $title): void + { + $this->title = $title; + } } diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Recipe.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Recipe.php index 01e88c75..39201c5f 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Recipe.php +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Recipe.php @@ -5,6 +5,7 @@ namespace T3apiTests\FunctionalTest\Domain\Model; use SourceBroker\T3api\Annotation\ApiResource; +use SourceBroker\T3api\Annotation as T3api; use TYPO3\CMS\Extbase\DomainObject\AbstractEntity; use TYPO3\CMS\Extbase\Persistence\ObjectStorage; @@ -63,9 +64,23 @@ class Recipe extends AbstractEntity /** * @var ObjectStorage + * @T3api\ORM\Cascade("persist") */ protected ObjectStorage $steps; + /** + * @T3api\ORM\Cascade("persist") + */ + protected ?Category $category = null; + + /** + * @var ObjectStorage + * @T3api\ORM\Cascade("persist") + */ + protected ObjectStorage $tags; + + protected ?FileReference $image = null; + /** * @var ObjectStorage */ @@ -74,6 +89,7 @@ class Recipe extends AbstractEntity public function __construct() { $this->steps = new ObjectStorage(); + $this->tags = new ObjectStorage(); $this->images = new ObjectStorage(); } @@ -103,6 +119,42 @@ public function setSteps(ObjectStorage $steps): void $this->steps = $steps; } + public function getCategory(): ?Category + { + return $this->category; + } + + public function setCategory(?Category $category): void + { + $this->category = $category; + } + + /** + * @return ObjectStorage + */ + public function getTags(): ObjectStorage + { + return $this->tags; + } + + /** + * @param ObjectStorage $tags + */ + public function setTags(ObjectStorage $tags): void + { + $this->tags = $tags; + } + + public function getImage(): ?FileReference + { + return $this->image; + } + + public function setImage(?FileReference $image): void + { + $this->image = $image; + } + /** * @return ObjectStorage */ diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Tag.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Tag.php index 9505f686..63050510 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Tag.php +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Domain/Model/Tag.php @@ -14,4 +14,9 @@ public function getTitle(): string { return $this->title; } + + public function setTitle(string $title): void + { + $this->title = $title; + } } diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php index f00a2c06..4bc03b26 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Configuration/TCA/tx_functionaltest_domain_model_recipe.php @@ -19,6 +19,32 @@ 'foreign_field' => 'recipe', ], ], + 'category' => [ + 'label' => 'Category', + 'config' => [ + 'type' => 'select', + 'renderType' => 'selectSingle', + 'foreign_table' => 'tx_functionaltest_domain_model_category', + 'default' => 0, + ], + ], + 'tags' => [ + 'label' => 'Tags', + 'config' => [ + 'type' => 'select', + 'renderType' => 'selectMultipleSideBySide', + 'foreign_table' => 'tx_functionaltest_domain_model_tag', + 'MM' => 'tx_functionaltest_recipe_tag_mm', + ], + ], + 'image' => [ + 'label' => 'Image', + 'config' => [ + 'type' => 'file', + 'maxitems' => 1, + 'allowed' => 'common-image-types', + ], + ], 'images' => [ 'label' => 'Images', 'config' => [ @@ -27,5 +53,5 @@ ], ], ], - 'title, steps, images' + 'title, steps, category, tags, image, images' ); diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Category.yml b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Category.yml new file mode 100644 index 00000000..f9de1863 --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Category.yml @@ -0,0 +1,4 @@ +T3apiTests\FunctionalTest\Domain\Model\Category: + properties: + title: + groups: [api_recipe_write] diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml index b4bef49a..9defc799 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Recipe.yml @@ -8,3 +8,11 @@ T3apiTests\FunctionalTest\Domain\Model\Recipe: groups: [api_recipe_write] images: groups: [api_recipe_write] + category: + groups: [api_recipe_write] + tags: + # API resource subclass of the related class - related records are compared by UID, not by instance + type: 'TYPO3\CMS\Extbase\Persistence\ObjectStorage' + groups: [api_recipe_write] + image: + groups: [api_recipe_write] diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.RecipeStep.yml b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.RecipeStep.yml new file mode 100644 index 00000000..8d29789d --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.RecipeStep.yml @@ -0,0 +1,4 @@ +T3apiTests\FunctionalTest\Domain\Model\RecipeStep: + properties: + title: + groups: [api_recipe_write] diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Tag.yml b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Tag.yml new file mode 100644 index 00000000..8654725b --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Resources/Private/Serializer/Domain.Model.Tag.yml @@ -0,0 +1,4 @@ +T3apiTests\FunctionalTest\Domain\Model\Tag: + properties: + title: + groups: [api_recipe_write] diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql index 4ae5d296..6d7a8a88 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql @@ -38,9 +38,22 @@ CREATE TABLE tx_functionaltest_domain_model_article ( CREATE TABLE tx_functionaltest_domain_model_recipe ( title varchar(255) DEFAULT '' NOT NULL, steps int(11) unsigned DEFAULT '0' NOT NULL, + category int(11) unsigned DEFAULT '0' NOT NULL, + tags int(11) unsigned DEFAULT '0' NOT NULL, + image int(11) unsigned DEFAULT '0' NOT NULL, images int(11) unsigned DEFAULT '0' NOT NULL ); +CREATE TABLE tx_functionaltest_recipe_tag_mm ( + uid_local int(11) unsigned DEFAULT '0' NOT NULL, + uid_foreign int(11) unsigned DEFAULT '0' NOT NULL, + sorting int(11) unsigned DEFAULT '0' NOT NULL, + sorting_foreign int(11) unsigned DEFAULT '0' NOT NULL, + PRIMARY KEY (uid_local, uid_foreign), + KEY uid_local (uid_local), + KEY uid_foreign (uid_foreign) +); + CREATE TABLE tx_functionaltest_domain_model_recipestep ( title varchar(255) DEFAULT '' NOT NULL, recipe int(11) unsigned DEFAULT '0' NOT NULL diff --git a/Tests/Functional/Service/InlineRelationServiceTest.php b/Tests/Functional/Service/InlineRelationServiceTest.php new file mode 100644 index 00000000..d64bf1a4 --- /dev/null +++ b/Tests/Functional/Service/InlineRelationServiceTest.php @@ -0,0 +1,93 @@ +getConnectionPool()->getConnectionForTable(self::STEP_TABLE); + foreach ([10 => 1, 11 => 5, 12 => 5, 13 => 7, 14 => 0, 15 => 7] as $uid => $recipe) { + $steps->insert(self::STEP_TABLE, ['uid' => $uid, 'pid' => 0, 'title' => 'Step ' . $uid, 'recipe' => $recipe]); + } + } + + #[Test] + #[DataProvider('translatedParentProvider')] + public function recordOfCollectionOfTranslatedParentIsAssignable(int $recordUid, int $recordLocalizedUid, bool $expected): void + { + $recipe = $this->createDomainObject(Recipe::class, 1, 5); + $step = $this->createDomainObject(RecipeStep::class, $recordUid, $recordLocalizedUid); + + self::assertSame($expected, $this->get(InlineRelationService::class)->isAssignable($recipe, 'steps', $step)); + } + + public static function translatedParentProvider(): array + { + return [ + 'stored with localized UID of parent' => [12, 12, true], + 'translation stored with localized UID of parent' => [10, 11, true], + 'stored with default language UID of parent, would be moved' => [10, 10, false], + 'stored with other parent' => [13, 13, false], + 'without parent' => [14, 14, false], + 'default language record of parent, translation of other parent' => [10, 15, false], + ]; + } + + #[Test] + public function recordIsNotAssignableToNewParent(): void + { + $step = $this->createDomainObject(RecipeStep::class, 14, 14); + + self::assertFalse($this->get(InlineRelationService::class)->isAssignable(new Recipe(), 'steps', $step)); + } + + #[Test] + public function onlyRelationsStoredInRelatedTableAreDetected(): void + { + $service = $this->get(InlineRelationService::class); + $recipe = new Recipe(); + + self::assertTrue($service->isStoredInRelatedTable($recipe, 'steps')); + self::assertTrue($service->isStoredInRelatedTable($recipe, 'images')); + self::assertFalse($service->isStoredInRelatedTable($recipe, 'title')); + } + + /** + * @template T of AbstractDomainObject + * @param class-string $className + * @return T + */ + private function createDomainObject(string $className, int $uid, int $localizedUid): AbstractDomainObject + { + $object = new $className(); + $object->_setProperty(AbstractDomainObject::PROPERTY_UID, $uid); + $object->_setProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID, $localizedUid); + + return $object; + } +} From d48f6f0be8cac525e2dd8cdcd44deba0bd46c2b3 Mon Sep 17 00:00:00 2001 From: Krystian Szymukowicz Date: Tue, 3 Nov 2026 12:17:48 +0100 Subject: [PATCH 3/3] [TASK] Allow NULL in single relation columns of test fixtur --- .../Fixtures/Extensions/t3api_functional_test/ext_tables.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql index 6d7a8a88..21539596 100644 --- a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/ext_tables.sql @@ -35,12 +35,13 @@ CREATE TABLE tx_functionaltest_domain_model_article ( title varchar(255) DEFAULT '' NOT NULL ); +-- `category` and `image` allow NULL: Extbase in TYPO3 12 writes NULL for an empty single relation (PUT, value 0) CREATE TABLE tx_functionaltest_domain_model_recipe ( title varchar(255) DEFAULT '' NOT NULL, steps int(11) unsigned DEFAULT '0' NOT NULL, - category int(11) unsigned DEFAULT '0' NOT NULL, + category int(11) unsigned DEFAULT NULL, tags int(11) unsigned DEFAULT '0' NOT NULL, - image int(11) unsigned DEFAULT '0' NOT NULL, + image int(11) unsigned DEFAULT NULL, images int(11) unsigned DEFAULT '0' NOT NULL );