diff --git a/.github/workflows/_lint.yaml b/.github/workflows/_lint.yaml index db4d88d..0913651 100644 --- a/.github/workflows/_lint.yaml +++ b/.github/workflows/_lint.yaml @@ -18,6 +18,18 @@ jobs: - name: Run PHP-CS-Fixer run: composer run lint-composer + lint-php: + name: Lint PHP + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - uses: ./.github/actions/composer + with: + composer-install: 'false' + - name: Run PHP-CS-Fixer + run: composer run lint-php + php-cs-fixer: name: PHP-CS-Fixer runs-on: ubuntu-latest @@ -45,6 +57,8 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - uses: ./.github/actions/composer + with: + composer-install: 'false' - uses: actions/setup-node@v4 with: node-version: 22 diff --git a/composer.json b/composer.json index 254ab8a..335e5b8 100644 --- a/composer.json +++ b/composer.json @@ -65,6 +65,7 @@ }, "lint": [ "@lint-composer", + "@lint-php", "@lint-php-cs-fixer", "@lint-phpstan", "@lint-prettier" @@ -76,6 +77,7 @@ "@lint-php-cs-fixer-fix", "@lint-prettier-fix" ], + "lint-php": "failed=1; find . -name \"*.php\" -not -path \"./vendor/*\" -not -path \"./var/*\" -exec php -l \"{}\" \\; 2>&1 | grep \"PHP Parse error\" || failed=0; test \"$failed\" -eq \"0\"", "lint-php-cs-fixer": "vendor/bin/php-cs-fixer fix --diff --dry-run", "lint-php-cs-fixer-fix": "vendor/bin/php-cs-fixer fix", "lint-phpstan": [ @@ -92,7 +94,7 @@ "phpunit": "@php vendor/bin/phpunit", "phpunit-html": [ "rm public/tests -rf", - "@php vendor/bin/phpunit --coverage-html public/tests" + "@php -dextension=pcov.so -dpcov.enabled=1 vendor/bin/phpunit --coverage-html public/tests" ], "phpunit-report": "@php -dpcov.enabled=1 vendor/bin/phpunit --coverage-html public/tests --coverage-clover reports/coverage.clover.xml --coverage-text --colors=always --testdox" } diff --git a/phpstan.neon b/phpstan.neon index 01ae162..666c121 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -40,7 +40,6 @@ parameters: - identifier: property.internalClass - identifier: property.internalInterface - identifier: return.internalClass - - identifier: staticMethod.internal services: - # register the class, so we can decorate it, but don't tag it as a rule, so only our decorator is used by PHPStan diff --git a/src/ApiDefinition/EntitySchemaGeneratorDecorator.php b/src/ApiDefinition/EntitySchemaGeneratorDecorator.php index 9eb6995..fb0c71b 100644 --- a/src/ApiDefinition/EntitySchemaGeneratorDecorator.php +++ b/src/ApiDefinition/EntitySchemaGeneratorDecorator.php @@ -22,7 +22,7 @@ */ final class EntitySchemaGeneratorDecorator extends EntitySchemaGenerator { - private const array CLEANUP_PROPERTY_DEFINITION = [ + private const CLEANUP_PROPERTY_DEFINITION = [ 'type' => 'boolean', 'flags' => [ // Reading is never allowed, the field is not returned by the API @@ -32,7 +32,12 @@ final class EntitySchemaGeneratorDecorator extends EntitySchemaGenerator ], ]; - public function __construct(private ApiDefinitionGeneratorInterface $decorated) {} + /** + * @codeCoverageIgnore + */ + public function __construct( + private readonly ApiDefinitionGeneratorInterface $decorated, + ) {} /** * @codeCoverageIgnore diff --git a/src/DataAbstractionLayer/CleanupRelationData.php b/src/DataAbstractionLayer/CleanupRelationData.php index f8c9fc9..aa1feca 100644 --- a/src/DataAbstractionLayer/CleanupRelationData.php +++ b/src/DataAbstractionLayer/CleanupRelationData.php @@ -19,10 +19,10 @@ final class CleanupRelationData * @param array $relatedPrimaryKeyFields */ public function __construct( - readonly public EntityDefinition $definition, - readonly public array $parentPrimaryKey, - readonly public array $parentPrimaryKeyFields, - readonly public array $relatedPrimaryKeyFields, + public readonly EntityDefinition $definition, + public readonly array $parentPrimaryKey, + public readonly array $parentPrimaryKeyFields, + public readonly array $relatedPrimaryKeyFields, ) {} /** diff --git a/src/DataAbstractionLayer/ObsoleteRelationsDeleter.php b/src/DataAbstractionLayer/ObsoleteRelationsDeleter.php index 327500e..9316de4 100644 --- a/src/DataAbstractionLayer/ObsoleteRelationsDeleter.php +++ b/src/DataAbstractionLayer/ObsoleteRelationsDeleter.php @@ -93,7 +93,7 @@ private function getDeletePrimaryKeys(CleanupRelationData $cleanupRelation, Cont $this->getMainPrimaryKey($cleanupRelation->parentPrimaryKey), ), ); - $criteria->addFilter(new NotFilter(MultiFilter::CONNECTION_AND, $existingIdFilters)); + $criteria->addFilter(new NotFilter(MultiFilter::CONNECTION_OR, $existingIdFilters)); return $repository->searchIds($criteria, $context)->getIds(); } diff --git a/src/DataAbstractionLayer/WriteCommandExtractorDecorator.php b/src/DataAbstractionLayer/WriteCommandExtractorDecorator.php index 60a69c4..337813c 100644 --- a/src/DataAbstractionLayer/WriteCommandExtractorDecorator.php +++ b/src/DataAbstractionLayer/WriteCommandExtractorDecorator.php @@ -127,7 +127,7 @@ private function filterVersionFields(array $primaryKeys, EntityDefinition $defin private function getForeignKeyPropertyNameByStorageName( CompiledFieldCollection $fields, string $storageName, - ) { + ): string { $fk = $fields->getByStorageName($storageName); assert( diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeDefinition.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeDefinition.php index b471324..e8704ac 100644 --- a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeDefinition.php +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeDefinition.php @@ -14,7 +14,7 @@ class PropertyGroupOptionExcludeDefinition extends MappingEntityDefinition { - final public const string ENTITY_NAME = 'property_group_option_exclude'; + final public const ENTITY_NAME = 'property_group_option_exclude'; public function getEntityName(): string { diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeExtension.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeExtension.php index ba78bfc..0706e0a 100644 --- a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeExtension.php +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/PropertyGroupOptionExcludeExtension.php @@ -13,7 +13,7 @@ class PropertyGroupOptionExcludeExtension extends EntityExtension { - public const string EXTENSION_NAME = 'excludedOptions'; + public const EXTENSION_NAME = 'excludedOptions'; public function extendFields(FieldCollection $collection): void { diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildCollection.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildCollection.php new file mode 100644 index 0000000..f3c741c --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildCollection.php @@ -0,0 +1,18 @@ + + */ +class VersionedChildCollection extends EntityCollection +{ + protected function getExpectedClass(): string + { + return VersionedChildEntity::class; + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildDefinition.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildDefinition.php new file mode 100644 index 0000000..dd5ed87 --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildDefinition.php @@ -0,0 +1,58 @@ +addFlags(new PrimaryKey(), new Required(), new ApiAware()), + new VersionField(), + (new ReferenceVersionField(VersionedParentDefinition::class, 'parent_version_id')) + ->addFlags(new Required(), new ApiAware()), + (new StringField('name', 'name'))->addFlags(new Required(), new ApiAware()), + + (new FkField('parent_id', 'parentId', VersionedParentDefinition::class)) + ->addFlags(new Required(), new ApiAware()), + (new ManyToOneAssociationField('parent', 'parent_id', VersionedParentDefinition::class)) + ->addFlags(new ApiAware()), + ]); + } + + protected function getParentDefinitionClass(): ?string + { + return VersionedParentDefinition::class; + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildEntity.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildEntity.php new file mode 100644 index 0000000..98be6e9 --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedChildEntity.php @@ -0,0 +1,61 @@ +name; + } + + public function getParent(): ?VersionedParentEntity + { + return $this->parent; + } + + public function getParentId(): ?string + { + return $this->parentId; + } + + public function getParentVersionId(): string + { + return $this->parentVersionId; + } + + public function setName(?string $name): void + { + $this->name = $name; + } + + public function setParent(?VersionedParentEntity $parent): void + { + $this->parent = $parent; + } + + public function setParentId(string $parentId): void + { + $this->parentId = $parentId; + } + + public function setParentVersionId(string $parentVersionId): void + { + $this->parentVersionId = $parentVersionId; + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentCollection.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentCollection.php new file mode 100644 index 0000000..c00be60 --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentCollection.php @@ -0,0 +1,18 @@ + + */ +class VersionedParentCollection extends EntityCollection +{ + protected function getExpectedClass(): string + { + return VersionedParentEntity::class; + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentDefinition.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentDefinition.php new file mode 100644 index 0000000..164cf9e --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentDefinition.php @@ -0,0 +1,52 @@ +addFlags(new PrimaryKey(), new Required(), new ApiAware()), + (new VersionField())->addFlags(new ApiAware()), + (new ReferenceVersionField(self::class, 'parent_version_id'))->addFlags(new Required(), new ApiAware()), + + (new StringField('name', 'name')) + ->addFlags(new Required(), new ApiAware()), + + (new OneToManyAssociationField('children', VersionedChildDefinition::class, 'parent_id')) + ->addFlags(new CascadeDelete(), new ApiAware()), + ]); + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentEntity.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentEntity.php new file mode 100644 index 0000000..50c2f36 --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Entity/VersionedParentEntity.php @@ -0,0 +1,49 @@ +children; + } + + public function getName(): string + { + return $this->name; + } + + public function getParentVersionId(): string + { + return $this->parentVersionId; + } + + public function setChildren(VersionedChildCollection $children): void + { + $this->children = $children; + } + + public function setName(string $name): void + { + $this->name = $name; + } + + public function setParentVersionId(string $parentVersionId): void + { + $this->parentVersionId = $parentVersionId; + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Migration/Migration1756750652VersionedRelation.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Migration/Migration1756750652VersionedRelation.php new file mode 100644 index 0000000..cbc38ab --- /dev/null +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Migration/Migration1756750652VersionedRelation.php @@ -0,0 +1,54 @@ +executeStatement($sql); + + $sql = <<<'SQL' + CREATE TABLE versioned_child + ( + id binary(16) NOT NULL, + version_id binary(16) NOT NULL, + parent_id binary(16) NOT NULL, + parent_version_id binary(16) NOT NULL, + name varchar(255) NOT NULL, + created_at datetime(3) NOT NULL, + updated_at datetime(3) NULL, + related_property_group_id binary(16) NULL, + PRIMARY KEY (id, version_id), + CONSTRAINT `fk.versioned_child.parent_id` + FOREIGN KEY (parent_id, parent_version_id) REFERENCES versioned_parent (id, version_id) ON UPDATE CASCADE ON DELETE CASCADE + ); + SQL; + + $connection->executeStatement($sql); + } +} diff --git a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Resources/config/services.php b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Resources/config/services.php index efedcbf..3395567 100644 --- a/tests/Fixtures/SmartRelationSyncTestPlugin/src/Resources/config/services.php +++ b/tests/Fixtures/SmartRelationSyncTestPlugin/src/Resources/config/services.php @@ -4,6 +4,8 @@ use Swh\SmartRelationSyncTestPlugin\Entity\PropertyGroupOptionExcludeDefinition; use Swh\SmartRelationSyncTestPlugin\Entity\PropertyGroupOptionExcludeExtension; +use Swh\SmartRelationSyncTestPlugin\Entity\VersionedChildDefinition; +use Swh\SmartRelationSyncTestPlugin\Entity\VersionedParentDefinition; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; return static function (ContainerConfigurator $containerConfigurator): void { @@ -15,4 +17,7 @@ $services->set(PropertyGroupOptionExcludeDefinition::class); $services->set(PropertyGroupOptionExcludeExtension::class); + + $services->set(VersionedParentDefinition::class); + $services->set(VersionedChildDefinition::class); }; diff --git a/tests/Functional/ApiDefinition/OpenApiDefinitionSchemaBuilderDecoratorTest.php b/tests/Functional/ApiDefinition/OpenApiDefinitionSchemaBuilderDecoratorTest.php index c7d259a..96c9e68 100644 --- a/tests/Functional/ApiDefinition/OpenApiDefinitionSchemaBuilderDecoratorTest.php +++ b/tests/Functional/ApiDefinition/OpenApiDefinitionSchemaBuilderDecoratorTest.php @@ -17,18 +17,7 @@ class OpenApiDefinitionSchemaBuilderDecoratorTest extends TestCase { use AdminFunctionalTestBehaviour; - /** - * @return non-empty-array[] - */ - public static function provideOpenApiTypes(): array - { - return [ - [DefinitionService::TYPE_JSON], - [DefinitionService::TYPE_JSON_API], - ]; - } - - #[DataProvider('provideOpenApiTypes')] + #[DataProvider('provideApiSchemaReturnsExpectedPropertiesCases')] public function testApiSchemaReturnsExpectedProperties(string $type): void { $this->getBrowser()->jsonRequest( @@ -70,4 +59,15 @@ public function testApiSchemaReturnsExpectedProperties(string $type): void self::assertIsArray($extensions['excludedOptionsCleanupRelations']); self::assertSame('boolean', $extensions['excludedOptionsCleanupRelations']['type']); } + + /** + * @return non-empty-array[] + */ + public static function provideApiSchemaReturnsExpectedPropertiesCases(): iterable + { + return [ + [DefinitionService::TYPE_JSON], + [DefinitionService::TYPE_JSON_API], + ]; + } } diff --git a/tests/Functional/DataAbstractionLayer/AbstractEntityWriteSubscriberTestCase.php b/tests/Functional/DataAbstractionLayer/AbstractEntityWriteSubscriberTestCase.php index 4290ecf..73bf152 100644 --- a/tests/Functional/DataAbstractionLayer/AbstractEntityWriteSubscriberTestCase.php +++ b/tests/Functional/DataAbstractionLayer/AbstractEntityWriteSubscriberTestCase.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Shopware\Core\Content\Category\CategoryCollection; +use Shopware\Core\Content\Product\Aggregate\ProductPrice\ProductPriceCollection; use Shopware\Core\Content\Product\ProductCollection; use Shopware\Core\Content\Product\ProductEntity; use Shopware\Core\Content\Property\Aggregate\PropertyGroupOption\PropertyGroupOptionCollection; @@ -18,12 +19,30 @@ use Shopware\Core\Framework\Test\TestCaseBase\IntegrationTestBehaviour; use Shopware\Core\Framework\Uuid\Uuid; use Swh\SmartRelationSync\Tests\Compatibility\IdsCollection; +use Swh\SmartRelationSyncTestPlugin\Entity\VersionedChildCollection; +use Swh\SmartRelationSyncTestPlugin\Entity\VersionedParentCollection; abstract class AbstractEntityWriteSubscriberTestCase extends TestCase { use IntegrationTestBehaviour; - private const string PRODUCT_NUMBER = 'P384584'; + private const PRODUCT_NUMBER = 'P384584'; + + private const VERSIONED_PARENT_PAYLOAD = [ + 'id' => 'c56368909789d4f61662603c97687a97', + 'name' => 'Parent test', + 'children' => [ + [ + 'id' => 'a9742e0d21d75726412c3e32cf9c08fa', + 'name' => 'Child test 1', + ], + [ + 'id' => '063595fd7ed5d047f29c83214fa967b8', + 'name' => 'Child test 2', + ], + ], + 'childrenCleanupRelations' => true, + ]; protected Context $context; @@ -37,21 +56,10 @@ abstract protected function upsertEntity(string $entity, array $payload): void; protected function setUp(): void { - $this->context = Context::createDefaultContext(); + $this->context = Context::createCLIContext(); $this->ids = new IdsCollection(); } - /** - * @return array - */ - public static function trueFalseDataProvider(): array - { - return [ - [true], - [false], - ]; - } - public function testSyncManyToMany(): void { $builder = $this->createProductBuilder() @@ -69,7 +77,7 @@ public function testSyncManyToMany(): void self::assertSame($this->ids->get('Test 2'), $categories?->first()?->getId()); } - #[DataProvider('trueFalseDataProvider')] + #[DataProvider('provideSyncManyToManyExtensionCases')] public function testSyncManyToManyExtension(bool $useExtensionsProperty): void { $excludedOption1 = ['id' => Uuid::randomHex(), 'name' => 'Excluded option 1', 'group' => ['name' => 'Group 1']]; @@ -106,6 +114,17 @@ public function testSyncManyToManyExtension(bool $useExtensionsProperty): void self::assertSame($excludedOption2['id'], $extension->first()?->getId()); } + /** + * @return array + */ + public static function provideSyncManyToManyExtensionCases(): iterable + { + return [ + [true], + [false], + ]; + } + public function testSyncManyToManyWithEmptyArray(): void { $builder = $this->createProductBuilder() @@ -172,6 +191,60 @@ public function testSyncOneToMany(): void self::assertSame($this->ids->get('test2'), $prices?->first()?->getRuleId()); } + public function testSyncOneToManyKeepsExisting(): void + { + $productBuilder = $this->createProductBuilder() + ->prices('test', 14.28); + + $this->upsertProductWithRelationCleanup($productBuilder->build()); + + $productBuilder = $productBuilder + ->prices('test2', 115.0); + + $this->upsertProductWithRelationCleanup($productBuilder->build()); + + $criteria = new Criteria([$this->ids->get(self::PRODUCT_NUMBER)]); + $criteria->addAssociation('prices'); + + $product = $this->searchProductSingle($criteria); + $prices = $product->getPrices() ?? new ProductPriceCollection(); + self::assertCount(2, $prices); + + $price1Id = Uuid::fromStringToHex($this->ids->get('test')); + self::assertSame(14.28, $prices->get($price1Id)?->getPrice()->first()?->getGross()); + + $price2Id = Uuid::fromStringToHex($this->ids->get('test2')); + self::assertSame(115.0, $prices->get($price2Id)?->getPrice()->first()?->getGross()); + } + + public function testSyncOneToManyWithVersioningKeepsExisting(): void + { + $this->upsertEntity('versioned_parent', self::VERSIONED_PARENT_PAYLOAD); + + $this->assertVersionedParentChildrenCount(2); + + $this->upsertEntity('versioned_parent', self::VERSIONED_PARENT_PAYLOAD); + + $this->assertVersionedParentChildrenCount(2); + } + + public function testSyncOneToManyWithVersioningReplacesEverything(): void + { + $payload = self::VERSIONED_PARENT_PAYLOAD; + + $this->upsertEntity('versioned_parent', $payload); + + $this->assertVersionedParentChildrenCount(2); + + $newId = Uuid::randomHex(); + + $payload['children'][0]['id'] = $newId; + $this->upsertEntity('versioned_parent', $payload); + + $children = $this->assertVersionedParentChildrenCount(2); + self::assertSame($newId, $children->first()?->getId()); + } + protected function loadCategories(): ?CategoryCollection { $criteria = new Criteria([$this->ids->get(self::PRODUCT_NUMBER)]); @@ -181,6 +254,20 @@ protected function loadCategories(): ?CategoryCollection return $product->getCategories(); } + private function assertVersionedParentChildrenCount(int $expectedCount): VersionedChildCollection + { + $criteria = new Criteria(['c56368909789d4f61662603c97687a97']); + $criteria->addAssociation('children'); + + $result = $this->getVersionedParentRepository()->search($criteria, $this->context)->first()?->getChildren(); + + self::assertInstanceOf(VersionedChildCollection::class, $result); + + self::assertCount($expectedCount, $result); + + return $result; + } + /** * @return array{ * id: non-empty-string, @@ -238,6 +325,17 @@ private function getProductRepository(): EntityRepository return $productRepository; } + /** + * @return EntityRepository + */ + private function getVersionedParentRepository(): EntityRepository + { + /** @var EntityRepository $productRepository */ + $productRepository = $this->getContainer()->get('versioned_parent.repository'); + assert($productRepository instanceof EntityRepository); + return $productRepository; + } + private function searchProductSingle(Criteria $criteria): ProductEntity { $product = $this->getProductRepository()->search($criteria, $this->context)->first(); @@ -255,6 +353,10 @@ private function upsertProductWithRelationCleanup(array $payload): void $payload['pricesCleanupRelations'] = true; $payload['categoriesCleanupRelations'] = true; + foreach ($payload['prices'] ?? [] as $key => $price) { + $payload['prices'][$key]['id'] = Uuid::fromStringToHex($price['rule']['id']); + } + $this->upsertEntity('product', $payload); } }