Merge pull request #4548 from neos/bugfix/4455-fix-legaciynodemigrati…

…on-behat-tests BUGFIX: Fix Neos.ContentRepository.LegacyNodeMigration behat tests
neos · Sep 26, 2023 · 033c07a · 033c07a
2 parents fee1867 + eeffbe0
commit 033c07a
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 23 deletions.
diff --git a/.composer.json b/.composer.json
@@ -5,7 +5,8 @@
   "type": "neos-package-collection",
   "require": {
     "neos/flow-development-collection": "9.0.x-dev",
-    "neos/neos-setup": "^2.0"
+    "neos/neos-setup": "^2.0",
+    "league/flysystem-memory": "^3"
   },
   "replace": {
   },
@@ -40,7 +41,8 @@
       "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentRepository.BehavioralTests/Tests/Behavior/behat.yml.dist",
       "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentGraph.DoctrineDbalAdapter/Tests/Behavior/behat.yml.dist",
       "../../flow doctrine:migrate --quiet; ../../flow cr:setup",
-      "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.Neos/Tests/Behavior/behat.yml.dist --tags ~@browser"
+      "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.Neos/Tests/Behavior/behat.yml.dist --tags ~@browser",
+      "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/behat.yml.dist"
     ],
     "test": [
       "@test:unit",

diff --git a/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php b/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php
@@ -5,6 +5,7 @@
 namespace Neos\ContentRepository\LegacyNodeMigration;
 
 use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
+use Doctrine\DBAL\Types\ConversionException;
 use League\Flysystem\Filesystem;
 use League\Flysystem\FilesystemException;
 use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet;
@@ -272,7 +273,11 @@ public function extractPropertyValuesAndReferences(array $nodeDataRow, NodeType
         $references = [];
 
         // Note: We use a PostgreSQL platform because the implementation is forward-compatible, @see JsonArrayType::convertToPHPValue()
-        $decodedProperties = (new JsonArrayType())->convertToPHPValue($nodeDataRow['properties'], new PostgreSQL100Platform());
+        try {
+            $decodedProperties = (new JsonArrayType())->convertToPHPValue($nodeDataRow['properties'], new PostgreSQL100Platform());
+        } catch (ConversionException $exception) {
+            throw new MigrationException(sprintf('Failed to decode properties %s of node "%s" (type: "%s"): %s', json_encode($nodeDataRow['properties']), $nodeDataRow['identifier'], $nodeType->name->value, $exception->getMessage()), 1695391558, $exception);
+        }
         if (!is_array($decodedProperties)) {
             throw new MigrationException(sprintf('Failed to decode properties %s of node "%s" (type: "%s")', json_encode($nodeDataRow['properties']), $nodeDataRow['identifier'], $nodeType->name->value), 1656057035);
         }

diff --git a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php
@@ -10,9 +10,16 @@
 use League\Flysystem\Filesystem;
 use League\Flysystem\InMemory\InMemoryFilesystemAdapter;
 use Neos\Behat\Tests\Behat\FlowContextTrait;
+use Neos\ContentRepository\BehavioralTests\TestSuite\Behavior\CRBehavioralTestsSubjectProvider;
+use Neos\ContentRepository\BehavioralTests\TestSuite\Behavior\GherkinPyStringNodeBasedNodeTypeManagerFactory;
+use Neos\ContentRepository\BehavioralTests\TestSuite\Behavior\GherkinTableNodeBasedContentDimensionSourceFactory;
 use Neos\ContentRepository\Core\ContentRepository;
 use Neos\ContentRepository\Core\EventStore\EventNormalizer;
-use Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\CRTestSuiteTrait;
+use Neos\ContentRepository\Core\Factory\ContentRepositoryId;
+use Neos\ContentRepository\Core\Factory\ContentRepositoryServiceFactoryInterface;
+use Neos\ContentRepository\Core\Factory\ContentRepositoryServiceInterface;
+use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
+use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
 use Neos\ContentRepository\Export\Asset\AssetExporter;
 use Neos\ContentRepository\Export\Asset\AssetLoaderInterface;
 use Neos\ContentRepository\Export\Asset\ResourceLoaderInterface;
@@ -24,9 +31,8 @@
 use Neos\ContentRepository\Export\Severity;
 use Neos\ContentRepository\LegacyNodeMigration\NodeDataToAssetsProcessor;
 use Neos\ContentRepository\LegacyNodeMigration\NodeDataToEventsProcessor;
-use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
-use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
-use Neos\ContentRepositoryRegistry\TestSuite\Behavior\CRRegistrySubjectProvider;
+use Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\CRTestSuiteTrait;
+use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry;
 use Neos\Flow\Property\PropertyMapper;
 use Neos\Flow\ResourceManagement\PersistentResource;
 use PHPUnit\Framework\Assert;
@@ -39,7 +45,7 @@ class FeatureContext implements Context
 {
     use FlowContextTrait;
     use CRTestSuiteTrait;
-    use CRRegistrySubjectProvider;
+    use CRBehavioralTestsSubjectProvider;
 
     protected $isolated = false;
 
@@ -53,18 +59,26 @@ class FeatureContext implements Context
 
     private ProcessorResult|null $lastMigrationResult = null;
 
+    /**
+     * @var array<string>
+     */
+    private array $loggedErrors = [];
+
     private ContentRepository $contentRepository;
 
+    protected ContentRepositoryRegistry $contentRepositoryRegistry;
+
     public function __construct()
     {
         if (self::$bootstrap === null) {
             self::$bootstrap = $this->initializeFlow();
         }
         $this->objectManager = self::$bootstrap->getObjectManager();
+        $this->contentRepositoryRegistry = $this->objectManager->get(ContentRepositoryRegistry::class);
+
         $this->mockFilesystemAdapter = new InMemoryFilesystemAdapter();
         $this->mockFilesystem = new Filesystem($this->mockFilesystemAdapter);
         $this->setupCRTestSuiteTrait();
-
     }
 
     /**
@@ -73,7 +87,10 @@ public function __construct()
     public function failIfLastMigrationHasErrors(): void
     {
         if ($this->lastMigrationResult !== null && $this->lastMigrationResult->severity === Severity::ERROR) {
-            Assert::fail(sprintf('The last migration run led to an error: %s', $this->lastMigrationResult->message));
+            throw new \RuntimeException(sprintf('The last migration run led to an error: %s', $this->lastMigrationResult->message));
+        }
+        if ($this->loggedErrors !== []) {
+            throw new \RuntimeException(sprintf('The last migration run logged %d error%s', count($this->loggedErrors), count($this->loggedErrors) === 1 ? '' : 's'));
         }
     }
 
@@ -126,6 +143,11 @@ public function iRunTheEventMigration(string $contentStream = null): void
         if ($contentStream !== null) {
             $migration->setContentStreamId(ContentStreamId::fromString($contentStream));
         }
+        $migration->onMessage(function (Severity $severity, string $message) {
+            if ($severity === Severity::ERROR) {
+                $this->loggedErrors[] = $message;
+            }
+        });
         $this->lastMigrationResult = $migration->run();
     }
 
@@ -166,6 +188,15 @@ public function iExpectTheFollowingEventsToBeExported(TableNode $table): void
         Assert::assertCount(count($table->getHash()), $exportedEvents, 'Expected number of events does not match actual number');
     }
 
+    /**
+     * @Then I expect the following errors to be logged
+     */
+    public function iExpectTheFollwingErrorsToBeLogged(TableNode $table): void
+    {
+        Assert::assertSame($this->loggedErrors, $table->getColumn(0), 'Expected logged errors do not match');
+        $this->loggedErrors = [];
+    }
+
     /**
      * @Then I expect a MigrationError
      * @Then I expect a MigrationError with the message
@@ -278,6 +309,11 @@ public function findAssetById(string $assetId): SerializedAsset|SerializedImageV
         $this->mockFilesystemAdapter->deleteEverything();
         $assetExporter = new AssetExporter($this->mockFilesystem, $mockAssetLoader, $mockResourceLoader);
         $migration = new NodeDataToAssetsProcessor($nodeTypeManager, $assetExporter, $this->nodeDataRows);
+        $migration->onMessage(function (Severity $severity, string $message) {
+            if ($severity === Severity::ERROR) {
+                $this->loggedErrors[] = $message;
+            }
+        });
         $this->lastMigrationResult = $migration->run();
     }
 
@@ -345,4 +381,24 @@ private function parseJsonTable(TableNode $table): array
             }, $row);
         }, $table->getHash());
     }
+
+    protected function getContentRepositoryService(
+        ContentRepositoryServiceFactoryInterface $factory
+    ): ContentRepositoryServiceInterface {
+        return $this->contentRepositoryRegistry->buildService(
+            $this->currentContentRepository->id,
+            $factory
+        );
+    }
+
+    protected function createContentRepository(
+        ContentRepositoryId $contentRepositoryId
+    ): ContentRepository {
+        $this->contentRepositoryRegistry->resetFactoryInstance($contentRepositoryId);
+        $contentRepository = $this->contentRepositoryRegistry->get($contentRepositoryId);
+        GherkinTableNodeBasedContentDimensionSourceFactory::reset();
+        GherkinPyStringNodeBasedNodeTypeManagerFactory::reset();
+
+        return $contentRepository;
+    }
 }
diff --git a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature
@@ -183,3 +183,5 @@ Feature: Export of used Assets, Image Variants and Persistent Resources
     Then I expect no Assets to be exported
     And I expect no ImageVariants to be exported
     And I expect no PersistentResources to be exported
+    And I expect the following errors to be logged
+      | Failed to extract assets of property "string" of node "site-node-id" (type: "Some.Package:SomeNodeType"): Failed to find mock asset with id "non-existing-asset" |
diff --git a/...ests/Behavior/Features/Exceptions.feature → ...on/Tests/Behavior/Features/Errors.feature b/...ests/Behavior/Features/Exceptions.feature → ...on/Tests/Behavior/Features/Errors.feature
@@ -31,19 +31,23 @@ Feature: Exceptional cases during migrations
     Node aggregate with id "site-node-id" has a type of "Some.Package:SomeOtherNodeType" in content dimension [{"language":"en"}]. I was visited previously for content dimension [{"language":"de"}] with the type "Some.Package:SomeNodeType". Node variants must not have different types
     """
 
+    # Note: The behavior was changed with https://github.com/neos/neos-development-collection/pull/4201
   Scenario: Node with missing parent
     When I have the following node data rows:
       | Identifier | Path       |
       | sites      | /sites     |
       | a          | /sites/a   |
       | c          | /sites/b/c |
     And I run the event migration
-    Then I expect a MigrationError with the message
-    """
-    Failed to find parent node for node with id "c" and dimensions: []. Did you properly configure your dimensions setup to be in sync with the old setup?
-    """
+    Then I expect the following events to be exported
+      | Type                                | Payload                                                                                                |
+      | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites", "nodeTypeName": "Neos.Neos:Sites", "nodeAggregateClassification": "root"} |
+      | NodeAggregateWithNodeWasCreated     | {"nodeAggregateId": "a", "parentNodeAggregateId": "sites"}                                             |
+    And I expect the following errors to be logged
+      | Failed to find parent node for node with id "c" and dimensions: []. Please ensure that the new content repository has a valid content dimension configuration. Also note that the old CR can sometimes have orphaned nodes. |
 
-  # TODO: is it possible that nodes are processed in an order where a ancestor node is processed after a child node? -> in that case the following example should work (i.e. the scenario should fail)
+
+    # Note: The behavior was changed with https://github.com/neos/neos-development-collection/pull/4201
   Scenario: Nodes out of order
     When I have the following node data rows:
       | Identifier | Path       |
@@ -52,10 +56,14 @@ Feature: Exceptional cases during migrations
       | c          | /sites/b/c |
       | b          | /sites/b   |
     And I run the event migration
-    Then I expect a MigrationError with the message
-    """
-    Failed to find parent node for node with id "c" and dimensions: []. Did you properly configure your dimensions setup to be in sync with the old setup?
-    """
+    Then I expect the following events to be exported
+      | Type                                | Payload                                                                                                |
+      | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites", "nodeTypeName": "Neos.Neos:Sites", "nodeAggregateClassification": "root"} |
+      | NodeAggregateWithNodeWasCreated     | {"nodeAggregateId": "a", "parentNodeAggregateId": "sites"}                                             |
+      | NodeAggregateWithNodeWasCreated     | {"nodeAggregateId": "b", "parentNodeAggregateId": "sites"}                                             |
+    And I expect the following errors to be logged
+      | Failed to find parent node for node with id "c" and dimensions: []. Please ensure that the new content repository has a valid content dimension configuration. Also note that the old CR can sometimes have orphaned nodes. |
+
 
   Scenario: Invalid dimension configuration (unknown value)
     Given I change the content dimensions in content repository "default" to:
@@ -66,7 +74,11 @@ Feature: Exceptional cases during migrations
       | sites      | /sites   |                           |
       | a          | /sites/a | {"language": ["unknown"]} |
     And I run the event migration
-    Then I expect a MigrationError
+    Then I expect the following events to be exported
+      | Type                                | Payload                                                                                                |
+      | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites", "nodeTypeName": "Neos.Neos:Sites", "nodeAggregateClassification": "root"} |
+    And I expect the following errors to be logged
+      | Failed to find parent node for node with id "a" and dimensions: {"language":"unknown"}. Please ensure that the new content repository has a valid content dimension configuration. Also note that the old CR can sometimes have orphaned nodes. |
 
   Scenario: Invalid dimension configuration (no json)
     When I have the following node data rows:
@@ -84,7 +96,7 @@ Feature: Exceptional cases during migrations
     And I run the event migration
     Then I expect a MigrationError with the message
     """
-    Failed to decode properties "not json" of node "a" (type: "Some.Package:SomeNodeType")
+    Failed to decode properties "not json" of node "a" (type: "Some.Package:SomeNodeType"): Could not convert database value "not json" to Doctrine Type flow_json_array
     """
 
   Scenario: Node variants with the same dimension

diff --git a/composer.json b/composer.json
@@ -39,7 +39,8 @@
         "neos/fusion-form": "^1.0 || ^2.0",
         "behat/transliterator": "~1.0",
         "neos/form": "*",
-        "neos/kickstarter": "~9.0.0"
+        "neos/kickstarter": "~9.0.0",
+        "league/flysystem-memory": "^3"
     },
     "replace": {
         "packagefactory/atomicfusion-afx": "*",
@@ -117,7 +118,8 @@
             "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentRepository.BehavioralTests/Tests/Behavior/behat.yml.dist",
             "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentGraph.DoctrineDbalAdapter/Tests/Behavior/behat.yml.dist",
             "../../flow doctrine:migrate --quiet; ../../flow cr:setup",
-            "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.Neos/Tests/Behavior/behat.yml.dist --tags ~@browser"
+            "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.Neos/Tests/Behavior/behat.yml.dist --tags ~@browser",
+            "../../bin/behat -vvv --stop-on-failure -f progress -c Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/behat.yml.dist"
         ],
         "test": [
             "@test:unit",