diff --git a/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToAssetsProcessor.php b/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToAssetsProcessor.php index f96db3b58cc..808ce0f40f7 100644 --- a/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToAssetsProcessor.php +++ b/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToAssetsProcessor.php @@ -62,7 +62,12 @@ public function run(): ProcessorResult continue; } foreach ($properties as $propertyName => $propertyValue) { - $propertyType = $nodeType->getPropertyType($propertyName); + try { + $propertyType = $nodeType->getPropertyType($propertyName); + } catch (\InvalidArgumentException $exception) { + $this->dispatch(Severity::WARNING, 'Skipped node data processing for the property "%s". The property name is not part of the NodeType schema for the NodeType "%s". (Node: %s)', $propertyName, $nodeType->name->value, $nodeDataRow['identifier']); + continue; + } foreach ($this->extractAssetIdentifiers($propertyType, $propertyValue) as $assetId) { if (array_key_exists($assetId, $this->processedAssetIds)) { continue; diff --git a/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php b/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php index cc3ad13609b..4ea8a74d16f 100644 --- a/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php +++ b/Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php @@ -316,7 +316,12 @@ public function extractPropertyValuesAndReferences(array $nodeDataRow, NodeType } foreach ($decodedProperties as $propertyName => $propertyValue) { - $type = $nodeType->getPropertyType($propertyName); + try { + $type = $nodeType->getPropertyType($propertyName); + } catch (\InvalidArgumentException $exception) { + $this->dispatch(Severity::WARNING, 'Skipped node data processing for the property "%s". The property name is not part of the NodeType schema for the NodeType "%s". (Node: %s)', $propertyName, $nodeType->name->value, $nodeDataRow['identifier']); + continue; + } if ($type === 'reference' || $type === 'references') { if (!empty($propertyValue)) { diff --git a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php index 0054ab25685..ac0db3ac9dc 100644 --- a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php +++ b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Bootstrap/FeatureContext.php @@ -62,6 +62,11 @@ class FeatureContext implements Context */ private array $loggedErrors = []; + /** + * @var array + */ + private array $loggedWarnings = []; + private ContentRepository $contentRepository; protected ContentRepositoryRegistry $contentRepositoryRegistry; @@ -141,6 +146,8 @@ public function iRunTheEventMigration(string $contentStream = null): void $migration->onMessage(function (Severity $severity, string $message) { if ($severity === Severity::ERROR) { $this->loggedErrors[] = $message; + } elseif ($severity === Severity::WARNING) { + $this->loggedWarnings[] = $message; } }); $this->lastMigrationResult = $migration->run(); @@ -186,12 +193,21 @@ public function iExpectTheFollowingEventsToBeExported(TableNode $table): void /** * @Then I expect the following errors to be logged */ - public function iExpectTheFollwingErrorsToBeLogged(TableNode $table): void + public function iExpectTheFollowingErrorsToBeLogged(TableNode $table): void { Assert::assertSame($table->getColumn(0), $this->loggedErrors, 'Expected logged errors do not match'); $this->loggedErrors = []; } + /** + * @Then I expect the following warnings to be logged + */ + public function iExpectTheFollowingWarningsToBeLogged(TableNode $table): void + { + Assert::assertSame($table->getColumn(0), $this->loggedWarnings, 'Expected logged warnings do not match'); + $this->loggedWarnings = []; + } + /** * @Then I expect a MigrationError * @Then I expect a MigrationError with the message @@ -307,6 +323,8 @@ public function findAssetById(string $assetId): SerializedAsset|SerializedImageV $migration->onMessage(function (Severity $severity, string $message) { if ($severity === Severity::ERROR) { $this->loggedErrors[] = $message; + } elseif ($severity === Severity::WARNING) { + $this->loggedWarnings[] = $message; } }); $this->lastMigrationResult = $migration->run(); diff --git a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature index f4ca3e07455..52ac42afe7e 100644 --- a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature +++ b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/Assets.feature @@ -43,9 +43,9 @@ Feature: Export of used Assets, Image Variants and Persistent Resources Scenario: Exporting an Image Variant includes the original Image asset as well When I have the following node data rows: - | Identifier | Path | Node Type | Properties | - | sites-node-id | /sites | unstructured | | - | site-node-id | /sites/test-site | Some.Package:Homepage | {"string": "asset:\/\/variant1"} | + | Identifier | Path | Node Type | Properties | + | sites-node-id | /sites | unstructured | | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"string": "asset:\/\/variant1"} | And I run the asset migration Then I expect the following Assets to be exported: """ @@ -87,12 +87,12 @@ Feature: Export of used Assets, Image Variants and Persistent Resources Scenario: Assets and image variants are only exported once each When I have the following node data rows: - | Identifier | Path | Node Type | Dimension Values | Properties | - | sites-node-id | /sites | unstructured | | | - | site-node-id | /sites/test-site | Some.Package:Homepage | | {"string": "asset:\/\/asset1"} | - | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["ch"]} | {"image": {"__flow_object_type": "Neos\\Media\\Domain\\Model\\Image", "__identifier": "asset2"}} | - | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["en"]} | {"assets": [{"__flow_object_type": "Neos\\Media\\Domain\\Model\\Document", "__identifier": "asset3"}, {"__flow_object_type": "Neos\\Media\\Domain\\Model\\Image", "__identifier": "asset2"}, {"__flow_object_type": "Neos\\Media\\Domain\\Model\\ImageVariant", "__identifier": "variant1"}]} | - | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["de"]} | {"string": "some text with an asset link"} | + | Identifier | Path | Node Type | Dimension Values | Properties | + | sites-node-id | /sites | unstructured | | | + | site-node-id | /sites/test-site | Some.Package:Homepage | | {"string": "asset:\/\/asset1"} | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["ch"]} | {"image": {"__flow_object_type": "Neos\\Media\\Domain\\Model\\Image", "__identifier": "asset2"}} | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["en"]} | {"assets": [{"__flow_object_type": "Neos\\Media\\Domain\\Model\\Document", "__identifier": "asset3"}, {"__flow_object_type": "Neos\\Media\\Domain\\Model\\Image", "__identifier": "asset2"}, {"__flow_object_type": "Neos\\Media\\Domain\\Model\\ImageVariant", "__identifier": "variant1"}]} | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"language": ["de"]} | {"string": "some text with an asset link"} | And I run the asset migration Then I expect the following Assets to be exported: """ @@ -178,12 +178,22 @@ Feature: Export of used Assets, Image Variants and Persistent Resources Scenario: Referring to non-existing asset When I have the following node data rows: - | Identifier | Path | Node Type | Properties | - | sites-node-id | /sites | unstructured | | - | site-node-id | /sites/test-site | Some.Package:Homepage | {"string": "asset:\/\/non-existing-asset"} | + | Identifier | Path | Node Type | Properties | + | sites-node-id | /sites | unstructured | | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"string": "asset:\/\/non-existing-asset"} | And I run the asset migration 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:Homepage"): Failed to find mock asset with id "non-existing-asset" | + + Scenario: Nodes with properties that are not part of the node type schema (see https://github.com/neos/neos-development-collection/issues/4804) + When I have the following node data rows: + | Identifier | Path | Node Type | Properties | + | sites-node-id | /sites | unstructured | | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"unknownProperty": "asset:\/\/variant1"} | + And I run the asset migration + Then I expect no Assets to be exported + And I expect the following warnings to be logged + | Skipped node data processing for the property "unknownProperty". The property name is not part of the NodeType schema for the NodeType "Some.Package:Homepage". (Node: site-node-id) | diff --git a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/References.feature b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/References.feature index 52e659f6017..8cd77523574 100644 --- a/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/References.feature +++ b/Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/Features/References.feature @@ -36,12 +36,12 @@ Feature: Migrations that contain nodes with "reference" or "references propertie | c | /sites/site/c | Some.Package:SomeNodeType | {"text": "This is c", "refs": ["a", "b"]} | And I run the event migration Then I expect the following events to be exported - | Type | Payload | - | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "site"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "a"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "b"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "c"} | + | Type | Payload | + | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "site"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "a"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "b"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "c"} | | NodeReferencesWereSet | {"sourceNodeAggregateId":"a","affectedSourceOriginDimensionSpacePoints":[[]],"referenceName":"ref","references":{"b":{"targetNodeAggregateId":"b","properties":null}}} | | NodeReferencesWereSet | {"sourceNodeAggregateId":"c","affectedSourceOriginDimensionSpacePoints":[[]],"referenceName":"refs","references":{"a":{"targetNodeAggregateId":"a","properties":null},"b":{"targetNodeAggregateId":"b","properties":null}}} | @@ -60,13 +60,26 @@ Feature: Migrations that contain nodes with "reference" or "references propertie | c | /sites/site/c | Some.Package:SomeNodeType | {"language": ["ch"]} | {"text": "This is c", "refs": ["a", "b"]} | And I run the event migration Then I expect the following events to be exported - | Type | Payload | - | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "site"} | - | NodePeerVariantWasCreated | {} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "a"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "b"} | - | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "c"} | + | Type | Payload | + | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "site"} | + | NodePeerVariantWasCreated | {} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "a"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "b"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "c"} | | NodeReferencesWereSet | {"sourceNodeAggregateId":"a","affectedSourceOriginDimensionSpacePoints":[{"language": "en"}],"referenceName":"ref","references":{"b":{"targetNodeAggregateId":"b","properties":null}}} | | NodeReferencesWereSet | {"sourceNodeAggregateId":"b","affectedSourceOriginDimensionSpacePoints":[{"language": "de"}],"referenceName":"ref","references":{"a":{"targetNodeAggregateId":"a","properties":null}}} | | NodeReferencesWereSet | {"sourceNodeAggregateId":"c","affectedSourceOriginDimensionSpacePoints":[{"language": "ch"}],"referenceName":"refs","references":{"a":{"targetNodeAggregateId":"a","properties":null},"b":{"targetNodeAggregateId":"b","properties":null}}} | + + Scenario: Nodes with properties that are not part of the node type schema (see https://github.com/neos/neos-development-collection/issues/4804) + When I have the following node data rows: + | Identifier | Path | Node Type | Properties | + | sites-node-id | /sites | unstructured | | + | site-node-id | /sites/test-site | Some.Package:Homepage | {"unknownProperty": "ref"} | + And I run the event migration + Then I expect the following events to be exported + | Type | Payload | + | RootNodeAggregateWithNodeWasCreated | {"nodeAggregateId": "sites-node-id"} | + | NodeAggregateWithNodeWasCreated | {"nodeAggregateId": "site-node-id"} | + And I expect the following warnings to be logged + | Skipped node data processing for the property "unknownProperty". The property name is not part of the NodeType schema for the NodeType "Some.Package:Homepage". (Node: site-node-id) |