Skip to content

Commit 171acef

Browse files
kitsunetkitsunet
authored
Merge pull request #5517 from mhsdesign/feature/add-option-to-emit-warnings-before-handling-node-migrations
FEATURE: Add option to emit warnings before handling node migrations
2 parents 0e6c9c9 + 8ea006f commit 171acef

27 files changed

+443
-149
lines changed

‎Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/EventSourced/Migration/ChangePropertyValue_Dimensions.feature

+12-28
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ Feature: Change Property Value across dimensions; and test DimensionSpacePoints
203203

204204

205205
Scenario: matching only happens based on originDimensionSpacePoint, not on visibleDimensionSpacePoints - we try to change CH, but should not see any modification (includeSpecializations = FALSE - default)
206-
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:
206+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs" and exceptions are caught:
207207
"""yaml
208208
migration:
209209
-
@@ -225,22 +225,14 @@ Feature: Change Property Value across dimensions; and test DimensionSpacePoints
225225
property: 'text'
226226
newSerializedValue: 'fixed value'
227227
"""
228-
229-
# neither DE or CH is modified
230-
When I am in workspace "migration-workspace" and dimension space point {"language": "de"}
231-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{"language": "de"} to exist in the content graph
232-
And I expect this node to have the following properties:
233-
| Key | Value |
234-
| text | "Original text" |
235-
236-
When I am in workspace "migration-workspace" and dimension space point {"language": "ch"}
237-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{"language": "de"} to exist in the content graph
238-
And I expect this node to have the following properties:
239-
| Key | Value |
240-
| text | "Original text" |
228+
Then the last command should have thrown an exception of type "MigrationException" with message:
229+
"""
230+
Migration did not issue any commands.
231+
"""
232+
And I expect the content stream "migration-cs" to not exist
241233

242234
Scenario: matching only happens based on originDimensionSpacePoint, not on visibleDimensionSpacePoints - we try to change CH, but should not see any modification (includeSpecializations = TRUE)
243-
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:
235+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs" and exceptions are caught:
244236
"""yaml
245237
migration:
246238
-
@@ -263,16 +255,8 @@ Feature: Change Property Value across dimensions; and test DimensionSpacePoints
263255
property: 'text'
264256
newSerializedValue: 'fixed value'
265257
"""
266-
267-
# neither DE or CH is modified
268-
When I am in workspace "migration-workspace" and dimension space point {"language": "de"}
269-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{"language": "de"} to exist in the content graph
270-
And I expect this node to have the following properties:
271-
| Key | Value |
272-
| text | "Original text" |
273-
274-
When I am in workspace "migration-workspace" and dimension space point {"language": "ch"}
275-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{"language": "de"} to exist in the content graph
276-
And I expect this node to have the following properties:
277-
| Key | Value |
278-
| text | "Original text" |
258+
Then the last command should have thrown an exception of type "MigrationException" with message:
259+
"""
260+
Migration did not issue any commands.
261+
"""
262+
And I expect the content stream "migration-cs" to not exist

‎Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/EventSourced/Migration/ChangePropertyValue_NoDimensions.feature

+7-7
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Feature: Change Property
6868
| text | "fixed value" |
6969

7070
Scenario: Ignoring transformation if property does not exist on node
71-
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:
71+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs" and exceptions are caught:
7272
"""yaml
7373
migration:
7474
-
@@ -84,12 +84,12 @@ Feature: Change Property
8484
property: 'notExisting'
8585
newSerializedValue: 'fixed value'
8686
"""
87-
# we did not change anything because notExisting does not exist
88-
When I am in workspace "migration-workspace" and dimension space point {}
89-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{} to exist in the content graph
90-
And I expect this node to have the following properties:
91-
| Key | Value |
92-
| text | "Original text" |
87+
88+
Then the last command should have thrown an exception of type "MigrationException" with message:
89+
"""
90+
Migration did not issue any commands.
91+
"""
92+
And I expect the content stream "migration-cs" to not exist
9393

9494
Scenario: replacement using default currentValuePlaceholder
9595
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:

‎Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/EventSourced/Migration/RemoveProperty_NoDimensions.feature

+7-7
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ Feature: Remove Property
6565
And I expect this node to have no properties
6666

6767
Scenario: Ignoring transformation if property does not exist on node
68-
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:
68+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs" and exceptions are caught:
6969
"""yaml
7070
migration:
7171
-
@@ -80,9 +80,9 @@ Feature: Remove Property
8080
settings:
8181
property: 'notExisting'
8282
"""
83-
# we did not change anything because notExisting does not exist
84-
When I am in workspace "migration-workspace" and dimension space point {}
85-
Then I expect a node identified by migration-cs;sir-david-nodenborough;{} to exist in the content graph
86-
And I expect this node to have the following properties:
87-
| Key | Value |
88-
| text | "Original text" |
83+
84+
Then the last command should have thrown an exception of type "MigrationException" with message:
85+
"""
86+
Migration did not issue any commands.
87+
"""
88+
And I expect the content stream "migration-cs" to not exist

‎Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/EventSourced/Migration/UpdateRootNodeAggregateDimensions.feature

+17-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,23 @@ Feature: Update root node aggregate dimensions
110110
| Identifier | Values | Generalizations |
111111
| language | mul, de, ch | ch->de->mul |
112112

113-
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", without publishing on success:
113+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs" and exceptions are caught:
114+
"""yaml
115+
migration:
116+
-
117+
transformations:
118+
-
119+
type: 'UpdateRootNodeAggregateDimensions'
120+
settings:
121+
nodeType: 'Neos.ContentRepository:Root'
122+
"""
123+
124+
Then the last command should have thrown an exception of type "NodeMigrationRequireConfirmationException" with message:
125+
"""
126+
1 warnings: commands UpdateRootNodeAggregateDimensions require confirmation: Updating the dimensions of root node lady-eleonode-rootford will remove all its descendants in dimensions [{"language":"en"}]
127+
"""
128+
129+
When I run the following node migration for workspace "live", creating target workspace "migration-workspace" on contentStreamId "migration-cs", with force and publishing on success:
114130
"""yaml
115131
migration:
116132
-

‎Neos.ContentRepository.Core/Classes/CommandHandler/Commands.php

+5
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ public function getIterator(): \Traversable
6060
yield from $this->items;
6161
}
6262

63+
public function isEmpty(): bool
64+
{
65+
return $this->items === [];
66+
}
67+
6368
public function count(): int
6469
{
6570
return count($this->items);

‎Neos.ContentRepository.NodeMigration/src/Command/ExecuteMigration.php

+59-7
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,66 @@
2020
/**
2121
* Execute a Content Repository migration (which is defined in a YAML file)
2222
*/
23-
final class ExecuteMigration
23+
final readonly class ExecuteMigration
2424
{
25-
public function __construct(
26-
public readonly MigrationConfiguration $migrationConfiguration,
27-
public readonly WorkspaceName $sourceWorkspaceName,
28-
public readonly WorkspaceName $targetWorkspaceName,
29-
public readonly bool $publishOnSuccess,
30-
public readonly ContentStreamId $contentStreamId,
25+
private function __construct(
26+
public MigrationConfiguration $migrationConfiguration,
27+
public WorkspaceName $sourceWorkspaceName,
28+
public WorkspaceName $targetWorkspaceName,
29+
public ContentStreamId $contentStreamId,
30+
public bool $publishOnSuccess,
31+
public bool $requireConfirmation,
3132
) {
3233
}
34+
35+
public static function create(
36+
MigrationConfiguration $migrationConfiguration,
37+
WorkspaceName $sourceWorkspaceName,
38+
WorkspaceName $targetWorkspaceName
39+
): self {
40+
return new self(
41+
$migrationConfiguration,
42+
$sourceWorkspaceName,
43+
$targetWorkspaceName,
44+
ContentStreamId::create(),
45+
publishOnSuccess: true,
46+
requireConfirmation: true
47+
);
48+
}
49+
50+
public function withoutPublishOnSuccess(): self
51+
{
52+
return new self(
53+
$this->migrationConfiguration,
54+
$this->sourceWorkspaceName,
55+
$this->targetWorkspaceName,
56+
$this->contentStreamId,
57+
false,
58+
$this->requireConfirmation,
59+
);
60+
}
61+
62+
public function withoutRequiringConfirmation(): self
63+
{
64+
return new self(
65+
$this->migrationConfiguration,
66+
$this->sourceWorkspaceName,
67+
$this->targetWorkspaceName,
68+
$this->contentStreamId,
69+
$this->publishOnSuccess,
70+
false,
71+
);
72+
}
73+
74+
public function withContentStreamId(ContentStreamId $contentStreamId): self
75+
{
76+
return new self(
77+
$this->migrationConfiguration,
78+
$this->sourceWorkspaceName,
79+
$this->targetWorkspaceName,
80+
$contentStreamId,
81+
$this->publishOnSuccess,
82+
$this->requireConfirmation,
83+
);
84+
}
3385
}

‎Neos.ContentRepository.NodeMigration/src/NodeMigrationRequireConfirmationException.php

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Neos\ContentRepository\NodeMigration;
6+
7+
use Neos\ContentRepository\Core\CommandHandler\CommandInterface;
8+
use Neos\ContentRepository\NodeMigration\Transformation\TransformationSteps;
9+
10+
final class NodeMigrationRequireConfirmationException extends \DomainException
11+
{
12+
public static function becauseStepsRequireConfirmation(TransformationSteps $transformationSteps): self
13+
{
14+
$additionalInformation = [];
15+
foreach ($transformationSteps as $transformationStep) {
16+
$additionalInformation[] = sprintf(
17+
'commands %s require confirmation: %s',
18+
join(',', array_map(fn (CommandInterface $command) => substr(strrchr($command::class, '\\') ?: '', 1), iterator_to_array($transformationStep->commands))),
19+
$transformationStep->confirmationReason,
20+
);
21+
}
22+
return new self(
23+
sprintf('%d warnings: %s', count($transformationSteps), join(";\n", $additionalInformation)),
24+
1742060622
25+
);
26+
}
27+
}

‎Neos.ContentRepository.NodeMigration/src/NodeMigrationService.php

+46-23
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Neos\ContentRepository\NodeMigration\Filter\FiltersFactory;
1717
use Neos\ContentRepository\NodeMigration\Filter\InvalidMigrationFilterSpecified;
1818
use Neos\ContentRepository\NodeMigration\Transformation\TransformationsFactory;
19+
use Neos\ContentRepository\NodeMigration\Transformation\TransformationSteps;
1920

2021
/**
2122
* Node Migrations are manually written adjustments to the Node tree;
@@ -65,34 +66,47 @@ public function executeMigration(ExecuteMigration $command): void
6566
), 1611688225);
6667
}
6768

68-
$targetWorkspaceWasCreated = false;
69-
if ($targetWorkspace = $this->contentRepository->findWorkspaceByName($command->targetWorkspaceName)) {
70-
if ($targetWorkspace->hasPublishableChanges()) {
71-
throw new MigrationException(sprintf('Target workspace "%s" already exists an is not empty. Please clear the workspace before.', $targetWorkspace->workspaceName->value));
69+
$targetWorkspace = $this->contentRepository->findWorkspaceByName($command->targetWorkspaceName);
70+
if ($targetWorkspace?->hasPublishableChanges()) {
71+
throw new MigrationException(sprintf('Target workspace "%s" already exists an is not empty. Please clear the workspace before.', $targetWorkspace->workspaceName->value));
72+
}
73+
74+
$transformationSteps = TransformationSteps::createEmpty();
75+
foreach ($command->migrationConfiguration->getMigration() as $migrationDescription) {
76+
$transformationSteps = $transformationSteps->merge($this->executeSubMigration(
77+
$migrationDescription,
78+
$command->sourceWorkspaceName,
79+
$command->targetWorkspaceName
80+
));
81+
}
82+
83+
if ($command->requireConfirmation) {
84+
$stepsThatRequireConfirmation = $transformationSteps->filterConfirmationRequired();
85+
if (!$stepsThatRequireConfirmation->isEmpty()) {
86+
throw NodeMigrationRequireConfirmationException::becauseStepsRequireConfirmation($stepsThatRequireConfirmation);
7287
}
88+
}
7389

74-
} else {
90+
if ($transformationSteps->isEmpty()) {
91+
throw new MigrationException('Migration did not issue any commands.', 1742117823);
92+
}
93+
94+
$targetWorkspaceWasCreated = false;
95+
if ($targetWorkspace === null) {
7596
$this->contentRepository->handle(
7697
CreateWorkspace::create(
7798
$command->targetWorkspaceName,
7899
$sourceWorkspace->workspaceName,
79100
$command->contentStreamId,
80101
)
81102
);
82-
$targetWorkspace = $this->contentRepository->findWorkspaceByName($command->targetWorkspaceName);
83103
$targetWorkspaceWasCreated = true;
84104
}
85105

86-
if ($targetWorkspace === null) {
87-
throw new MigrationException(sprintf('Target workspace "%s" could not loaded nor created.', $command->targetWorkspaceName->value));
88-
}
89-
90-
foreach ($command->migrationConfiguration->getMigration() as $migrationDescription) {
91-
$this->executeSubMigration(
92-
$migrationDescription,
93-
$command->sourceWorkspaceName,
94-
$command->targetWorkspaceName
95-
);
106+
foreach ($transformationSteps as $transformationStep) {
107+
foreach ($transformationStep->commands as $transformationCommand) {
108+
$this->contentRepository->handle($transformationCommand);
109+
}
96110
}
97111

98112
if ($command->publishOnSuccess === true) {
@@ -118,7 +132,7 @@ protected function executeSubMigration(
118132
array $migrationDescription,
119133
WorkspaceName $workspaceNameForReading,
120134
WorkspaceName $workspaceNameForWriting
121-
): void {
135+
): TransformationSteps {
122136
$filters = $this->filterFactory->buildFilterConjunction($migrationDescription['filters'] ?? []);
123137
$transformations = $this->transformationFactory->buildTransformation(
124138
$migrationDescription['transformations'] ?? []
@@ -145,8 +159,11 @@ protected function executeSubMigration(
145159
);
146160
}
147161

162+
$transformationSteps = TransformationSteps::createEmpty();
148163
if ($transformations->containsGlobal()) {
149-
$transformations->executeGlobal($workspaceNameForWriting);
164+
$transformationSteps = $transformationSteps->merge(
165+
$transformations->executeGlobal($workspaceNameForReading, $workspaceNameForWriting)
166+
);
150167
} elseif ($transformations->containsNodeAggregateBased()) {
151168
$contentGraph = $this->contentRepository->getContentGraph($workspaceNameForReading);
152169
foreach ($contentGraph->findUsedNodeTypeNames() as $nodeTypeName) {
@@ -156,7 +173,9 @@ protected function executeSubMigration(
156173
) as $nodeAggregate
157174
) {
158175
if ($filters->matchesNodeAggregate($nodeAggregate)) {
159-
$transformations->executeNodeAggregateBased($nodeAggregate, $workspaceNameForWriting);
176+
$transformationSteps = $transformationSteps->merge(
177+
$transformations->executeNodeAggregateBased($nodeAggregate, $workspaceNameForWriting)
178+
);
160179
}
161180
}
162181
}
@@ -182,16 +201,20 @@ protected function executeSubMigration(
182201
);
183202

184203
if ($filters->matchesNode($node)) {
185-
$transformations->executeNodeBased(
186-
$node,
187-
$coveredDimensionSpacePoints,
188-
$workspaceNameForWriting
204+
$transformationSteps = $transformationSteps->merge(
205+
$transformations->executeNodeBased(
206+
$node,
207+
$coveredDimensionSpacePoints,
208+
$workspaceNameForWriting
209+
)
189210
);
190211
}
191212
}
192213
}
193214
}
194215
}
195216
}
217+
218+
return $transformationSteps;
196219
}
197220
}

0 commit comments

Comments
 (0)