From 7ab5b444ebc1486b24498c43d54d49e55e256b69 Mon Sep 17 00:00:00 2001 From: Oliver Skroblin Date: Wed, 19 Apr 2023 14:03:37 +0200 Subject: [PATCH 1/2] NEXT-25734 - Added option to provide a criteria inside the sync operations in case of delete actions --- phpstan-baseline.neon | 5 - src/Core/Framework/Api/ApiException.php | 22 +++++ .../Api/Controller/SyncController.php | 9 +- src/Core/Framework/Api/Sync/SyncOperation.php | 27 +++++- src/Core/Framework/Api/Sync/SyncService.php | 43 +++++++- .../data-abstraction-layer.xml | 3 + .../Test/Api/Sync/SyncServiceTest.php | 52 ++++++++++ .../Api/Controller/SyncControllerTest.php | 64 ++++++++++++ .../Framework/Api/Sync/SyncServiceTest.php | 97 +++++++++++++++++++ 9 files changed, 314 insertions(+), 8 deletions(-) create mode 100644 src/Core/Framework/Api/ApiException.php create mode 100644 tests/unit/php/Core/Framework/Api/Controller/SyncControllerTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index cf8ba914ad4..eadcd62c5db 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -7444,11 +7444,6 @@ parameters: count: 1 path: src/Core/Framework/Api/Controller/SalesChannelProxyController.php - - - message: "#^Parameter \\#2 \\$skipIndexers of class Shopware\\\\Core\\\\Framework\\\\Api\\\\Sync\\\\SyncBehavior constructor expects list\\, array\\, non\\-falsy\\-string\\> given\\.$#" - count: 1 - path: src/Core/Framework/Api/Controller/SyncController.php - - message: "#^Anonymous variable in a `\\$event\\-\\>\\.\\.\\.\\(\\)` method call can lead to false dead methods\\. Make sure the variable type is known$#" count: 2 diff --git a/src/Core/Framework/Api/ApiException.php b/src/Core/Framework/Api/ApiException.php new file mode 100644 index 00000000000..ee0cdd31d2b --- /dev/null +++ b/src/Core/Framework/Api/ApiException.php @@ -0,0 +1,22 @@ + $indexingSkips */ $indexingSkips = array_filter(explode(',', (string) $request->headers->get(PlatformRequest::HEADER_INDEXING_SKIP, ''))); $behavior = new SyncBehavior( @@ -55,7 +56,13 @@ public function sync(Request $request, Context $context): JsonResponse if (isset($operation['key'])) { $key = $operation['key']; } - $operations[] = new SyncOperation((string) $key, $operation['entity'], $operation['action'], $operation['payload']); + $operations[] = new SyncOperation( + (string) $key, + $operation['entity'], + $operation['action'], + $operation['payload'] ?? [], + $operation['criteria'] ?? [] + ); } $result = $context->scope(Context::CRUD_API_SCOPE, fn (Context $context): SyncResult => $this->syncService->sync($operations, $context, $behavior)); diff --git a/src/Core/Framework/Api/Sync/SyncOperation.php b/src/Core/Framework/Api/Sync/SyncOperation.php index 39d515bada6..b1a42aa7074 100644 --- a/src/Core/Framework/Api/Sync/SyncOperation.php +++ b/src/Core/Framework/Api/Sync/SyncOperation.php @@ -13,12 +13,14 @@ class SyncOperation extends Struct /** * @param array $payload + * @param array $criteria */ public function __construct( protected string $key, protected string $entity, protected string $action, - protected array $payload + protected array $payload, + protected array $criteria = [] ) { } @@ -89,4 +91,27 @@ public function validate(): array return $errors; } + + /** + * @internal used to replace payload in case of api shorthands (e.g. delete mappings with wild cards, etc) + * + * @param array $payload + */ + public function replacePayload(array $payload): void + { + $this->payload = $payload; + } + + /** + * @return array $criteria + */ + public function getCriteria(): array + { + return $this->criteria; + } + + public function hasCriteria(): bool + { + return !empty($this->criteria); + } } diff --git a/src/Core/Framework/Api/Sync/SyncService.php b/src/Core/Framework/Api/Sync/SyncService.php index 1313ea4c497..029732e7604 100644 --- a/src/Core/Framework/Api/Sync/SyncService.php +++ b/src/Core/Framework/Api/Sync/SyncService.php @@ -4,12 +4,17 @@ use Doctrine\DBAL\ConnectionException; use Shopware\Core\Framework\Adapter\Database\ReplicaConnection; +use Shopware\Core\Framework\Api\ApiException; use Shopware\Core\Framework\Api\Exception\InvalidSyncOperationException; use Shopware\Core\Framework\Context; +use Shopware\Core\Framework\DataAbstractionLayer\DefinitionInstanceRegistry; use Shopware\Core\Framework\DataAbstractionLayer\EntityWriteResult; use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityWrittenContainerEvent; use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityWrittenEvent; use Shopware\Core\Framework\DataAbstractionLayer\Indexing\EntityIndexerRegistry; +use Shopware\Core\Framework\DataAbstractionLayer\Search\Criteria; +use Shopware\Core\Framework\DataAbstractionLayer\Search\EntitySearcherInterface; +use Shopware\Core\Framework\DataAbstractionLayer\Search\RequestCriteriaBuilder; use Shopware\Core\Framework\DataAbstractionLayer\Write\EntityWriterInterface; use Shopware\Core\Framework\DataAbstractionLayer\Write\WriteContext; use Shopware\Core\Framework\Log\Package; @@ -24,7 +29,10 @@ class SyncService implements SyncServiceInterface */ public function __construct( private readonly EntityWriterInterface $writer, - private readonly EventDispatcherInterface $eventDispatcher + private readonly EventDispatcherInterface $eventDispatcher, + private readonly DefinitionInstanceRegistry $registry, + private readonly EntitySearcherInterface $searcher, + private readonly RequestCriteriaBuilder $criteriaBuilder ) { } @@ -40,6 +48,8 @@ public function sync(array $operations, Context $context, SyncBehavior $behavior $context = clone $context; + $this->loopOperations($operations, $context); + if (\count($behavior->getSkipIndexers())) { $context->addExtension(EntityIndexerRegistry::EXTENSION_INDEXER_SKIP, new ArrayEntity(['skips' => $behavior->getSkipIndexers()])); } @@ -113,4 +123,35 @@ private function getWrittenEntitiesByEvent(EntityWrittenContainerEvent $result): return $entities; } + + /** + * Function to loop through all operations and provide some special handling for wildcard operations, or other short hands + * + * @param SyncOperation[] $operations + */ + private function loopOperations(array $operations, Context $context): void + { + foreach ($operations as $operation) { + if ($operation->getAction() === SyncOperation::ACTION_DELETE && $operation->hasCriteria()) { + $this->handleCriteriaDelete($operation, $context); + + continue; + } + } + } + + private function handleCriteriaDelete(SyncOperation $operation, Context $context): void + { + $definition = $this->registry->getByEntityName($operation->getEntity()); + + $criteria = $this->criteriaBuilder->fromArray(['filter' => $operation->getCriteria()], new Criteria(), $definition, $context); + + if (empty($criteria->getFilters())) { + throw ApiException::invalidSyncCriteriaException($operation->getKey()); + } + + $ids = $this->searcher->search($definition, $criteria, $context); + + $operation->replacePayload(\array_values($ids->getIds())); + } } diff --git a/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml b/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml index d4f3121b783..44c666292ab 100644 --- a/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml +++ b/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml @@ -604,6 +604,9 @@ + + + diff --git a/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php b/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php index 48da2526393..1802aea3b71 100644 --- a/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php +++ b/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php @@ -238,4 +238,56 @@ public function testError(): void static::assertInstanceOf(WriteConstraintViolationException::class, $first); static::assertStringStartsWith('/manufacturers/1/translations', $first->getPath()); } + + public function testDeleteWithWildCards(): void + { + $ids = new IdsCollection(); + + $products = [ + (new ProductBuilder($ids, 'p1')) + ->price(100) + ->category('c1') + ->category('c2') + ->build(), + (new ProductBuilder($ids, 'p2')) + ->price(100) + ->category('c1') + ->category('c3') + ->build(), + (new ProductBuilder($ids, 'p3')) + ->price(100) + ->category('c4') + ->build(), + ]; + + $this->getContainer()->get('product.repository')->create($products, Context::createDefaultContext()); + + $operations = [ + new SyncOperation('delete-mapping', 'product_category', SyncOperation::ACTION_DELETE, [], [ + ['type' => 'or', 'queries' => [ + ['type' => 'equals', 'field' => 'categoryId', 'value' => $ids->get('c4')], + ['type' => 'equalsAny', 'field' => 'productId', 'value' => $ids->getList(['p1', 'p2'])], + ]], + ]), + new SyncOperation('new-mapping', 'product_category', SyncOperation::ACTION_UPSERT, [ + ['productId' => $ids->get('p1'), 'categoryId' => $ids->get('c1')], + ['productId' => $ids->get('p2'), 'categoryId' => $ids->get('c1')], + ['productId' => $ids->get('p3'), 'categoryId' => $ids->get('c1')], + ]), + ]; + + $this->service->sync($operations, Context::createDefaultContext(), new SyncBehavior()); + + $existing = $this->connection->fetchFirstColumn( + 'SELECT CONCAT(LOWER(HEX(product_id)), \'-\', LOWER(HEX(category_id))) FROM product_category WHERE product_id IN (:ids)', + ['ids' => Uuid::fromHexToBytesList($ids->getList(['p1', 'p2', 'p3']))], + ['ids' => ArrayParameterType::STRING] + ); + + static::assertCount(3, $existing); + + static::assertContains($ids->get('p1') . '-' . $ids->get('c1'), $existing); + static::assertContains($ids->get('p2') . '-' . $ids->get('c1'), $existing); + static::assertContains($ids->get('p3') . '-' . $ids->get('c1'), $existing); + } } diff --git a/tests/unit/php/Core/Framework/Api/Controller/SyncControllerTest.php b/tests/unit/php/Core/Framework/Api/Controller/SyncControllerTest.php new file mode 100644 index 00000000000..ed1acacc27b --- /dev/null +++ b/tests/unit/php/Core/Framework/Api/Controller/SyncControllerTest.php @@ -0,0 +1,64 @@ + 'or', + 'queries' => [ + ['type' => 'equals', 'field' => 'categoryId', 'value' => 'foo'], + ['type' => 'equalsAny', 'field' => 'productId', 'value' => ['bar']], + ], + ], + ]; + + $operations = [ + 'delete-mapping' => [ + 'action' => 'delete', + 'entity' => 'product', + 'criteria' => $criteria, + ], + ]; + + $request = new Request([], [], [], [], [], [], (string) \json_encode($operations)); + + $service = $this->createMock(SyncService::class); + $service->expects(static::once()) + ->method('sync') + ->willReturnCallback(function ($operations) use ($criteria) { + static::assertCount(1, $operations); + static::assertInstanceOf(SyncOperation::class, $operations[0]); + + $operation = $operations[0]; + static::assertSame('delete-mapping', $operation->getKey()); + static::assertSame('product', $operation->getEntity()); + static::assertSame('delete', $operation->getAction()); + static::assertEquals($criteria, $operation->getCriteria()); + + return new SyncResult([]); + }); + + $controller = new SyncController($service, new Serializer([], [new JsonEncoder()])); + + $controller->sync($request, Context::createDefaultContext()); + } +} diff --git a/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php b/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php index e3b1bb560b8..9774fdf2eab 100644 --- a/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php +++ b/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php @@ -3,13 +3,23 @@ namespace Shopware\Tests\Unit\Core\Framework\Api\Sync; use PHPUnit\Framework\TestCase; +use Shopware\Core\Content\Product\Aggregate\ProductCategory\ProductCategoryDefinition; +use Shopware\Core\Content\Product\ProductDefinition; use Shopware\Core\Framework\Api\Sync\SyncBehavior; use Shopware\Core\Framework\Api\Sync\SyncOperation; use Shopware\Core\Framework\Api\Sync\SyncService; use Shopware\Core\Framework\Context; use Shopware\Core\Framework\DataAbstractionLayer\EntityWriteResult; +use Shopware\Core\Framework\DataAbstractionLayer\Search\Criteria; +use Shopware\Core\Framework\DataAbstractionLayer\Search\EntitySearcherInterface; +use Shopware\Core\Framework\DataAbstractionLayer\Search\Filter\EqualsAnyFilter; +use Shopware\Core\Framework\DataAbstractionLayer\Search\IdSearchResult; +use Shopware\Core\Framework\DataAbstractionLayer\Search\RequestCriteriaBuilder; +use Shopware\Core\Framework\DataAbstractionLayer\Write\EntityWriteGatewayInterface; use Shopware\Core\Framework\DataAbstractionLayer\Write\EntityWriterInterface; use Shopware\Core\Framework\DataAbstractionLayer\Write\WriteResult; +use Shopware\Tests\Unit\Common\Stubs\DataAbstractionLayer\StaticDefinitionInstanceRegistry; +use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** @@ -40,6 +50,13 @@ public function testSyncSingleOperation(): void $service = new SyncService( $writer, $this->createMock(EventDispatcherInterface::class), + new StaticDefinitionInstanceRegistry( + [ProductDefinition::class], + $this->createMock(ValidatorInterface::class), + $this->createMock(EntityWriteGatewayInterface::class), + ), + $this->createMock(EntitySearcherInterface::class), + $this->createMock(RequestCriteriaBuilder::class) ); $upsert = new SyncOperation('foo', 'product', SyncOperation::ACTION_UPSERT, [ @@ -69,4 +86,84 @@ public function testSyncSingleOperation(): void static::assertSame([], $result->getNotFound()); } + + public function testWildcardDeleteForMappingEntities(): void + { + $writer = $this->createMock(EntityWriterInterface::class); + $writer + ->expects(static::once()) + ->method('sync') + ->willReturnCallback(function ($operations) { + static::assertCount(1, $operations); + static::assertInstanceOf(SyncOperation::class, $operations[0]); + + $operation = $operations[0]; + + static::assertCount(4, $operation->getPayload()); + + $map = \array_map(function (array $payload) { + return $payload['productId'] . '-' . $payload['categoryId']; + }, $operation->getPayload()); + + static::assertContains('product-1-category-1', $map); + static::assertContains('product-1-category-2', $map); + static::assertContains('product-2-category-1', $map); + static::assertContains('product-2-category-2', $map); + + return new WriteResult([]); + }); + + $searcher = $this->createMock(EntitySearcherInterface::class); + + $criteriaBuilder = $this->createMock(RequestCriteriaBuilder::class); + + $filter = [ + ['type' => 'equalsAny', 'field' => 'productId', 'value' => ['product-1', 'product-2']], + ]; + + $criteria = new Criteria(); + $criteria->addFilter(new EqualsAnyFilter('productId', ['product-1', 'product-2'])); + + $criteriaBuilder->expects(static::once()) + ->method('fromArray') + ->with(['filter' => $filter]) + ->willReturn($criteria); + + $data = [ + ['primaryKey' => ['productId' => 'product-1', 'categoryId' => 'category-1'], 'data' => []], + ['primaryKey' => ['productId' => 'product-1', 'categoryId' => 'category-2'], 'data' => []], + ['primaryKey' => ['productId' => 'product-2', 'categoryId' => 'category-1'], 'data' => []], + ['primaryKey' => ['productId' => 'product-2', 'categoryId' => 'category-2'], 'data' => []], + ]; + + $ids = new IdSearchResult(4, $data, new Criteria(), Context::createDefaultContext()); + + $searcher->expects(static::once()) + ->method('search') + ->willReturn($ids); + + $service = new SyncService( + $writer, + $this->createMock(EventDispatcherInterface::class), + new StaticDefinitionInstanceRegistry( + [ProductCategoryDefinition::class], + $this->createMock(ValidatorInterface::class), + $this->createMock(EntityWriteGatewayInterface::class), + ), + $searcher, + $criteriaBuilder + ); + + $delete = new SyncOperation( + 'delete-mapping', + 'product_category', + SyncOperation::ACTION_DELETE, + [], + $filter + ); + + $behavior = new SyncBehavior('disable-indexing', ['product.indexer']); + + $service->sync([$delete], Context::createDefaultContext(), $behavior); + } } From cc5c28ab48e3de851b70aa2102b969af46e8e050 Mon Sep 17 00:00:00 2001 From: Oliver Skroblin Date: Wed, 19 Apr 2023 19:16:21 +0200 Subject: [PATCH 2/2] NEXT-25734 - Allow foreign key resolving by different unique constraint in sync api calls --- .../Content/DependencyInjection/product.xml | 5 + .../Content/DependencyInjection/property.xml | 1 - .../Product/Api/ProductNumberFkResolver.php | 55 +++++++ src/Core/Framework/Api/ApiException.php | 22 ++- .../Api/Controller/SyncController.php | 8 +- .../Framework/Api/Sync/AbstractFkResolver.php | 21 +++ src/Core/Framework/Api/Sync/FkReference.php | 18 +++ .../Framework/Api/Sync/SyncFkResolver.php | 143 ++++++++++++++++++ src/Core/Framework/Api/Sync/SyncService.php | 14 +- .../data-abstraction-layer.xml | 6 + .../Api/Controller/SyncControllerTest.php | 8 +- .../Framework}/Api/Sync/SyncServiceTest.php | 51 ++++++- .../Framework/Api/Sync/SyncFkResolverTest.php | 95 ++++++++++++ .../Framework/Api/Sync/SyncServiceTest.php | 7 +- 14 files changed, 443 insertions(+), 11 deletions(-) create mode 100644 src/Core/Content/Product/Api/ProductNumberFkResolver.php create mode 100644 src/Core/Framework/Api/Sync/AbstractFkResolver.php create mode 100644 src/Core/Framework/Api/Sync/FkReference.php create mode 100644 src/Core/Framework/Api/Sync/SyncFkResolver.php rename {src/Core/Framework/Test => tests/integration/php/Core/Framework}/Api/Sync/SyncServiceTest.php (87%) create mode 100644 tests/unit/php/Core/Framework/Api/Sync/SyncFkResolverTest.php diff --git a/src/Core/Content/DependencyInjection/product.xml b/src/Core/Content/DependencyInjection/product.xml index a6e2b678eb0..ee450a1238f 100644 --- a/src/Core/Content/DependencyInjection/product.xml +++ b/src/Core/Content/DependencyInjection/product.xml @@ -508,5 +508,10 @@ + + + + + diff --git a/src/Core/Content/DependencyInjection/property.xml b/src/Core/Content/DependencyInjection/property.xml index e8470a9121c..850c47a8dae 100644 --- a/src/Core/Content/DependencyInjection/property.xml +++ b/src/Core/Content/DependencyInjection/property.xml @@ -20,6 +20,5 @@ - diff --git a/src/Core/Content/Product/Api/ProductNumberFkResolver.php b/src/Core/Content/Product/Api/ProductNumberFkResolver.php new file mode 100644 index 00000000000..f3857e77c6a --- /dev/null +++ b/src/Core/Content/Product/Api/ProductNumberFkResolver.php @@ -0,0 +1,55 @@ + $map + * + * @return array + */ + public function resolve(array $map): array + { + $numbers = \array_map(fn ($id) => $id->value, $map); + + $numbers = \array_filter(\array_unique($numbers)); + + if (empty($numbers)) { + return $map; + } + + $hash = $this->connection->fetchAllKeyValue( + 'SELECT product_number, LOWER(HEX(id)) FROM product WHERE product_number IN (:numbers) AND version_id = :version', + ['numbers' => $numbers, 'version' => Uuid::fromHexToBytes(Defaults::LIVE_VERSION)], + ['numbers' => ArrayParameterType::STRING] + ); + + foreach ($map as $reference) { + $reference->resolved = $hash[$reference->value]; + } + + return $map; + } +} diff --git a/src/Core/Framework/Api/ApiException.php b/src/Core/Framework/Api/ApiException.php index ee0cdd31d2b..f50920e6a1b 100644 --- a/src/Core/Framework/Api/ApiException.php +++ b/src/Core/Framework/Api/ApiException.php @@ -10,13 +10,33 @@ class ApiException extends HttpException { public const API_INVALID_SYNC_CRITERIA_EXCEPTION = 'API_INVALID_SYNC_CRITERIA_EXCEPTION'; + public const API_RESOLVER_NOT_FOUND_EXCEPTION = 'API_RESOLVER_NOT_FOUND_EXCEPTION'; + public const API_INVALID_SYNC_OPERATION_EXCEPTION = 'FRAMEWORK__INVALID_SYNC_OPERATION'; public static function invalidSyncCriteriaException(string $operationKey): self { return new self( Response::HTTP_BAD_REQUEST, self::API_INVALID_SYNC_CRITERIA_EXCEPTION, - \sprintf('Sync operation %s, with action "delete", requires a criteria with at least one filter', $operationKey) + \sprintf('Sync operation %s, with action "delete", requires a criteria with at least one filter and can only be applied for mapping entities', $operationKey) + ); + } + + public static function invalidSyncOperationException(string $message): self + { + return new self( + Response::HTTP_BAD_REQUEST, + self::API_INVALID_SYNC_OPERATION_EXCEPTION, + $message + ); + } + + public static function resolverNotFoundException(string $key): self + { + return new self( + Response::HTTP_BAD_REQUEST, + self::API_RESOLVER_NOT_FOUND_EXCEPTION, + \sprintf('Foreign key resolver for key %s not found', $key) ); } } diff --git a/src/Core/Framework/Api/Controller/SyncController.php b/src/Core/Framework/Api/Controller/SyncController.php index 3f3d4d74535..cb9f23b1cf2 100644 --- a/src/Core/Framework/Api/Controller/SyncController.php +++ b/src/Core/Framework/Api/Controller/SyncController.php @@ -3,6 +3,7 @@ namespace Shopware\Core\Framework\Api\Controller; use Doctrine\DBAL\ConnectionException; +use Shopware\Core\Framework\Api\ApiException; use Shopware\Core\Framework\Api\Exception\InvalidSyncOperationException; use Shopware\Core\Framework\Api\Sync\SyncBehavior; use Shopware\Core\Framework\Api\Sync\SyncOperation; @@ -56,13 +57,18 @@ public function sync(Request $request, Context $context): JsonResponse if (isset($operation['key'])) { $key = $operation['key']; } + $key = (string) $key; $operations[] = new SyncOperation( - (string) $key, + $key, $operation['entity'], $operation['action'], $operation['payload'] ?? [], $operation['criteria'] ?? [] ); + + if (empty($operation['entity'])) { + throw ApiException::invalidSyncOperationException(sprintf('Missing "entity" argument for operation with key "%s". It needs to be a non-empty string.', (string) $key)); + } } $result = $context->scope(Context::CRUD_API_SCOPE, fn (Context $context): SyncResult => $this->syncService->sync($operations, $context, $behavior)); diff --git a/src/Core/Framework/Api/Sync/AbstractFkResolver.php b/src/Core/Framework/Api/Sync/AbstractFkResolver.php new file mode 100644 index 00000000000..107435cdfe9 --- /dev/null +++ b/src/Core/Framework/Api/Sync/AbstractFkResolver.php @@ -0,0 +1,21 @@ + $map + * + * @return array + */ + abstract public function resolve(array $map): array; +} diff --git a/src/Core/Framework/Api/Sync/FkReference.php b/src/Core/Framework/Api/Sync/FkReference.php new file mode 100644 index 00000000000..5a05f2c2a12 --- /dev/null +++ b/src/Core/Framework/Api/Sync/FkReference.php @@ -0,0 +1,18 @@ + $resolvers + */ + public function __construct( + private readonly DefinitionInstanceRegistry $registry, + private readonly iterable $resolvers + ) { + } + + /** + * @param array> $payload + * + * @return array> + */ + public function resolve(string $entity, array $payload): array + { + $map = $this->collect($entity, $payload); + + if (empty($map)) { + return $payload; + } + + foreach ($map as $key => &$values) { + $values = $this->getResolver($key)->resolve($values); + } + + \array_walk_recursive($payload, function (&$value): void { + $value = $value instanceof FkReference ? $value->resolved : $value; + }); + + return $payload; + } + + /** + * @param array> $payload + * + * @return array> + */ + private function collect(string $entity, array &$payload): array + { + $definition = $this->registry->getByEntityName($entity); + + $map = []; + foreach ($payload as &$row) { + foreach ($row as $key => &$value) { + if (\is_array($value) && isset($value['resolver']) && isset($value['value'])) { + $definition = $this->registry->getByEntityName($entity); + + $field = $definition->getField($key); + + $ref = match (true) { + $field instanceof FkField => $field->getReferenceDefinition()->getEntityName(), + $field instanceof IdField => $entity, + default => null + }; + + if ($ref === null) { + continue; + } + + $resolver = (string) $value['resolver']; + + $row[$key] = $reference = new FkReference($value['value']); + + $map[$resolver][] = $reference; + } + + if (\is_array($value)) { + $field = $definition->getField($key); + + if (!$field instanceof AssociationField) { + continue; + } + + $nested = []; + if ($field instanceof ManyToManyAssociationField || $field instanceof OneToManyAssociationField) { + $ref = $field instanceof ManyToManyAssociationField ? $field->getToManyReferenceDefinition()->getEntityName() : $field->getReferenceDefinition()->getEntityName(); + $nested = $this->collect($ref, $value); + } elseif ($field instanceof ManyToOneAssociationField || $field instanceof OneToOneAssociationField) { + $tmp = [$value]; + $nested = $this->collect($field->getReferenceDefinition()->getEntityName(), $tmp); + $value = \array_shift($tmp); + } + + $map = $this->merge($map, $nested); + } + } + } + + return $map; + } + + /** + * @param array> $map + * @param array> $nested + * + * @return array> + */ + private function merge(array $map, array $nested): array + { + foreach ($nested as $resolver => $values) { + foreach ($values as $value) { + $map[$resolver][] = $value; + } + } + + return $map; + } + + private function getResolver(string $key): AbstractFkResolver + { + foreach ($this->resolvers as $resolver) { + if ($resolver::getName() === $key) { + return $resolver; + } + } + + throw ApiException::resolverNotFoundException($key); + } +} diff --git a/src/Core/Framework/Api/Sync/SyncService.php b/src/Core/Framework/Api/Sync/SyncService.php index 029732e7604..c8b7c8a4ceb 100644 --- a/src/Core/Framework/Api/Sync/SyncService.php +++ b/src/Core/Framework/Api/Sync/SyncService.php @@ -12,6 +12,7 @@ use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityWrittenContainerEvent; use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityWrittenEvent; use Shopware\Core\Framework\DataAbstractionLayer\Indexing\EntityIndexerRegistry; +use Shopware\Core\Framework\DataAbstractionLayer\MappingEntityDefinition; use Shopware\Core\Framework\DataAbstractionLayer\Search\Criteria; use Shopware\Core\Framework\DataAbstractionLayer\Search\EntitySearcherInterface; use Shopware\Core\Framework\DataAbstractionLayer\Search\RequestCriteriaBuilder; @@ -32,7 +33,8 @@ public function __construct( private readonly EventDispatcherInterface $eventDispatcher, private readonly DefinitionInstanceRegistry $registry, private readonly EntitySearcherInterface $searcher, - private readonly RequestCriteriaBuilder $criteriaBuilder + private readonly RequestCriteriaBuilder $criteriaBuilder, + private readonly SyncFkResolver $syncFkResolver ) { } @@ -137,6 +139,12 @@ private function loopOperations(array $operations, Context $context): void continue; } + + if ($operation->getAction() === SyncOperation::ACTION_UPSERT) { + $resolved = $this->syncFkResolver->resolve($operation->getEntity(), $operation->getPayload()); + + $operation->replacePayload($resolved); + } } } @@ -144,6 +152,10 @@ private function handleCriteriaDelete(SyncOperation $operation, Context $context { $definition = $this->registry->getByEntityName($operation->getEntity()); + if (!$definition instanceof MappingEntityDefinition) { + throw ApiException::invalidSyncCriteriaException($operation->getKey()); + } + $criteria = $this->criteriaBuilder->fromArray(['filter' => $operation->getCriteria()], new Criteria(), $definition, $context); if (empty($criteria->getFilters())) { diff --git a/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml b/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml index 44c666292ab..d8fe1e8620c 100644 --- a/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml +++ b/src/Core/Framework/DependencyInjection/data-abstraction-layer.xml @@ -607,6 +607,12 @@ + + + + + + diff --git a/src/Core/Framework/Test/Api/Controller/SyncControllerTest.php b/src/Core/Framework/Test/Api/Controller/SyncControllerTest.php index 74b2abdb457..745ca369201 100644 --- a/src/Core/Framework/Test/Api/Controller/SyncControllerTest.php +++ b/src/Core/Framework/Test/Api/Controller/SyncControllerTest.php @@ -54,8 +54,8 @@ public function testMultipleProductInsert(): void 'id' => $id1, 'productNumber' => Uuid::randomHex(), 'stock' => 1, - 'manufacturer' => ['name' => 'test'], - 'tax' => ['name' => 'test', 'taxRate' => 15], + 'manufacturer' => ['name' => 'manufacturer'], + 'tax' => ['name' => 'tax', 'taxRate' => 15], 'name' => 'CREATE-1', 'price' => [['currencyId' => Defaults::CURRENCY, 'gross' => 50, 'net' => 25, 'linked' => false]], ], @@ -63,9 +63,9 @@ public function testMultipleProductInsert(): void 'id' => $id2, 'productNumber' => Uuid::randomHex(), 'stock' => 1, - 'manufacturer' => ['name' => 'test'], + 'manufacturer' => ['name' => 'manufacturer'], 'name' => 'CREATE-2', - 'tax' => ['name' => 'test', 'taxRate' => 15], + 'tax' => ['name' => 'tax', 'taxRate' => 15], 'price' => [['currencyId' => Defaults::CURRENCY, 'gross' => 50, 'net' => 25, 'linked' => false]], ], ], diff --git a/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php b/tests/integration/php/Core/Framework/Api/Sync/SyncServiceTest.php similarity index 87% rename from src/Core/Framework/Test/Api/Sync/SyncServiceTest.php rename to tests/integration/php/Core/Framework/Api/Sync/SyncServiceTest.php index 1802aea3b71..620fdb51497 100644 --- a/src/Core/Framework/Test/Api/Sync/SyncServiceTest.php +++ b/tests/integration/php/Core/Framework/Api/Sync/SyncServiceTest.php @@ -1,6 +1,6 @@ get('p2') . '-' . $ids->get('c1'), $existing); static::assertContains($ids->get('p3') . '-' . $ids->get('c1'), $existing); } + + public function testResolveForeignKeys(): void + { + $ids = new IdsCollection(); + + $products = [ + (new ProductBuilder($ids, 'p1')) + ->price(100) + ->build(), + ]; + + $this->getContainer()->get('product.repository')->create($products, Context::createDefaultContext()); + + $categories = [ + ['id' => $ids->create('c1'), 'name' => 'c1'], + ['id' => $ids->create('c2'), 'name' => 'c2'], + ]; + + $this->getContainer()->get('category.repository')->create($categories, Context::createDefaultContext()); + + $operations = [ + new SyncOperation( + 'map-categories', + 'product_category', + SyncOperation::ACTION_UPSERT, + [ + [ + 'categoryId' => $ids->get('c1'), + 'productId' => ['resolver' => 'product.number', 'value' => 'p1'], + ], + [ + 'categoryId' => $ids->get('c2'), + 'productId' => ['resolver' => 'product.number', 'value' => 'p1'], + ], + ] + ), + ]; + + $this->service->sync($operations, Context::createDefaultContext(), new SyncBehavior()); + + $existing = $this->connection->fetchFirstColumn( + 'SELECT LOWER(HEX(category_id)) FROM product_category WHERE product_id = :id', + ['id' => $ids->getBytes('p1')] + ); + + static::assertCount(2, $existing); + static::assertContains($ids->get('c1'), $existing); + static::assertContains($ids->get('c2'), $existing); + } } diff --git a/tests/unit/php/Core/Framework/Api/Sync/SyncFkResolverTest.php b/tests/unit/php/Core/Framework/Api/Sync/SyncFkResolverTest.php new file mode 100644 index 00000000000..81505825bfd --- /dev/null +++ b/tests/unit/php/Core/Framework/Api/Sync/SyncFkResolverTest.php @@ -0,0 +1,95 @@ + ['resolver' => 'dummy', 'value' => 't1'], + + // many-to-many-case + 'categories' => [ + ['id' => ['resolver' => 'dummy', 'value' => 'c1']], + ['id' => ['resolver' => 'dummy', 'value' => 'c2']], + ], + + // nesting case + 'visibilities' => [ + ['visibility' => 1, 'salesChannelId' => ['resolver' => 'dummy', 'value' => 's1']], + ], + ]; + + $resolver = new SyncFkResolver( + new StaticDefinitionInstanceRegistry( + [ + ProductDefinition::class, + TaxDefinition::class, + ProductVisibilityDefinition::class, + SalesChannelDefinition::class, + CategoryDefinition::class, + ProductCategoryDefinition::class, + ], + $this->createMock(ValidatorInterface::class), + $this->createMock(EntityWriteGatewayInterface::class) + ), + [new DummyFkResolver()] + ); + + $resolved = $resolver->resolve('product', [$payload]); + + $expected = [ + 'taxId' => 't1', + 'categories' => [ + ['id' => 'c1'], + ['id' => 'c2'], + ], + 'visibilities' => [ + ['visibility' => 1, 'salesChannelId' => 's1'], + ], + ]; + + static::assertCount(1, $resolved); + static::assertEquals($expected, $resolved[0]); + } +} + +/** + * @internal + */ +class DummyFkResolver extends AbstractFkResolver +{ + public static function getName(): string + { + return 'dummy'; + } + + public function resolve(array $map): array + { + foreach ($map as $value) { + $value->resolved = $value->value; + } + + return $map; + } +} diff --git a/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php b/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php index 9774fdf2eab..e2420141f3d 100644 --- a/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php +++ b/tests/unit/php/Core/Framework/Api/Sync/SyncServiceTest.php @@ -6,6 +6,7 @@ use Shopware\Core\Content\Product\Aggregate\ProductCategory\ProductCategoryDefinition; use Shopware\Core\Content\Product\ProductDefinition; use Shopware\Core\Framework\Api\Sync\SyncBehavior; +use Shopware\Core\Framework\Api\Sync\SyncFkResolver; use Shopware\Core\Framework\Api\Sync\SyncOperation; use Shopware\Core\Framework\Api\Sync\SyncService; use Shopware\Core\Framework\Context; @@ -56,7 +57,8 @@ public function testSyncSingleOperation(): void $this->createMock(EntityWriteGatewayInterface::class), ), $this->createMock(EntitySearcherInterface::class), - $this->createMock(RequestCriteriaBuilder::class) + $this->createMock(RequestCriteriaBuilder::class), + $this->createMock(SyncFkResolver::class) ); $upsert = new SyncOperation('foo', 'product', SyncOperation::ACTION_UPSERT, [ @@ -151,7 +153,8 @@ public function testWildcardDeleteForMappingEntities(): void $this->createMock(EntityWriteGatewayInterface::class), ), $searcher, - $criteriaBuilder + $criteriaBuilder, + $this->createMock(SyncFkResolver::class) ); $delete = new SyncOperation(