fix: resolve all dal:validate errors (#87)

CHANGELOG_de-DE.md

@@ -1,3 +1,6 @@
+# 5.3.1
+- Behoben: Alle 8 Fehler bei der Ausführung von `dal:validate` unter Shopware 6.7.14 [#87](https://github.com/Werkstattl/OpenBlogware/issues/87)
+
 # 5.3.0
 - Behoben: Falsche Kriterien-Associations [#84](https://github.com/Werkstattl/OpenBlogware/pull/84)
 - Behoben: Mehrere Fehler [#82](https://github.com/Werkstattl/OpenBlogware/pull/82)

CHANGELOG_en-GB.md

@@ -1,3 +1,6 @@
+# 5.3.1
+- Fixed: all 8 errors reported by `dal:validate` on Shopware 6.7.14 [#87](https://github.com/Werkstattl/OpenBlogware/issues/87)
+
 # 5.3.0
 - Fixed: wrong criteria associations [#84](https://github.com/Werkstattl/OpenBlogware/pull/84)
 - Fixed: several errors [#82](https://github.com/Werkstattl/OpenBlogware/pull/82)

src/Content/Blog/BlogEntryDefinition.php

@@ -17,13 +17,13 @@
 use Shopware\Core\Framework\DataAbstractionLayer\Field\IdField;
 use Shopware\Core\Framework\DataAbstractionLayer\Field\ManyToManyAssociationField;
 use Shopware\Core\Framework\DataAbstractionLayer\Field\ManyToOneAssociationField;
-use Shopware\Core\Framework\DataAbstractionLayer\Field\OneToOneAssociationField;
 use Shopware\Core\Framework\DataAbstractionLayer\Field\ReferenceVersionField;
 use Shopware\Core\Framework\DataAbstractionLayer\Field\TranslatedField;
 use Shopware\Core\Framework\DataAbstractionLayer\Field\TranslationsAssociationField;
 use Shopware\Core\Framework\DataAbstractionLayer\FieldCollection;
 use Shopware\Core\System\Tag\TagDefinition;
 use Werkl\OpenBlogware\Content\Blog\Aggregate\BlogEntryBlogCategoryMappingDefinition;
+use Werkl\OpenBlogware\Content\Blog\Aggregate\BlogEntryTagMappingDefinition;
 use Werkl\OpenBlogware\Content\Blog\BlogEntryTranslation\BlogEntryTranslationDefinition;
 use Werkl\OpenBlogware\Content\BlogAuthor\BlogAuthorDefinition;
 use Werkl\OpenBlogware\Content\BlogCategory\BlogCategoryDefinition;
@@ -61,7 +61,7 @@ protected function defineFields(): FieldCollection
 
             (new FkField('author_id', 'authorId', BlogAuthorDefinition::class))->addFlags(new Required()),
             (new FkField('cms_page_id', 'cmsPageId', CmsPageDefinition::class))->addFlags(new ApiAware(), new Inherited()),
-            (new ReferenceVersionField(CmsPageDefinition::class))->addFlags(new PrimaryKey(), new Required()),
+            (new ReferenceVersionField(CmsPageDefinition::class))->addFlags(new Required(), new ApiAware()),
 
             (new TranslatedField('mediaId'))->addFlags(new ApiAware()),
             (new TranslatedField('title'))->addFlags(new ApiAware(), new SearchRanking(SearchRanking::HIGH_SEARCH_RANKING)),
@@ -78,8 +78,8 @@ protected function defineFields(): FieldCollection
 
             (new ManyToManyAssociationField('blogCategories', BlogCategoryDefinition::class, BlogEntryBlogCategoryMappingDefinition::class, 'werkl_blog_entry_id', 'werkl_blog_category_id'))->addFlags(new CascadeDelete(), new ApiAware(), new SearchRanking(SearchRanking::ASSOCIATION_SEARCH_RANKING)),
             (new ManyToOneAssociationField('blogAuthor', 'author_id', BlogAuthorDefinition::class, 'id', false))->addFlags(new ApiAware(), new SearchRanking(SearchRanking::ASSOCIATION_SEARCH_RANKING)),
-            (new OneToOneAssociationField('cmsPage', 'cms_page_id', 'id', CmsPageDefinition::class, false))->addFlags(new ApiAware()),
-            (new ManyToManyAssociationField('tags', TagDefinition::class, 'werkl_blog_entry_tag', 'werkl_blog_entry_id', 'tag_id'))->addFlags(new ApiAware()),
+            (new ManyToOneAssociationField('cmsPage', 'cms_page_id', CmsPageDefinition::class, 'id', false))->addFlags(new ApiAware()),
+            (new ManyToManyAssociationField('tags', TagDefinition::class, BlogEntryTagMappingDefinition::class, 'werkl_blog_entry_id', 'tag_id'))->addFlags(new ApiAware()),
         ]);
     }
 }

src/Content/Blog/BlogEntryEntity.php

@@ -44,6 +44,10 @@ class BlogEntryEntity extends Entity
 
     protected ?string $mediaId = null;
 
+    /**
+     * @internal The teaser image is stored in the translations (mediaId), this association is
+     *           hydrated manually by the BlogSubscriber and is not backed by a DAL field.
+     */
     protected ?MediaEntity $media = null;
 
     protected \DateTimeInterface $publishedAt;
@@ -241,7 +245,7 @@ public function getTags(): ?TagCollection
         return $this->tags;
     }
 
-    public function setTags(?TagCollection $tags): void
+    public function setTags(TagCollection $tags): void
     {
         $this->tags = $tags;
     }

src/Content/Extension/TagExtension.php

@@ -0,0 +1,27 @@
+<?php
+declare(strict_types=1);
+
+namespace Werkl\OpenBlogware\Content\Extension;
+
+use Shopware\Core\Framework\DataAbstractionLayer\EntityExtension;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\Flag\CascadeDelete;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\ManyToManyAssociationField;
+use Shopware\Core\Framework\DataAbstractionLayer\FieldCollection;
+use Shopware\Core\System\Tag\TagDefinition;
+use Werkl\OpenBlogware\Content\Blog\Aggregate\BlogEntryTagMappingDefinition;
+use Werkl\OpenBlogware\Content\Blog\BlogEntryDefinition;
+
+class TagExtension extends EntityExtension
+{
+    public function extendFields(FieldCollection $collection): void
+    {
+        $collection->add(
+            (new ManyToManyAssociationField('blogEntries', BlogEntryDefinition::class, BlogEntryTagMappingDefinition::class, 'tag_id', 'werkl_blog_entry_id'))->addFlags(new CascadeDelete()),
+        );
+    }
+
+    public function getEntityName(): string
+    {
+        return TagDefinition::ENTITY_NAME;
+    }
+}

src/Migration/Migration1789100000NullableTranslationColumns.php

@@ -0,0 +1,38 @@
+<?php
+declare(strict_types=1);
+
+namespace Werkl\OpenBlogware\Migration;
+
+use Doctrine\DBAL\Connection;
+use Shopware\Core\Framework\Migration\MigrationStep;
+
+class Migration1789100000NullableTranslationColumns extends MigrationStep
+{
+    public function getCreationTimestamp(): int
+    {
+        return 1789100000;
+    }
+
+    public function update(Connection $connection): void
+    {
+        /**
+         * Translated columns must be nullable so values can fall back through the language
+         * inheritance, as enforced by `dal:validate` (DefinitionValidator::validateTranslatedColumnsAreNullable).
+         */
+        $connection->executeStatement('
+            ALTER TABLE `werkl_blog_entry_translation`
+            MODIFY COLUMN `title` VARCHAR(255) NULL,
+            MODIFY COLUMN `slug` VARCHAR(255) NULL;
+        ');
+
+        $connection->executeStatement('
+            ALTER TABLE `werkl_blog_category_translation`
+            MODIFY COLUMN `name` VARCHAR(255) NULL;
+        ');
+    }
+
+    public function updateDestructive(Connection $connection): void
+    {
+        // implement update destructive
+    }
+}

src/Resources/config/services.xml

@@ -1,4 +1,4 @@
 <?xml version="1.0" ?>
 
 <container xmlns="http://symfony.com/schema/dic/services"
            xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
@@ -53,6 +53,10 @@
             <tag name="shopware.entity.extension"/>
         </service>
 
+        <service id="Werkl\OpenBlogware\Content\Extension\TagExtension">
+            <tag name="shopware.entity.extension"/>
+        </service>
+
         <service id="Werkl\OpenBlogware\Page\Blog\BlogPageLoader" public="true">
             <argument type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService"/>
             <argument type="service" id="Shopware\Storefront\Page\GenericPageLoader"/>

tests/unit/Content/Blog/BlogEntryDefinitionTest.php

@@ -0,0 +1,171 @@
+<?php
+declare(strict_types=1);
+
+namespace Werkl\OpenBlogware\Tests\Unit\Content\Blog;
+
+use PHPUnit\Framework\TestCase;
+use Shopware\Core\Content\Cms\CmsPageDefinition;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\Flag\CascadeDelete;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\Flag\PrimaryKey;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\Flag\Required;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\ManyToManyAssociationField;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\ManyToOneAssociationField;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\OneToManyAssociationField;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\ReferenceVersionField;
+use Shopware\Core\Framework\DataAbstractionLayer\Field\TranslatedField;
+use Shopware\Core\Framework\DataAbstractionLayer\FieldCollection;
+use Shopware\Core\System\Tag\TagDefinition;
+use Werkl\OpenBlogware\Content\Blog\Aggregate\BlogEntryTagMappingDefinition;
+use Werkl\OpenBlogware\Content\Blog\BlogEntryDefinition;
+use Werkl\OpenBlogware\Content\Blog\BlogEntryEntity;
+use Werkl\OpenBlogware\Content\Extension\CmsPageExtension;
+use Werkl\OpenBlogware\Content\Extension\TagExtension;
+
+/**
+ * Regression tests for the definitions reported as errors by `dal:validate`,
+ * see https://github.com/Werkstattl/OpenBlogware/issues/87
+ */
+class BlogEntryDefinitionTest extends TestCase
+{
+    /**
+     * @var array<string, \Shopware\Core\Framework\DataAbstractionLayer\Field\Field>
+     */
+    private array $fields;
+
+    protected function setUp(): void
+    {
+        $this->fields = $this->defineFieldsByName(new BlogEntryDefinition());
+    }
+
+    /**
+     * The table has PRIMARY KEY (id) only, so no other field may carry the PrimaryKey flag.
+     */
+    public function testOnlyIdFieldIsFlaggedAsPrimaryKey(): void
+    {
+        $propertyNames = [];
+
+        foreach ($this->fields as $propertyName => $field) {
+            if ($field->is(PrimaryKey::class)) {
+                $propertyNames[] = $propertyName;
+            }
+        }
+
+        static::assertSame(['id'], $propertyNames);
+    }
+
+    public function testCmsPageVersionReferenceFieldIsNotPrimaryKeyButRequired(): void
+    {
+        $field = $this->fields['cmsPageVersionId'];
+
+        static::assertInstanceOf(ReferenceVersionField::class, $field);
+        static::assertTrue($field->is(Required::class));
+        static::assertFalse($field->is(PrimaryKey::class));
+    }
+
+    public function testCmsPageAssociationIsManyToOne(): void
+    {
+        $association = $this->fields['cmsPage'];
+
+        static::assertInstanceOf(ManyToOneAssociationField::class, $association);
+        static::assertSame(CmsPageDefinition::class, $association->getReferenceClass());
+    }
+
+    /**
+     * The CmsPageExtension provides the reverse one-to-many side the DAL validator requires.
+     */
+    public function testCmsPageExtensionProvidesReverseAssociation(): void
+    {
+        $extensionFields = new FieldCollection();
+        (new CmsPageExtension())->extendFields($extensionFields);
+
+        $reverse = $this->fieldsByName($extensionFields)['blogEntries'] ?? null;
+
+        static::assertNotNull($reverse);
+        static::assertInstanceOf(OneToManyAssociationField::class, $reverse);
+        static::assertSame(BlogEntryDefinition::class, $reverse->getReferenceClass());
+    }
+
+    public function testTagsAssociationUsesTagMappingDefinition(): void
+    {
+        $association = $this->fields['tags'];
+
+        static::assertInstanceOf(ManyToManyAssociationField::class, $association);
+        // for many-to-many fields the reference class is the mapping definition
+        static::assertSame(BlogEntryTagMappingDefinition::class, $association->getReferenceClass());
+        static::assertSame(TagDefinition::class, $this->getToManyReferenceClass($association));
+    }
+
+    /**
+     * The TagExtension provides the reverse many-to-many side the DAL validator requires.
+     */
+    public function testTagExtensionProvidesReverseAssociation(): void
+    {
+        $extensionFields = new FieldCollection();
+        (new TagExtension())->extendFields($extensionFields);
+
+        $reverse = $this->fieldsByName($extensionFields)['blogEntries'] ?? null;
+
+        static::assertNotNull($reverse);
+        static::assertInstanceOf(ManyToManyAssociationField::class, $reverse);
+        static::assertSame(BlogEntryDefinition::class, $this->getToManyReferenceClass($reverse));
+        static::assertSame(BlogEntryTagMappingDefinition::class, $reverse->getReferenceClass());
+        static::assertTrue($reverse->is(CascadeDelete::class));
+    }
+
+    /**
+     * The teaser image lives in the translations; the media association on the entity is
+     * hydrated manually by the BlogSubscriber and must therefore stay out of the DAL
+     * field collection, but be documented as internal for the definition validator.
+     */
+    public function testMediaIsNotADalFieldButAnInternalHydratedAssociation(): void
+    {
+        static::assertArrayNotHasKey('media', $this->fields);
+        static::assertInstanceOf(TranslatedField::class, $this->fields['mediaId']);
+
+        $docComment = (new \ReflectionProperty(BlogEntryEntity::class, 'media'))->getDocComment();
+
+        static::assertIsString($docComment);
+        static::assertStringContainsString('@internal', $docComment);
+    }
+
+    public function testSetTagsDoesNotAcceptNull(): void
+    {
+        $parameter = (new \ReflectionMethod(BlogEntryEntity::class, 'setTags'))->getParameters()[0];
+
+        static::assertFalse($parameter->allowsNull());
+    }
+
+    /**
+     * @return array<string, \Shopware\Core\Framework\DataAbstractionLayer\Field\Field>
+     */
+    private function defineFieldsByName(BlogEntryDefinition $definition): array
+    {
+        $method = new \ReflectionMethod(BlogEntryDefinition::class, 'defineFields');
+
+        return $this->fieldsByName($method->invoke($definition));
+    }
+
+    /**
+     * @return array<string, \Shopware\Core\Framework\DataAbstractionLayer\Field\Field>
+     */
+    private function fieldsByName(FieldCollection $collection): array
+    {
+        $fields = [];
+
+        foreach ($collection as $field) {
+            $fields[$field->getPropertyName()] = $field;
+        }
+
+        return $fields;
+    }
+
+    /**
+     * Reads the (uncompiled) to-many reference class of a many-to-many association.
+     */
+    private function getToManyReferenceClass(object $association): string
+    {
+        $property = new \ReflectionProperty(ManyToManyAssociationField::class, 'toManyDefinitionClass');
+
+        return $property->getValue($association);
+    }
+}