refactor: migration validation #117

larskemper · 2026-01-21T14:59:24Z

Overall refactor of the migration validation behavior. Should lead to much less needed fixes and more accurate / actually fixable errors.

sorry, for the big pr, but refactor was needed 😅. Reach out if I can explain anything and please give me your ideas for better validation!

Changes (file by file)

src/Controller/DataProviderController.php, src/Controller/HistoryController.php, src/Controller/PremappingController.php, src/Controller/StatusController.php:
Introduced all common route definition constants to unify with platform
[Added] src/Controller/ErrorResolutionController.php:
Added two new routes:
- /error-resolution/validate: provides validation for user created fixes to make sure the fix matches original migration validation
- /error-resolution/example-field-structure: in case we can not determine the field type in the administration this route provides an example field structure of json association fields, so the user can follow this structure to create a valid fix.
src/Migration/ErrorResolution/MigrationErrorResolutionService.php: added missing early return if no fixes could be loaded to avoid event spam and unnecessary code execution
[Added] src/Migration/ErrorResolution/MigrationFieldExampleGenerator.php: the generator used for the example field structure route. It takes in a field and tries to build a accurate example structure for the user
[!!!] src/Migration/ErrorResolution/MigrationFix.php: see the comment
src/Migration/Mapping/MappingService.php: removed the mapping lookup. We decided to not validate of a given association is actually existing and just validate the given data
src/Migration/Service/MediaFileProcessorService.php: add where clause to only fetch unprocessed media
src/Migration/Service/MigrationDataWriter.php: change to ->count() to return number of entities that are processed in the current batch, not the absolute total
[!!!] src/Migration/Validation/MigrationEntityValidationService.php:
- Deleted association mapping lookup
- New definition of required field: not system managed (createdAt, ...), actually required in db, no default value
- Validated nested converted data
- More naive way of validating association field, cause the before validation by serializer needed context that we actually can't provide in the migration. Now more like a structural validation of association data.
- Move Exceptions to validation domain for better separation
[!!!] src/Migration/Validation/MigrationFieldValidationService.php:
- same as entity validator: more naive validation, support of nested fields
- splited from entity validator to provide standalone field validation for the error resolution controller
src/Migration/Validation/MigrationValidationResult.php: added log key generation to avoid multiple log for the same invalidation (avoid multiple fixes for same issue)
src/Profile/Shopware/Converter/OrderConverter.php: bug fix (custom field was set to null before, instead of just not adding it to the converted data)
src/Resources/app/administration/src/module/swag-migration/component/swag-migration-error-resolution/swag-migration-error-resolution-field/swag-migration-error-resolution-field-scalar/index.ts: added display of error messages that are provides by the validation route
src/Resources/app/administration/src/module/swag-migration/component/swag-migration-error-resolution/swag-migration-error-resolution-field/swag-migration-error-resolution-field/index.ts: added auto fetch of example field structure in case the admin could not determine a specific field type
src/Resources/app/administration/src/module/swag-migration/service/swag-migration-error-resolution.service.ts: support of nested field paths and overall simplification

…r-optional-database-field

larskemper · 2026-01-22T07:30:32Z

src/Migration/ErrorResolution/MigrationFix.php

+        }
+
+        // key points to a list, apply to all items in the list
+        if (isset($data[$key]) && \is_array($data[$key]) && \array_is_list($data[$key])) {


example case for this:
entity number_range with converted data like this:

[ 'id' => 'uuid' // ... 'numberRangeSalesChannels' => [ [ // ... 'typeId' => null // (invalid, has to be set) ], [ 'typeId' => null ] ] ]

with this solution the fix would be applied for all sub child of numberRangeSalesChannels.
To support individual fixes for sub childs we would have to rebuild quite a lot of the error resolution to a path syntax like parent.sub.[index].property which we currently are not supporting and would cause problems in the ui (because we group by fieldname).

What do you think? Leave it like this or maybe not support it at all for the moment (would mean these entities can not be migrated, like before anyway)

Is this a realistic scenario with the number ranges?

Im fine with this solution but we should make it clear that this will happen to the user with a warning or tooltip

it just feels not completely right, but it seemed quite common. The problem is that if we don't to this adjustment we ask the user to provide a fix that we actually are not supporting to fix. So we would need new extra logic to prevent them to appear "fixable"

tests/integration/Migration/Validation/MigrationEntityValidationServiceTest.php

…or' into feat/add-validation-of-resolution-values

src/Controller/DataProviderController.php

DennisGarding · 2026-01-23T06:37:46Z

src/Migration/ErrorResolution/MigrationErrorResolutionService.php

    /**
     * @param array<int|string, array<int|string, mixed>> $data
+     *
+     * @throws Exception


@throws Exception can be removed. In the function is no throw

fetchAllAssociative() inside of loadFixes() can throw

yes, but normaly annotate exceptions if its realy thrown in the mehtod. So fetchAllAssociative need the @throw tag

DennisGarding · 2026-01-23T06:38:44Z

src/Migration/ErrorResolution/MigrationErrorResolutionService.php

    /**
     * Loads fixes from the database and populates them in the context.
+     *
+     * @throws Exception


jozsefdamokos · 2026-01-23T13:40:00Z

src/Migration/ErrorResolution/MigrationFix.php

     */
    public function apply(array &$item): void
    {
-        /*


We should keep this I think

jozsefdamokos · 2026-01-23T13:42:33Z

src/Migration/ErrorResolution/MigrationFix.php

+     * @param array<string|int, mixed> $data
+     * @param array<int, string> $remainingPath
+     */
+    private function applyToPath(array &$data, array $remainingPath, mixed $value): void


Maybe better to call the function param simply $path and set $remainingPath inside the function

jozsefdamokos · 2026-01-23T13:50:22Z

src/Migration/Validation/MigrationEntityValidationService.php

+    /**
+     * Validates that all required fields are present in the converted data.
+     *
+     * @throws \Exception|Exception


Is this needed? Same question for all methods in this class 😉

i mean technically definitely not, but documenting the most ide's will highlight the usage of the method "this can throw..." and that's not unrelevant for the validation, cause all thrown errors should be catched to create logs / fixes. But I can also live with removing them 😄

~~For me, this line resulted in a "Why both \Exception and Exception?" question 😄~~

nevermind 😅

use Doctrine\DBAL\Exception;

jozsefdamokos · 2026-01-23T13:50:31Z

src/Migration/Validation/MigrationEntityValidationService.php

+     * @param array<string, mixed>|null $convertedEntity
+     * @param array<string, mixed> $sourceData
+     *
+     * @throws \Exception


Is this needed?

jozsefdamokos · 2026-01-23T14:05:59Z

src/Migration/Validation/MigrationFieldValidationService.php

+     * Resolves a potentially nested field path (e.g., "prices.shippingMethodId") to its target.
+     * Traverses through association fields to find the final entity definition and field.
+     *
+     * @throws \Exception


Is this needed? Same question for the other methods where @throws is used

vintagesucks

🚀🚀🚀

vintagesucks · 2026-01-23T13:38:39Z

src/Migration/Validation/Exception/MigrationValidationException.php

+        return new self(
+            Response::HTTP_INTERNAL_SERVER_ERROR,
+            self::VALIDATION_INVALID_ID,
+            'The id "{{ entityId }}" for entity "{{ entityName }}" is not a valid Uuid',


Suggested change

'The id "{{ entityId }}" for entity "{{ entityName }}" is not a valid Uuid',

'The id "{{ entityId }}" for entity "{{ entityName }}" is not a valid UUID',

vintagesucks · 2026-01-23T13:40:28Z

src/Migration/Validation/Log/MigrationValidationInvalidRequiredFieldValueLog.php

    }

    public function getCode(): string
    {
-        return 'SWAG_MIGRATION_VALIDATION_UNEXPECTED_FIELD';


Snippets are still present

vintagesucks · 2026-01-23T13:44:53Z

src/Migration/Validation/MigrationEntityValidationService.php

+class MigrationEntityValidationService implements ResetInterface
+{
+    /**
+     * System managed that managed by Shopware and should not be validated as required fields


Suggested change

* System managed that managed by Shopware and should not be validated as required fields

* System managed fields are managed by Shopware and should not be validated as required fields

or

Suggested change

* System managed that managed by Shopware and should not be validated as required fields

* System managed fields that are managed by Shopware and should not be validated as required fields

vintagesucks · 2026-01-23T13:53:52Z

...ag-migration-error-resolution-field/swag-migration-error-resolution-field-unhandled/index.ts

-                return value;
-            } catch {
-                this.error = {
-                    detail: this.$tc('swag-migration.index.error-resolution.errors.invalidJsonInput'),


Snippets are still present

jozsefdamokos · 2026-01-23T14:15:41Z

src/Migration/Validation/MigrationEntityValidationService.php

+                : $field->getReferenceDefinition();
+
+            foreach ($value as $nestedEntityData) {
+                $this->validateNestedEntityData(


Can we not use the same method validateFieldValues here? From the docblocks it seems to me that these two methods do a similar job.

jozsefdamokos · 2026-01-23T14:32:04Z

tests/integration/Migration/Validation/MigrationFieldValidationServiceTest.php

Some tests are missing for validating association structures

jozsefdamokos · 2026-01-23T14:33:36Z

Looks good! Great job!

larskemper added 24 commits December 19, 2025 10:13

refactor: filter optional database fields

1fc5d46

test: nullable validation fields

9d5ab13

refactor: implement reset interface

5bfc5e4

fix: search not flushed mappings for associations

2260f76

Merge branch 'feature/migration-logging-refactor' into refactor/filte…

e35fa66

…r-optional-database-field

refactor: validation of migration

b13a0b2

test: fix

99968d0

refactor: validation

49d4f79

feat: cleanup and add tests

54de53b

fix: null order custom field

2be5679

Merge branch 'feature/migration-logging-refactor' into refactor/filte…

89d8147

…r-optional-database-field

fix: test

9526887

feat: add validation of resolution values

9036ca7

test: fix

9cb5a7a

feat: add defaults to examples

6e7a457

test: error resolution controller

7dd489b

test: jest

4b4a984

refactor: unify controller annoations

82a5c79

refactor: rebase & adjust to changes

ec3f5da

fix: typo

0de2bed

fix: card border

f29f329

feat: add nested field validation

6b22087

refactor: validation

4a4ddef

test: refine php tests

6b242c5

larskemper self-assigned this Jan 21, 2026

larskemper changed the title ~~feat: add validation of resolution values~~ refactor: migration validation Jan 21, 2026

refactor: simplify

534d4d1

larskemper force-pushed the feat/add-validation-of-resolution-values branch from 80b2502 to 534d4d1 Compare January 21, 2026 15:52

larskemper commented Jan 22, 2026

View reviewed changes

larskemper requested a review from a team January 22, 2026 08:33

larskemper requested review from DennisGarding, MalteJanz, ennasus4sun, jozsefdamokos and vintagesucks and removed request for a team January 22, 2026 08:33

larskemper marked this pull request as ready for review January 22, 2026 08:40

larskemper commented Jan 22, 2026

View reviewed changes

tests/integration/Migration/Validation/MigrationEntityValidationServiceTest.php Outdated Show resolved Hide resolved

larskemper added 3 commits January 22, 2026 13:15

test: add indexed generator keys

2f81945

fix: remove old invalid code

9583ee2

Merge remote-tracking branch 'origin/feature/migration-logging-refact…

a2554ff

…or' into feat/add-validation-of-resolution-values

DennisGarding approved these changes Jan 23, 2026

View reviewed changes

jozsefdamokos reviewed Jan 23, 2026

View reviewed changes

vintagesucks approved these changes Jan 23, 2026

View reviewed changes

jozsefdamokos reviewed Jan 23, 2026

View reviewed changes

jozsefdamokos requested changes Jan 23, 2026

View reviewed changes

	'The id "{{ entityId }}" for entity "{{ entityName }}" is not a valid Uuid',
	'The id "{{ entityId }}" for entity "{{ entityName }}" is not a valid UUID',

	* System managed that managed by Shopware and should not be validated as required fields
	* System managed fields are managed by Shopware and should not be validated as required fields

	* System managed that managed by Shopware and should not be validated as required fields
	* System managed fields that are managed by Shopware and should not be validated as required fields

refactor: migration validation #117

Are you sure you want to change the base?

refactor: migration validation #117

Uh oh!

Conversation

larskemper commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes (file by file)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larskemper Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DennisGarding Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jozsefdamokos Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larskemper Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vintagesucks Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vintagesucks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vintagesucks Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jozsefdamokos commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

larskemper commented Jan 21, 2026 •

edited

Loading

larskemper Jan 23, 2026 •

edited

Loading

DennisGarding Jan 23, 2026 •

edited

Loading

jozsefdamokos Jan 23, 2026 •

edited

Loading

larskemper Jan 23, 2026 •

edited

Loading

vintagesucks Jan 23, 2026 •

edited

Loading

vintagesucks Jan 23, 2026 •

edited

Loading