From b9519af3701c6343586dfe0ecbba29c81ab85c47 Mon Sep 17 00:00:00 2001 From: Enrico Ellguth Date: Wed, 21 Oct 2026 16:20:54 +0200 Subject: [PATCH 1/2] Fix clean-up-empty-columns for columns with a default value mergeEntity() looked up the attribute code in the clean-up column keys, which hold header positions. The lookup never matched, so an empty cell of a column with a default value never deleted the stored value, even when the column was listed in clean-up-empty-columns. The attribute code is translated into its header position first. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 6 + src/Observers/AttributeObserverTrait.php | 6 +- .../AttributeObserverTraitMergeEntityImpl.php | 216 ++++++++++++++++++ .../AttributeObserverTraitMergeEntityTest.php | 131 +++++++++++ 4 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php create mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index cadb015..33d6d1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Version 18.3.1 + +## Bugfixes + +* Fix `AttributeObserverTrait::mergeEntity()`: columns with a default value (`default-values`) were never emptied by `clean-up-empty-columns`, because the attribute code was looked up in the list of header positions + # Version 18.3.0 ## Features diff --git a/src/Observers/AttributeObserverTrait.php b/src/Observers/AttributeObserverTrait.php index 4a147b6..e71eae1 100644 --- a/src/Observers/AttributeObserverTrait.php +++ b/src/Observers/AttributeObserverTrait.php @@ -465,9 +465,11 @@ protected function mergeEntity(array $entity, array $attr, $changeSetName = null // if the value is empty AND it is IN the array with default column values // BUT it is NOT in the array with columns we want to clean-up the default // column value has to be removed, because we do NOT want to override the - // value existing in the database + // value existing in the database. The clean-up column keys are the header + // positions of the columns, so the attribute code has to be translated first if (array_key_exists($this->attributeCode, $this->getDefaultColumnValues()) && - array_key_exists($this->attributeCode, $this->getCleanUpEmptyColumnKeys()) === false + !($this->hasHeader($this->attributeCode) && + in_array($this->getHeader($this->attributeCode), $this->getCleanUpEmptyColumnKeys() ?? [], true)) ) { // remove the value from the array with the column values, because // this is the default value from the database and it should NOT diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php new file mode 100644 index 0000000..691113d --- /dev/null +++ b/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php @@ -0,0 +1,216 @@ + + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ + +namespace TechDivision\Import\Observers; + +/** + * Observer double that makes mergeEntity() of the attribute observer trait callable. + * + * @author TechDivision GmbH + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ +class AttributeObserverTraitMergeEntityImpl extends AbstractAttributeObserver +{ + + /** + * The header positions by column name. + * + * @var array + */ + private $headers; + + /** + * Initializes the double. + * + * @param array $headers The header positions by column name + * @param array $defaultColumnValues The columns with a default value + * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up + */ + public function __construct(array $headers, array $defaultColumnValues, ?array $cleanUpColumnKeys) + { + $this->headers = $headers; + $this->defaultColumnValues = $defaultColumnValues; + $this->cleanUpEmptyColumnKeys = $cleanUpColumnKeys; + } + + /** + * Merges an empty attribute value into an existing entity. + * + * @param string $attributeCode The attribute code + * @param array $entity The stored entity + * + * @return array The merged entity + */ + public function merge($attributeCode, array $entity) + { + $this->attributeCode = $attributeCode; + $this->attributeValue = ''; + return $this->mergeEntity($entity, array(\TechDivision\Import\Utils\MemberNames::VALUE => '')); + } + + /** + * Returns the operation the last merge has chosen. + * + * @return string The operation + */ + public function getOperation() + { + return $this->operation; + } + + /** + * {@inheritDoc} + */ + public function hasHeader($name) + { + return isset($this->headers[$name]); + } + + /** + * {@inheritDoc} + */ + public function getHeader($name) + { + return $this->headers[$name]; + } + + /** + * {@inheritDoc} + */ + protected function persistVarcharAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistIntAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistDecimalAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistDatetimeAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistTextAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function getAttributesByPrimaryKeyAndStoreId($pk, $storeId) + { + return array(); + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKey() + { + return null; + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKeyMemberName() + { + return 'entity_id'; + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKeyColumnName() + { + return 'sku'; + } + + /** + * {@inheritDoc} + */ + protected function storeViewHasBeenProcessed($pk, $storeViewCode) + { + return false; + } + + /** + * {@inheritDoc} + */ + protected function deleteDatetimeAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteDecimalAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteIntAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteTextAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteVarcharAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function hasChanges(array $entity) + { + return true; + } + + /** + * {@inheritDoc} + */ + public function hasValue($name) + { + return false; + } +} diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php new file mode 100644 index 0000000..0657c09 --- /dev/null +++ b/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php @@ -0,0 +1,131 @@ + + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ + +namespace TechDivision\Import\Observers; + +use PHPUnit\Framework\TestCase; +use TechDivision\Import\Utils\MemberNames; +use TechDivision\Import\Utils\OperationNames; + +/** + * Test class for the mergeEntity() method of the attribute observer trait. + * + * An empty cell of a column with a default value keeps the stored value, unless the column + * is listed in clean-up-empty-columns. The clean-up column keys hold the header positions + * of the columns, not the attribute codes. + * + * @author TechDivision GmbH + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ +class AttributeObserverTraitMergeEntityTest extends TestCase +{ + + /** + * The header positions of the test columns. + * + * @var array + */ + private $headers = array('sku' => 0, 'aup_is_new' => 1, 'aup_plain' => 2); + + /** + * Creates the observer double. + * + * @param array $defaultColumnValues The columns with a default value + * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up + * + * @return \TechDivision\Import\Observers\AttributeObserverTraitMergeEntityImpl The observer + */ + private function createObserver(array $defaultColumnValues, ?array $cleanUpColumnKeys) + { + return new AttributeObserverTraitMergeEntityImpl($this->headers, $defaultColumnValues, $cleanUpColumnKeys); + } + + /** + * An empty cell of a default value column that is listed to be cleaned up deletes the value. + * + * @return void + */ + public function testListedDefaultValueColumnIsDeleted() + { + $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_is_new'])); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::DELETE, $observer->getOperation()); + $this->assertSame('', $result[MemberNames::VALUE]); + } + + /** + * An empty cell of a default value column that is NOT listed keeps the stored value. + * + * @return void + */ + public function testUnlistedDefaultValueColumnKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_plain'])); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } + + /** + * Without any clean-up column the value of a default value column is kept as well. + * + * @return void + */ + public function testDefaultValueColumnWithoutCleanUpColumnsKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_is_new' => ''), null); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } + + /** + * A column without a default value is deleted on an empty cell, whatever the clean-up list says. + * + * @return void + */ + public function testColumnWithoutDefaultValueIsDeleted() + { + $observer = $this->createObserver(array(), array()); + + $result = $observer->merge('aup_plain', array(MemberNames::VALUE => 'x')); + + $this->assertSame(OperationNames::DELETE, $observer->getOperation()); + $this->assertSame('', $result[MemberNames::VALUE]); + } + + /** + * A default value column that is not part of the file is not translated and keeps the stored value. + * + * @return void + */ + public function testDefaultValueColumnWithoutHeaderKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_missing' => ''), array($this->headers['aup_is_new'])); + + $result = $observer->merge('aup_missing', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } +} From 179fe1e37980cb6ad48e6890dd3c124642271ca0 Mon Sep 17 00:00:00 2001 From: Victor Svizev Date: Tue, 3 Nov 2026 03:18:01 +0100 Subject: [PATCH 2/2] PAC-1000 Load XML DI configuration with the Symfony loader where available. - Use the XML loader shipped with `symfony/dependency-injection` on 6.4 and 7.x and fall back to the copied `XmlFileLoader` on 8.x. - Widen the `symfony/dependency-injection` constraint to `^6.4|^7.0|^8.0`. --- CHANGELOG.md | 1 + composer.json | 1 + .../DependencyInjection/ImportExtension.php | 6 +- .../AttributeObserverTraitMergeEntityImpl.php | 216 ------------------ .../AttributeObserverTraitMergeEntityTest.php | 131 ----------- 5 files changed, 7 insertions(+), 348 deletions(-) delete mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php delete mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d6d1a..2999e4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Bugfixes * Fix `AttributeObserverTrait::mergeEntity()`: columns with a default value (`default-values`) were never emptied by `clean-up-empty-columns`, because the attribute code was looked up in the list of header positions +* Fix XML DI configuration loading for `symfony/dependency-injection` 6.4 and 7.x: `ImportExtension` uses the loader shipped with Symfony again and falls back to the copied `XmlFileLoader` only where it is not available (Symfony 8) # Version 18.3.0 diff --git a/composer.json b/composer.json index c1247dd..c5c9a70 100755 --- a/composer.json +++ b/composer.json @@ -12,6 +12,7 @@ "league/event": "^2.2", "ramsey/uuid": "^4.2|^4.7", "symfony/mailer": "^5.4|^6.4|^7.4", + "symfony/dependency-injection": "^6.4|^7.0|^8.0", "laminas/laminas-filter": "^2.31", "techdivision/import-dbal": "^2.1", "techdivision/import-dbal-collection": "^2.2", diff --git a/symfony/DependencyInjection/ImportExtension.php b/symfony/DependencyInjection/ImportExtension.php index 9dbb380..b8e1ce5 100644 --- a/symfony/DependencyInjection/ImportExtension.php +++ b/symfony/DependencyInjection/ImportExtension.php @@ -17,6 +17,7 @@ use Symfony\Component\Config\FileLocator; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Loader\XmlFileLoader as SymfonyXmlFileLoader; use TechDivision\Import\DependencyInjection\Loader\XmlFileLoader; /** @@ -39,7 +40,10 @@ class ImportExtension extends Extension */ public function load(array $configs, ContainerBuilder $container): void { - $loader = new XmlFileLoader($container, new FileLocator(dirname(__DIR__) . '/Resources/config')); + // use the XML loader shipped with symfony/dependency-injection where available (6.4 and 7.x), + // symfony 8 removed it, so the copy of the 7.x loader has to be used there + $loaderClass = class_exists(SymfonyXmlFileLoader::class) ? SymfonyXmlFileLoader::class : XmlFileLoader::class; + $loader = new $loaderClass($container, new FileLocator(dirname(__DIR__) . '/Resources/config')); $loader->load('services.xml'); } } diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php deleted file mode 100644 index 691113d..0000000 --- a/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php +++ /dev/null @@ -1,216 +0,0 @@ - - * @copyright 2026 TechDivision GmbH - * @license https://opensource.org/licenses/MIT - * @link https://github.com/techdivision/import - * @link http://www.techdivision.com - */ - -namespace TechDivision\Import\Observers; - -/** - * Observer double that makes mergeEntity() of the attribute observer trait callable. - * - * @author TechDivision GmbH - * @copyright 2026 TechDivision GmbH - * @license https://opensource.org/licenses/MIT - * @link https://github.com/techdivision/import - * @link http://www.techdivision.com - */ -class AttributeObserverTraitMergeEntityImpl extends AbstractAttributeObserver -{ - - /** - * The header positions by column name. - * - * @var array - */ - private $headers; - - /** - * Initializes the double. - * - * @param array $headers The header positions by column name - * @param array $defaultColumnValues The columns with a default value - * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up - */ - public function __construct(array $headers, array $defaultColumnValues, ?array $cleanUpColumnKeys) - { - $this->headers = $headers; - $this->defaultColumnValues = $defaultColumnValues; - $this->cleanUpEmptyColumnKeys = $cleanUpColumnKeys; - } - - /** - * Merges an empty attribute value into an existing entity. - * - * @param string $attributeCode The attribute code - * @param array $entity The stored entity - * - * @return array The merged entity - */ - public function merge($attributeCode, array $entity) - { - $this->attributeCode = $attributeCode; - $this->attributeValue = ''; - return $this->mergeEntity($entity, array(\TechDivision\Import\Utils\MemberNames::VALUE => '')); - } - - /** - * Returns the operation the last merge has chosen. - * - * @return string The operation - */ - public function getOperation() - { - return $this->operation; - } - - /** - * {@inheritDoc} - */ - public function hasHeader($name) - { - return isset($this->headers[$name]); - } - - /** - * {@inheritDoc} - */ - public function getHeader($name) - { - return $this->headers[$name]; - } - - /** - * {@inheritDoc} - */ - protected function persistVarcharAttribute($attribute) - { - } - - /** - * {@inheritDoc} - */ - protected function persistIntAttribute($attribute) - { - } - - /** - * {@inheritDoc} - */ - protected function persistDecimalAttribute($attribute) - { - } - - /** - * {@inheritDoc} - */ - protected function persistDatetimeAttribute($attribute) - { - } - - /** - * {@inheritDoc} - */ - protected function persistTextAttribute($attribute) - { - } - - /** - * {@inheritDoc} - */ - protected function getAttributesByPrimaryKeyAndStoreId($pk, $storeId) - { - return array(); - } - - /** - * {@inheritDoc} - */ - protected function getPrimaryKey() - { - return null; - } - - /** - * {@inheritDoc} - */ - protected function getPrimaryKeyMemberName() - { - return 'entity_id'; - } - - /** - * {@inheritDoc} - */ - protected function getPrimaryKeyColumnName() - { - return 'sku'; - } - - /** - * {@inheritDoc} - */ - protected function storeViewHasBeenProcessed($pk, $storeViewCode) - { - return false; - } - - /** - * {@inheritDoc} - */ - protected function deleteDatetimeAttribute(array $row, $name = null) - { - } - - /** - * {@inheritDoc} - */ - protected function deleteDecimalAttribute(array $row, $name = null) - { - } - - /** - * {@inheritDoc} - */ - protected function deleteIntAttribute(array $row, $name = null) - { - } - - /** - * {@inheritDoc} - */ - protected function deleteTextAttribute(array $row, $name = null) - { - } - - /** - * {@inheritDoc} - */ - protected function deleteVarcharAttribute(array $row, $name = null) - { - } - - /** - * {@inheritDoc} - */ - protected function hasChanges(array $entity) - { - return true; - } - - /** - * {@inheritDoc} - */ - public function hasValue($name) - { - return false; - } -} diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php deleted file mode 100644 index 0657c09..0000000 --- a/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php +++ /dev/null @@ -1,131 +0,0 @@ - - * @copyright 2026 TechDivision GmbH - * @license https://opensource.org/licenses/MIT - * @link https://github.com/techdivision/import - * @link http://www.techdivision.com - */ - -namespace TechDivision\Import\Observers; - -use PHPUnit\Framework\TestCase; -use TechDivision\Import\Utils\MemberNames; -use TechDivision\Import\Utils\OperationNames; - -/** - * Test class for the mergeEntity() method of the attribute observer trait. - * - * An empty cell of a column with a default value keeps the stored value, unless the column - * is listed in clean-up-empty-columns. The clean-up column keys hold the header positions - * of the columns, not the attribute codes. - * - * @author TechDivision GmbH - * @copyright 2026 TechDivision GmbH - * @license https://opensource.org/licenses/MIT - * @link https://github.com/techdivision/import - * @link http://www.techdivision.com - */ -class AttributeObserverTraitMergeEntityTest extends TestCase -{ - - /** - * The header positions of the test columns. - * - * @var array - */ - private $headers = array('sku' => 0, 'aup_is_new' => 1, 'aup_plain' => 2); - - /** - * Creates the observer double. - * - * @param array $defaultColumnValues The columns with a default value - * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up - * - * @return \TechDivision\Import\Observers\AttributeObserverTraitMergeEntityImpl The observer - */ - private function createObserver(array $defaultColumnValues, ?array $cleanUpColumnKeys) - { - return new AttributeObserverTraitMergeEntityImpl($this->headers, $defaultColumnValues, $cleanUpColumnKeys); - } - - /** - * An empty cell of a default value column that is listed to be cleaned up deletes the value. - * - * @return void - */ - public function testListedDefaultValueColumnIsDeleted() - { - $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_is_new'])); - - $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); - - $this->assertSame(OperationNames::DELETE, $observer->getOperation()); - $this->assertSame('', $result[MemberNames::VALUE]); - } - - /** - * An empty cell of a default value column that is NOT listed keeps the stored value. - * - * @return void - */ - public function testUnlistedDefaultValueColumnKeepsTheStoredValue() - { - $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_plain'])); - - $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); - - $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); - $this->assertSame('0', $result[MemberNames::VALUE]); - } - - /** - * Without any clean-up column the value of a default value column is kept as well. - * - * @return void - */ - public function testDefaultValueColumnWithoutCleanUpColumnsKeepsTheStoredValue() - { - $observer = $this->createObserver(array('aup_is_new' => ''), null); - - $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); - - $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); - $this->assertSame('0', $result[MemberNames::VALUE]); - } - - /** - * A column without a default value is deleted on an empty cell, whatever the clean-up list says. - * - * @return void - */ - public function testColumnWithoutDefaultValueIsDeleted() - { - $observer = $this->createObserver(array(), array()); - - $result = $observer->merge('aup_plain', array(MemberNames::VALUE => 'x')); - - $this->assertSame(OperationNames::DELETE, $observer->getOperation()); - $this->assertSame('', $result[MemberNames::VALUE]); - } - - /** - * A default value column that is not part of the file is not translated and keeps the stored value. - * - * @return void - */ - public function testDefaultValueColumnWithoutHeaderKeepsTheStoredValue() - { - $observer = $this->createObserver(array('aup_missing' => ''), array($this->headers['aup_is_new'])); - - $result = $observer->merge('aup_missing', array(MemberNames::VALUE => '0')); - - $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); - $this->assertSame('0', $result[MemberNames::VALUE]); - } -}