[BUGFIX] Handle multiple items in mainEntity as array defined in WebP…

…age correctly Also: Deprecate SchemaManager->setMainEntityOfWebPage() in favour of SchemaManager->addMainEntityOfWebPage() Related: #25
brotkrueml · Nov 26, 2019 · 2b2358e · 2b2358e
1 parent a5b2865
commit 2b2358e
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- Handle multiple items in mainEntity as array defined in WebPage correctly (#25)
+
+### Deprecated
+- SchemaManager->setMainEntityOfWebPage() in favour of SchemaManager->addMainEntityOfWebPage() (#25)
+
 ## [1.4.0] - 2019-11-23
 
 ### Changed

diff --git a/Classes/Core/ViewHelpers/AbstractTypeViewHelper.php b/Classes/Core/ViewHelpers/AbstractTypeViewHelper.php
@@ -86,7 +86,7 @@ public function render()
 
         if ($this->stack->isEmpty()) {
             if ($this->isMainEntityOfWebPage) {
-                $this->schemaManager->setMainEntityOfWebPage($recent);
+                $this->schemaManager->addMainEntityOfWebPage($recent);
             } else {
                 $this->schemaManager->addType($recent);
             }

diff --git a/Classes/Manager/SchemaManager.php b/Classes/Manager/SchemaManager.php
@@ -87,7 +87,13 @@ private function setWebPage(AbstractType $webPage): void
         $webPage->clearProperty(static::WEBPAGE_PROPERTY_MAIN_ENTITY);
 
         if ($mainEntity instanceof AbstractType) {
-            $this->setMainEntityOfWebPage($mainEntity);
+            $this->addMainEntityOfWebPage($mainEntity);
+        } elseif (\is_array($mainEntity)) {
+            foreach ($mainEntity as $item) {
+                if ($item instanceof AbstractType) {
+                    $this->addMainEntityOfWebPage($item);
+                }
+            }
         }
 
         $this->webPage = $webPage;
@@ -114,12 +120,30 @@ public function hasWebPage(): bool
     }
 
     /**
-     * Set the main entity of the WebPage
+     * Add a main entity of the WebPage
      *
      * @param AbstractType $mainEntity
      * @return SchemaManager
+     *
+     * @deprecated since version 1.4.1, will be removed in 2.0.0
      */
     public function setMainEntityOfWebPage(AbstractType $mainEntity): self
+    {
+        @\trigger_error(
+            'Using SchemaManager->setMainEntityOfWebPage() is deprecated and will be removed in version 2.0. Please use SchemaManager->addMainEntityOfWebPage() instead.',
+            \E_USER_DEPRECATED
+        );
+
+        return $this->addMainEntityOfWebPage($mainEntity);
+    }
+
+    /**
+     * Add a main entity of the WebPage
+     *
+     * @param AbstractType $mainEntity
+     * @return SchemaManager
+     */
+    public function addMainEntityOfWebPage(AbstractType $mainEntity): self
     {
         $this->mainEntityOfWebPage[] = $mainEntity;
 
@@ -166,7 +190,7 @@ public function renderJsonLd(): string
 
         return \sprintf(
             static::TAG_TEMPLATE,
-            \json_encode($result, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)
+            \json_encode($result, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE)
         );
     }
 

diff --git a/Documentation/Developer/MainEntity.rst b/Documentation/Developer/MainEntity.rst
@@ -51,7 +51,7 @@ example that specifies a product as the primary content:
       ])
    ;
 
-   $schemaManager->setMainEntityOfWebPage($product);
+   $schemaManager->addMainEntityOfWebPage($product);
 
 The above example is rendered as JSON-LD. Let's assume the ``WebPage`` type is
 set to ``ItemPage`` - either in the page properties or via the API or a view

diff --git a/Documentation/Settings.cfg b/Documentation/Settings.cfg
@@ -2,7 +2,7 @@
 
 [general]
 project = schema
-release = 1.4.0
+release = 1.4.1-dev
 copyright = 2019 by Chris Müller
 
 [html_theme_options]

diff --git a/Tests/Unit/Manager/SchemaManagerTest.php b/Tests/Unit/Manager/SchemaManagerTest.php
@@ -8,6 +8,7 @@
 use Brotkrueml\Schema\Model\Type\CollectionPage;
 use Brotkrueml\Schema\Model\Type\Thing;
 use Brotkrueml\Schema\Model\Type\WebPage;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 class SchemaManagerTest extends Testcase
@@ -223,23 +224,23 @@ public function breadcrumbsFromWebPageWhichAreNotOfTypeBreadcrumbListAreIgnored(
     /**
      * @test
      */
-    public function setMainEntityOfWebPageReturnsInstanceOfSchemaManager(): void
+    public function addMainEntityOfWebPageReturnsInstanceOfSchemaManager(): void
     {
-        $actual = $this->subject->setMainEntityOfWebPage(new Thing());
+        $actual = $this->subject->addMainEntityOfWebPage(new Thing());
 
         self::assertInstanceOf(SchemaManager::class, $actual);
     }
 
     /**
      * @test
      */
-    public function setMainEntityOfWebPageWithWebPageAvailable(): void
+    public function addMainEntityOfWebPageWithWebPageAvailable(): void
     {
         $webPage = new WebPage();
         $this->subject->addType($webPage);
 
         $mainEntity = (new Thing())->setProperty('name', 'A thing, set as main entity');
-        $this->subject->setMainEntityOfWebPage($mainEntity);
+        $this->subject->addMainEntityOfWebPage($mainEntity);
 
         $actual = $this->subject->renderJsonLd();
 
@@ -249,10 +250,10 @@ public function setMainEntityOfWebPageWithWebPageAvailable(): void
     /**
      * @test
      */
-    public function setMainEntityOfWebPageWithoutWebPageAvailable(): void
+    public function addMainEntityOfWebPageWithoutWebPageAvailable(): void
     {
         $mainEntity = (new Thing())->setProperty('name', 'A thing, set as main entity');
-        $this->subject->setMainEntityOfWebPage($mainEntity);
+        $this->subject->addMainEntityOfWebPage($mainEntity);
 
         $actual = $this->subject->renderJsonLd();
 
@@ -262,16 +263,16 @@ public function setMainEntityOfWebPageWithoutWebPageAvailable(): void
     /**
      * @test
      */
-    public function setMainEntityOfWebPageTwiceWithWebPageAvailable(): void
+    public function addMainEntityOfWebPageTwiceWithWebPageAvailable(): void
     {
         $webPage = new WebPage();
         $this->subject->addType($webPage);
 
         $mainEntity1 = (new Thing())->setProperty('name', 'A thing, set as main entity #1');
-        $this->subject->setMainEntityOfWebPage($mainEntity1);
+        $this->subject->addMainEntityOfWebPage($mainEntity1);
 
         $mainEntity2 = (new Thing())->setProperty('name', 'A thing, set as main entity #2');
-        $this->subject->setMainEntityOfWebPage($mainEntity2);
+        $this->subject->addMainEntityOfWebPage($mainEntity2);
 
         $actual = $this->subject->renderJsonLd();
 
@@ -281,7 +282,7 @@ public function setMainEntityOfWebPageTwiceWithWebPageAvailable(): void
     /**
      * @test
      */
-    public function setWebPageAndMainEntityOfWebPageAfterThatPreservesFirstType(): void
+    public function addWebPageAndMainEntityOfWebPageAfterThatPreservesFirstType(): void
     {
         $webPage = (new WebPage())
             ->setProperty(
@@ -292,10 +293,84 @@ public function setWebPageAndMainEntityOfWebPageAfterThatPreservesFirstType(): v
         $this->subject->addType($webPage);
 
         $newMainEntity = (new Thing())->setProperty('name', 'A thing, set as new main entity');
-        $this->subject->setMainEntityOfWebPage($newMainEntity);
+        $this->subject->addMainEntityOfWebPage($newMainEntity);
 
         $actual = $this->subject->renderJsonLd();
 
         self::assertSame('<script type="application/ld+json">{"@context":"http://schema.org","@type":"WebPage","mainEntity":[{"@type":"Thing","name":"A thing, set as main entity directly in WebPage"},{"@type":"Thing","name":"A thing, set as new main entity"}]}</script>', $actual);
     }
+
+    /**
+     * @test
+     */
+    public function addWebPageAndMultipleMainEntityOfWebPage(): void
+    {
+        $webPage = (new WebPage())
+            ->addProperty(
+                'mainEntity',
+                (new Thing())
+                    ->setProperty('name', 'A thing, set as main entity directly in WebPage #1')
+            )
+            ->addProperty(
+                'mainEntity',
+                (new Thing())
+                    ->setProperty('name', 'A thing, set as main entity directly in WebPage #2')
+            );
+
+        $this->subject->addType($webPage);
+
+        $newMainEntity = (new Thing())->setProperty('name', 'A thing, set as new main entity');
+        $this->subject->addMainEntityOfWebPage($newMainEntity);
+
+        $actual = $this->subject->renderJsonLd();
+
+        self::assertSame('<script type="application/ld+json">{"@context":"http://schema.org","@type":"WebPage","mainEntity":[{"@type":"Thing","name":"A thing, set as main entity directly in WebPage #1"},{"@type":"Thing","name":"A thing, set as main entity directly in WebPage #2"},{"@type":"Thing","name":"A thing, set as new main entity"}]}</script>', $actual);
+    }
+
+    /**
+     * @test
+     */
+    public function addWebPageAndMultipleMainEntityOfWebPageAsArray(): void
+    {
+        $webPage = (new WebPage())
+            ->setProperty(
+                'mainEntity',
+                [
+                    (new Thing())
+                        ->setProperty('name', 'A thing, set as main entity directly in WebPage #1'),
+                    (new Thing())
+                        ->setProperty('name', 'A thing, set as main entity directly in WebPage #2'),
+                ]
+            );
+
+        $this->subject->addType($webPage);
+
+        $newMainEntity = (new Thing())->setProperty('name', 'A thing, set as new main entity');
+        $this->subject->addMainEntityOfWebPage($newMainEntity);
+
+        $actual = $this->subject->renderJsonLd();
+
+        self::assertSame('<script type="application/ld+json">{"@context":"http://schema.org","@type":"WebPage","mainEntity":[{"@type":"Thing","name":"A thing, set as main entity directly in WebPage #1"},{"@type":"Thing","name":"A thing, set as main entity directly in WebPage #2"},{"@type":"Thing","name":"A thing, set as new main entity"}]}</script>', $actual);
+    }
+
+    /**
+     * @test
+     */
+    public function setMainEntityOfWebPageCallsAddMainEntityOfWebPage(): void
+    {
+        /** @var MockObject|SchemaManager $subject */
+        $subject = $this->getMockBuilder(SchemaManager::class)
+            ->onlyMethods(['addMainEntityOfWebPage'])
+            ->getMock();
+
+        $mainEntity = (new Thing())->setProperty('name', 'Some main entity');
+
+        $subject
+            ->expects(self::once())
+            ->method('addMainEntityOfWebPage')
+            ->with($mainEntity)
+            ->willReturn($subject);
+
+        $subject->setMainEntityOfWebPage($mainEntity);
+    }
 }
diff --git a/composer.json b/composer.json
@@ -28,7 +28,7 @@
         "infection/infection": "^0.13",
         "mikey179/vfsstream": "^1.6",
         "phpstan/phpstan": "^0.11",
-        "phpunit/phpunit": "^8.1",
+        "phpunit/phpunit": "^8.4",
         "twig/twig": "^3.0",
         "typo3/testing-framework": "^4.13 || ^5.0"
     },

diff --git a/ext_emconf.php b/ext_emconf.php
@@ -9,7 +9,7 @@
     'clearCacheOnLoad' => true,
     'author' => 'Chris Müller',
     'author_email' => '[email protected]',
-    'version' => '1.4.0',
+    'version' => '1.4.1-dev',
     'constraints' => [
         'depends' => [
             'typo3' => '9.5.0-10.4.99',