Skip to content

refactor!: migration logging system #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
larskemper wants to merge 5 commits into
base: feature/migration-logging-refactor
Choose a base branch
from
Draft

refactor!: migration logging system #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4dedb9
refactor: connection & log usage
larskemper Aug 22, 2025
a1c80c1
docs: add todos
larskemper Aug 26, 2025
20f1be8
ci: fix
larskemper Aug 26, 2025
c83cec3
fix: phpunit
larskemper Aug 26, 2025
a54284c
feat: add meaningful data to magento migration logs (#32)
larskemper Aug 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/setup-magento-plugin/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ runs:
[
{
"name": "SwagMigrationAssistant",
"branch": "feature/migration-logging-refactor",
Copy link
Member Author

@larskemper larskemper Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi: this is needed as long as we haven't released the new SwagMigrationAssistant version

Copy link
Contributor

@MalteJanz MalteJanz Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAML should allow comments, so I would suggest adding something like # Todo: change back to trunk after assistant epic gets merged https://github.com/shopware/shopware/issues/10417

Copy link
Member Author

@larskemper larskemper Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be switch again before merging this, so I thought a thread is more visible

Copy link
Contributor

@MalteJanz MalteJanz Sep 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That also works, just thought this PR is targeting the feature branch in magento and later if the merge that feature branch into trunk again we would need to revert this change 🤷‍♂️ .

But either way, we just have to keep track of this somehow and a comment would at least be obvious during code review 😅

"repo": "https://github.com/shopware/SwagMigrationAssistant.git"
}
]
10 changes: 10 additions & 0 deletions phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,13 @@ parameters:

- # Ignore undefined function calls on MappingServiceInterface because PHPStan dont get the inheritance
message: '#Call to an undefined method SwagMigrationAssistant\\Migration\\Mapping\\MappingServiceInterface::createListItemMapping|getUuidList\(\)#'

- # TODO remove this for 6.8 compatibility
message: '#Access to constant on deprecated class Shopware\\Core\\Checkout\\Payment\\Cart\\PaymentHandler\\DebitPayment:#'
paths:
- tests/**/*Test.php

- # TODO remove this for 6.8 compatibility
message: '#Call to deprecated method __construct\(\) of class Shopware\\Core\\Framework\\DataAbstractionLayer\\EntityDefinition:#'
paths:
- tests/**/*Test.php
33 changes: 5 additions & 28 deletions src/Migration/Logging/FileHandleErrorLog.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@
namespace Swag\MigrationMagento\Migration\Logging;

use Shopware\Core\Framework\Log\Package;
use SwagMigrationAssistant\Migration\Logging\Log\BaseRunLogEntry;
use SwagMigrationAssistant\Migration\Logging\Log\Builder\AbstractSwagMigrationLogEntry;

#[Package('fundamentals@after-sales')]
class FileHandleErrorLog extends BaseRunLogEntry
readonly class FileHandleErrorLog extends AbstractSwagMigrationLogEntry
{
public function __construct(
string $runId,
string $entity,
?string $sourceId = null,
) {
parent::__construct($runId, $entity, $sourceId);
public function isUserFixable(): bool
{
return false;
}

public function getLevel(): string
Expand All @@ -30,24 +27,4 @@ public function getCode(): string
{
return 'SWAG_MIGRATION_MAGENTO__COULD_NOT_OPEN_FILE';
}

public function getTitle(): string
{
return 'An exception occurred';
}

public function getParameters(): array
{
return [
'entity' => $this->getEntity(),
'sourceId' => $this->getSourceId(),
'exceptionCode' => $this->getCode(),
'description' => 'Could not open file to read or write',
];
}

public function getDescription(): string
{
return $this->getParameters()['description'];
}
}
26 changes: 13 additions & 13 deletions src/Profile/Magento/Converter/CategoryConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace Swag\MigrationMagento\Profile\Magento\Converter;

use Shopware\Core\Content\Category\CategoryDefinition;
use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\Log\Package;
use Swag\MigrationMagento\Migration\Mapping\MagentoMappingServiceInterface;
Expand All @@ -15,6 +16,7 @@
use SwagMigrationAssistant\Exception\MigrationException;
use SwagMigrationAssistant\Migration\Converter\ConvertStruct;
use SwagMigrationAssistant\Migration\DataSelection\DefaultEntities;
use SwagMigrationAssistant\Migration\Logging\Log\Builder\SwagMigrationLogBuilder;
use SwagMigrationAssistant\Migration\Logging\Log\EmptyNecessaryFieldRunLog;
use SwagMigrationAssistant\Migration\Logging\LoggingServiceInterface;
use SwagMigrationAssistant\Migration\Mapping\Lookup\DefaultCmsPageLookup;
Expand Down Expand Up @@ -85,12 +87,7 @@ public function getSourceIdentifier(array $data): string
public function convert(array $data, Context $context, MigrationContextInterface $migrationContext): ConvertStruct
{
$this->context = $context;

$connection = $migrationContext->getConnection();
$this->connectionId = '';
if ($connection !== null) {
$this->connectionId = $connection->getId();
}
$this->connectionId = $migrationContext->getConnection()->getId();

// Ignore the magento root category
if (isset($data['parent_id']) && $data['parent_id'] === '0') {
Expand All @@ -104,16 +101,19 @@ public function convert(array $data, Context $context, MigrationContextInterface

return new ConvertStruct(null, $data);
}
$rootCategoryMapping = $this->mappingService->getMapping($this->connectionId, MagentoDefaults::ROOT_CATEGORY, $data['parent_id'], $context);

$rootCategoryMapping = $this->mappingService->getMapping($this->connectionId, MagentoDefaults::ROOT_CATEGORY, $data['parent_id'], $context);
$fields = $this->checkForEmptyRequiredDataFields($data, self::$requiredDataFieldKeys);

if (!empty($fields)) {
$this->loggingService->addLogEntry(new EmptyNecessaryFieldRunLog(
$migrationContext->getRunUuid(),
DefaultEntities::CATEGORY,
$data['entity_id'],
\implode(',', $fields)
));
$this->loggingService->addLogForEach(
$fields,
fn (string $key) => SwagMigrationLogBuilder::fromMigrationContext($migrationContext)
->withEntityName(CategoryDefinition::ENTITY_NAME)
->withFieldSourcePath($key)
->withSourceData($data)
->build(EmptyNecessaryFieldRunLog::class)
Comment on lines +109 to +115
Copy link
Contributor

@MalteJanz MalteJanz Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this would produce duplicated logs as well with your new validation right?
shopware/SwagMigrationAssistant#51 (comment)

Just something to keep in mind (you can also create a ticket / additional PR for adjusting Magento again, if that's necessary 🙈 )

Copy link
Member Author

@larskemper larskemper Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, basically all EmptyNecessaryFieldRunLog Logs are somewhat duplicates

Copy link
Member Author

@larskemper larskemper Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi: created a new ticket for checking the compatibility shopware/shopware#12442

);

return new ConvertStruct(null, $data);
}
Expand Down
6 changes: 1 addition & 5 deletions src/Profile/Magento/Converter/CountryConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ public function convert(array $data, Context $context, MigrationContextInterface
return new ConvertStruct(null, $data);
}

$connection = $migrationContext->getConnection();
$this->connectionId = '';
if ($connection !== null) {
$this->connectionId = $connection->getId();
}
$this->connectionId = $migrationContext->getConnection()->getId();

$this->generateChecksum($data);
$countryMapping = $this->mappingService->getMapping($this->connectionId, DefaultEntities::COUNTRY, $data['isoCode'], $context);
Expand Down
38 changes: 21 additions & 17 deletions src/Profile/Magento/Converter/CrossSellingConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@

namespace Swag\MigrationMagento\Profile\Magento\Converter;

use Shopware\Core\Content\Product\Aggregate\ProductCrossSelling\ProductCrossSellingDefinition;
use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\Log\Package;
use Swag\MigrationMagento\Profile\Magento\DataSelection\DefaultEntities as MagentoDefaultEntities;
use SwagMigrationAssistant\Migration\Converter\ConvertStruct;
use SwagMigrationAssistant\Migration\DataSelection\DefaultEntities;
use SwagMigrationAssistant\Migration\Logging\Log\AssociationRequiredMissingLog;
use SwagMigrationAssistant\Migration\Logging\Log\Builder\SwagMigrationLogBuilder;
use SwagMigrationAssistant\Migration\MigrationContextInterface;

#[Package('fundamentals@after-sales')]
Expand All @@ -39,11 +41,7 @@ public function convert(array $data, Context $context, MigrationContextInterface
$this->oldIdentifier = $data['link_id'];
$this->context = $context;

$connection = $migrationContext->getConnection();
$this->connectionId = '';
if ($connection !== null) {
$this->connectionId = $connection->getId();
}
$this->connectionId = $migrationContext->getConnection()->getId();

$this->mainMapping = $this->mappingService->getOrCreateMapping(
$this->connectionId,
Expand All @@ -64,12 +62,15 @@ public function convert(array $data, Context $context, MigrationContextInterface
);

if ($sourceProductMapping === null) {
$this->loggingService->addLogEntry(new AssociationRequiredMissingLog(
$this->runId,
DefaultEntities::PRODUCT,
$data['sourceProductId'],
$data['type']
));
$this->loggingService->addLogEntry(
SwagMigrationLogBuilder::fromMigrationContext($migrationContext)
->withEntityName(ProductCrossSellingDefinition::ENTITY_NAME)
->withFieldName('productId')
->withFieldSourcePath('sourceProductId')
->withSourceData($data)
->withConvertedData($converted)
->build(AssociationRequiredMissingLog::class)
);

return new ConvertStruct(null, $data);
}
Expand All @@ -83,12 +84,15 @@ public function convert(array $data, Context $context, MigrationContextInterface
);

if ($relatedProductMapping === null) {
$this->loggingService->addLogEntry(new AssociationRequiredMissingLog(
$this->runId,
DefaultEntities::PRODUCT,
$data['linked_product_id'],
$data['type']
));
$this->loggingService->addLogEntry(
SwagMigrationLogBuilder::fromMigrationContext($migrationContext)
->withEntityName(ProductCrossSellingDefinition::ENTITY_NAME)
->withFieldName('productId')
->withFieldSourcePath('linked_product_id')
->withSourceData($data)
->withConvertedData($converted)
->build(AssociationRequiredMissingLog::class)
);

return new ConvertStruct(null, $data);
}
Expand Down
6 changes: 1 addition & 5 deletions src/Profile/Magento/Converter/CurrencyConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ public function convert(array $data, Context $context, MigrationContextInterface
}
unset($data['isoCode']);

$connection = $migrationContext->getConnection();
$this->connectionId = '';
if ($connection !== null) {
$this->connectionId = $connection->getId();
}
$this->connectionId = $migrationContext->getConnection()->getId();

$this->generateChecksum($data);

Expand Down
20 changes: 9 additions & 11 deletions src/Profile/Magento/Converter/CustomFieldConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@

use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\Log\Package;
use Shopware\Core\System\CustomField\CustomFieldDefinition;
use SwagMigrationAssistant\Migration\Converter\Converter;
use SwagMigrationAssistant\Migration\Converter\ConvertStruct;
use SwagMigrationAssistant\Migration\DataSelection\DefaultEntities;
use SwagMigrationAssistant\Migration\Logging\Log\AssociationRequiredMissingLog;
use SwagMigrationAssistant\Migration\Logging\Log\Builder\SwagMigrationLogBuilder;
use SwagMigrationAssistant\Migration\MigrationContextInterface;

#[Package('fundamentals@after-sales')]
Expand Down Expand Up @@ -46,11 +48,7 @@ public function convert(array $data, Context $context, MigrationContextInterface
$this->migrationContext = $migrationContext;
$type = $this->validateType($data);

$connection = $migrationContext->getConnection();
$this->connectionId = '';
if ($connection !== null) {
$this->connectionId = $connection->getId();
}
$this->connectionId = $migrationContext->getConnection()->getId();

$entityName = $this->getDataSetEntity($migrationContext);

Expand All @@ -67,12 +65,12 @@ public function convert(array $data, Context $context, MigrationContextInterface

if ($defaultLocale === null) {
$this->loggingService->addLogEntry(
new AssociationRequiredMissingLog(
$migrationContext->getRunUuid(),
DefaultEntities::LOCALE,
'global_default',
DefaultEntities::CUSTOM_FIELD_SET
)
SwagMigrationLogBuilder::fromMigrationContext($migrationContext)
->withEntityName(CustomFieldDefinition::ENTITY_NAME)
->withFieldName('locale')
->withFieldSourcePath('global_default')
->withSourceData($data)
->build(AssociationRequiredMissingLog::class)
);

return new ConvertStruct(null, $data);
Expand Down
Loading