From 14caa066b0f9fb51dbb0f907f893e519a541c44c Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 15:27:47 -0400 Subject: [PATCH 1/7] MAGE-1383 Separate functions to Helper class for testability --- Console/Command/SynonymDeduplicateCommand.php | 43 ++-------- Helper/ArrayDeduplicator.php | 43 ++++++++++ Test/Unit/Helper/ArrayDeduplicatorTest.php | 86 +++++++++++++++++++ 3 files changed, 134 insertions(+), 38 deletions(-) create mode 100644 Helper/ArrayDeduplicator.php create mode 100644 Test/Unit/Helper/ArrayDeduplicatorTest.php diff --git a/Console/Command/SynonymDeduplicateCommand.php b/Console/Command/SynonymDeduplicateCommand.php index 3f5f582c0..f58d2d357 100644 --- a/Console/Command/SynonymDeduplicateCommand.php +++ b/Console/Command/SynonymDeduplicateCommand.php @@ -2,11 +2,13 @@ namespace Algolia\AlgoliaSearch\Console\Command; +use Algolia\AlgoliaSearch\Helper\ArrayDeduplicator; use Algolia\AlgoliaSearch\Service\AlgoliaConnector; use Algolia\AlgoliaSearch\Service\Product\IndexOptionsBuilder; use Algolia\AlgoliaSearch\Service\StoreNameFetcher; use Magento\Framework\App\State; use Magento\Framework\Console\Cli; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Store\Model\StoreManagerInterface; use Symfony\Component\Console\Input\InputInterface; @@ -20,6 +22,7 @@ public function __construct( protected StoreNameFetcher $storeNameFetcher, protected StoreManagerInterface $storeManager, protected IndexOptionsBuilder $indexOptionsBuilder, + protected ArrayDeduplicator $arrayDeduplicator, ?string $name = null ) { parent::__construct($state, $storeNameFetcher, $name); @@ -52,6 +55,7 @@ protected function getAdditionalDefinition(): array /** * @throws NoSuchEntityException + * @throws LocalizedException */ protected function execute(InputInterface $input, OutputInterface $output): int { @@ -114,7 +118,7 @@ public function dedupeSynonymsForStore(int $storeId): void $indexOptions = $this->indexOptionsBuilder->buildEntityIndexOptions($storeId); $settings = $this->algoliaConnector->getSettings($indexOptions); - $deduped = $this->dedupeSpecificSettings(['synonyms', 'altCorrections'], $settings); + $deduped = $this->arrayDeduplicator->dedupeSpecificSettings(['synonyms', 'altCorrections'], $settings); //bring over as is (no de-dupe necessary) if (array_key_exists('placeholders', $settings)) { @@ -139,41 +143,4 @@ public function dedupeSynonymsForAllStores(): void $this->dedupeSynonymsForStore($storeId); } } - - /** - * @param string[] $settingNames - * @param array $settings - * @return array - */ - protected function dedupeSpecificSettings(array $settingNames, array $settings): array - { - return array_filter( - array_combine( - $settingNames, - array_map( - fn($settingName) => isset($settings[$settingName]) - ? $this->dedupeArrayOfArrays($settings[$settingName]) - : null, - $settingNames - ) - ), - fn($val) => $val !== null - ); - } - - /** - * Find and remove the duplicates in an array of indexed arrays - * Does not work with associative arrays - * @param array $data - * @return array - */ - protected function dedupeArrayOfArrays(array $data): array { - $encoded = array_map('json_encode', $data); - $unique = array_values(array_unique($encoded)); - $decoded = array_map( - fn($item) => json_decode((string) $item, true), - $unique - ); - return $decoded; - } } diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php new file mode 100644 index 000000000..d51ce0e9d --- /dev/null +++ b/Helper/ArrayDeduplicator.php @@ -0,0 +1,43 @@ + $settings + * @return array + */ + public function dedupeSpecificSettings(array $settingNames, array $settings): array + { + return array_filter( + array_combine( + $settingNames, + array_map( + fn($settingName) => isset($settings[$settingName]) + ? $this->dedupeArrayOfArrays($settings[$settingName]) + : null, + $settingNames + ) + ), + fn($val) => $val !== null + ); + } + + /** + * Find and remove the duplicates in an array of indexed arrays + * Does not work with associative arrays + * @param array $data + * @return array + */ + public function dedupeArrayOfArrays(array $data): array { + $encoded = array_map('json_encode', $data); + $unique = array_values(array_unique($encoded)); + $decoded = array_map( + fn($item) => json_decode((string) $item, true), + $unique + ); + return $decoded; + } +} diff --git a/Test/Unit/Helper/ArrayDeduplicatorTest.php b/Test/Unit/Helper/ArrayDeduplicatorTest.php new file mode 100644 index 000000000..f7ff48a37 --- /dev/null +++ b/Test/Unit/Helper/ArrayDeduplicatorTest.php @@ -0,0 +1,86 @@ +deduplicator = new ArrayDeduplicator(); + } + + public function testDedupeArrayOfArraysRemovesExactDuplicates(): void + { + $data = [ + ['a' => 1, 'b' => 2], + ['a' => 1, 'b' => 2], // duplicate + ['a' => 2, 'b' => 3], + ]; + + $result = $this->deduplicator->dedupeArrayOfArrays($data); + + $this->assertCount(2, $result); + $this->assertContains(['a' => 1, 'b' => 2], $result); + $this->assertContains(['a' => 2, 'b' => 3], $result); + } + + public function testDedupeArrayOfArraysKeepsOrderOfFirstOccurrences(): void + { + $data = [ + ['id' => 1], + ['id' => 2], + ['id' => 1], // duplicate + ]; + + $result = $this->deduplicator->dedupeArrayOfArrays($data); + + $this->assertSame([['id' => 1], ['id' => 2]], $result); + } + + public function testDedupeSpecificSettingsOnlyProcessesRequestedSettings(): void + { + $settings = [ + 'synonyms' => [ + ['word' => 'foo'], + ['word' => 'foo'], + ], + 'altCorrections' => [ + ['word' => 'bar'], + ], + 'placeholders' => [ + ['word' => 'baz'], + ], + ]; + + $result = $this->deduplicator->dedupeSpecificSettings( + ['synonyms', 'altCorrections'], + $settings + ); + + $this->assertArrayHasKey('synonyms', $result); + $this->assertCount(1, $result['synonyms']); // deduped + $this->assertArrayHasKey('altCorrections', $result); + $this->assertCount(1, $result['altCorrections']); + $this->assertArrayNotHasKey('placeholders', $result); + } + + public function testDedupeSpecificSettingsHandlesMissingKeys(): void + { + $settings = [ + 'synonyms' => [['w' => 'x']], + ]; + + $result = $this->deduplicator->dedupeSpecificSettings( + ['synonyms', 'altCorrections'], + $settings + ); + + $this->assertArrayHasKey('synonyms', $result); + $this->assertArrayNotHasKey('altCorrections', $result); + } +} From 334f5593ae96ebbc5aa0420f009b7d2a0e82ff8c Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 16:35:30 -0400 Subject: [PATCH 2/7] MAGE-1383 Add data provider for more scenarios --- Test/Unit/Helper/ArrayDeduplicatorTest.php | 120 ++++++++++++++++++--- 1 file changed, 105 insertions(+), 15 deletions(-) diff --git a/Test/Unit/Helper/ArrayDeduplicatorTest.php b/Test/Unit/Helper/ArrayDeduplicatorTest.php index f7ff48a37..2bcf13a58 100644 --- a/Test/Unit/Helper/ArrayDeduplicatorTest.php +++ b/Test/Unit/Helper/ArrayDeduplicatorTest.php @@ -14,19 +14,109 @@ protected function setUp(): void $this->deduplicator = new ArrayDeduplicator(); } - public function testDedupeArrayOfArraysRemovesExactDuplicates(): void + public static function dedupeArrayOfArraysProvider(): array { - $data = [ - ['a' => 1, 'b' => 2], - ['a' => 1, 'b' => 2], // duplicate - ['a' => 2, 'b' => 3], + return [ + 'empty array' => [ + 'input' => [], + 'expectedCount' => 0, + 'expectedItems' => [] + ], + 'no duplicates' => [ + 'input' => [ + ['a' => 1, 'b' => 2], + ['a' => 2, 'b' => 3], + ['a' => 3, 'b' => 4] + ], + 'expectedCount' => 3, + 'expectedItems' => [ + ['a' => 1, 'b' => 2], + ['a' => 2, 'b' => 3], + ['a' => 3, 'b' => 4] + ] + ], + 'exact duplicates' => [ + 'input' => [ + ['a' => 1, 'b' => 2], + ['a' => 1, 'b' => 2], // duplicate + ['a' => 2, 'b' => 3] + ], + 'expectedCount' => 2, + 'expectedItems' => [ + ['a' => 1, 'b' => 2], + ['a' => 2, 'b' => 3] + ] + ], + 'multiple duplicates' => [ + 'input' => [ + ['id' => 1, 'name' => 'test'], + ['id' => 2, 'name' => 'test2'], + ['id' => 1, 'name' => 'test'], // duplicate + ['id' => 3, 'name' => 'test3'], + ['id' => 2, 'name' => 'test2'] // duplicate + ], + 'expectedCount' => 3, + 'expectedItems' => [ + ['id' => 1, 'name' => 'test'], + ['id' => 2, 'name' => 'test2'], + ['id' => 3, 'name' => 'test3'] + ] + ], + 'duplicate synonyms' => [ + 'input' => [ + ['gray', 'grey'], + ['trousers', 'pants'], + ['ipad', 'tablet'], + ['caulk', 'caulking'], + ['trousers', 'pants'], // duplicate + ['molding', 'moldings', 'moulding', 'mouldings'], + ['trash', 'garbage'], + ['molding', 'moldings', 'moulding', 'mouldings'], // duplicate + ], + 'expectedCount' => 6, + 'expectedItems' => [ + ['gray', 'grey'], + ['trousers', 'pants'], + ['ipad', 'tablet'], + ['caulk', 'caulking'], + ['molding', 'moldings', 'moulding', 'mouldings'], + ['trash', 'garbage'], + ] + ], + 'duplicate alt corrections' => [ + 'input' => [ + [ 'word' => 'trousers', 'nbTypos' => 1, 'correction' => 'pants' ], + [ 'word' => 'rod', 'nbTypos' => 1, 'correction' => 'bar' ], + [ 'word' => 'bell', 'nbTypos' => 1, 'correction' => 'buzzer' ], + [ 'word' => 'rod', 'nbTypos' => 1, 'correction' => 'bar' ], // duplicate + [ 'word' => 'blind', 'nbTypos' => 1, 'correction' => 'shade' ], + [ 'word' => 'blind', 'nbTypos' => 2, 'correction' => 'shade' ], // not a duplicate + [ 'word' => 'trousers', 'nbTypos' => 1, 'correction' => 'pants' ], // duplicate + ], + 'expectedCount' => 5, + 'expectedItems' => [ + [ 'word' => 'trousers', 'nbTypos' => 1, 'correction' => 'pants' ], + [ 'word' => 'rod', 'nbTypos' => 1, 'correction' => 'bar' ], + [ 'word' => 'bell', 'nbTypos' => 1, 'correction' => 'buzzer' ], + [ 'word' => 'blind', 'nbTypos' => 1, 'correction' => 'shade' ], + [ 'word' => 'blind', 'nbTypos' => 2, 'correction' => 'shade' ], + ] + ] ]; + } - $result = $this->deduplicator->dedupeArrayOfArrays($data); + /** + * @dataProvider dedupeArrayOfArraysProvider + */ + public function testDedupeArrayOfArrays(array $input, int $expectedCount, array $expectedItems): void + { + $result = $this->deduplicator->dedupeArrayOfArrays($input); + + $this->assertCount($expectedCount, $result); - $this->assertCount(2, $result); - $this->assertContains(['a' => 1, 'b' => 2], $result); - $this->assertContains(['a' => 2, 'b' => 3], $result); + foreach ($expectedItems as $expectedItem) { + $this->assertContains($expectedItem, $result); + } } public function testDedupeArrayOfArraysKeepsOrderOfFirstOccurrences(): void @@ -46,14 +136,14 @@ public function testDedupeSpecificSettingsOnlyProcessesRequestedSettings(): void { $settings = [ 'synonyms' => [ - ['word' => 'foo'], - ['word' => 'foo'], + ['red' => 'rouge'], + ['red' => 'rouge'], ], 'altCorrections' => [ - ['word' => 'bar'], + [ 'word' => 'bell', 'nbTypos' => 1, 'correction' => 'buzzer' ], ], - 'placeholders' => [ - ['word' => 'baz'], + 'placeholder' => [ + ['foo' => 'bar'], ], ]; @@ -72,7 +162,7 @@ public function testDedupeSpecificSettingsOnlyProcessesRequestedSettings(): void public function testDedupeSpecificSettingsHandlesMissingKeys(): void { $settings = [ - 'synonyms' => [['w' => 'x']], + 'synonyms' => [['red', 'rouge']], ]; $result = $this->deduplicator->dedupeSpecificSettings( From 3ea67901bc214346929a56fd16dbc96081394cf3 Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 17:02:04 -0400 Subject: [PATCH 3/7] MAGE-1383 Experiment with legacy callback style to appease Codacy rule --- Helper/ArrayDeduplicator.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php index d51ce0e9d..c49084e57 100644 --- a/Helper/ArrayDeduplicator.php +++ b/Helper/ArrayDeduplicator.php @@ -34,9 +34,16 @@ public function dedupeSpecificSettings(array $settingNames, array $settings): ar public function dedupeArrayOfArrays(array $data): array { $encoded = array_map('json_encode', $data); $unique = array_values(array_unique($encoded)); + // Original code - not passing Codacy + // $decoded = array_map( + // fn($item) => json_decode((string) $item, true), + // $unique + // ); + // Experiment 1 $decoded = array_map( - fn($item) => json_decode((string) $item, true), - $unique + 'json_decode', + $unique, + array_fill(0, count($unique), true) // force decoding as associative array ); return $decoded; } From 4a283d4554471b784ef54029c5f30c87bc0aa9c2 Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 17:08:30 -0400 Subject: [PATCH 4/7] MAGE-1383 Experiment with array_walk alternative --- Helper/ArrayDeduplicator.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php index c49084e57..39503baa6 100644 --- a/Helper/ArrayDeduplicator.php +++ b/Helper/ArrayDeduplicator.php @@ -40,11 +40,18 @@ public function dedupeArrayOfArrays(array $data): array { // $unique // ); // Experiment 1 - $decoded = array_map( - 'json_decode', - $unique, - array_fill(0, count($unique), true) // force decoding as associative array - ); + // $decoded = array_map( + // 'json_decode', + // $unique, + // array_fill(0, count($unique), true) // force decoding as associative array + // ); + + // Experiment 2 + $decoded = []; + array_walk($unique, function($item) use (&$decoded) { + $decoded[] = json_decode((string) $item, true); + }); + return $decoded; } } From 9dff1773c8ed652a48a1233a9d044afbe6cbb07f Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 17:16:26 -0400 Subject: [PATCH 5/7] MAGE-1383 Experiment with imperative for loop --- Helper/ArrayDeduplicator.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php index 39503baa6..72b65f2f2 100644 --- a/Helper/ArrayDeduplicator.php +++ b/Helper/ArrayDeduplicator.php @@ -47,10 +47,16 @@ public function dedupeArrayOfArrays(array $data): array { // ); // Experiment 2 + // $decoded = []; + // array_walk($unique, function($item) use (&$decoded) { + // $decoded[] = json_decode((string) $item, true); + // }); + + // Experiment 3 $decoded = []; - array_walk($unique, function($item) use (&$decoded) { + foreach ($unique as $item) { $decoded[] = json_decode((string) $item, true); - }); + } return $decoded; } From 987d550cfe0ca5ea4568dbf48572486c4fa2de16 Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 17:23:03 -0400 Subject: [PATCH 6/7] MAGE-1383 Experiment by switching all array_map calls to imperative for loops --- Helper/ArrayDeduplicator.php | 42 +++++++++++------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php index 72b65f2f2..63c62636e 100644 --- a/Helper/ArrayDeduplicator.php +++ b/Helper/ArrayDeduplicator.php @@ -11,16 +11,15 @@ class ArrayDeduplicator */ public function dedupeSpecificSettings(array $settingNames, array $settings): array { + $processedSettings = []; + foreach ($settingNames as $settingName) { + $processedSettings[$settingName] = isset($settings[$settingName]) + ? $this->dedupeArrayOfArrays($settings[$settingName]) + : null; + } + return array_filter( - array_combine( - $settingNames, - array_map( - fn($settingName) => isset($settings[$settingName]) - ? $this->dedupeArrayOfArrays($settings[$settingName]) - : null, - $settingNames - ) - ), + $processedSettings, fn($val) => $val !== null ); } @@ -32,27 +31,12 @@ public function dedupeSpecificSettings(array $settingNames, array $settings): ar * @return array */ public function dedupeArrayOfArrays(array $data): array { - $encoded = array_map('json_encode', $data); + $encoded = []; + foreach ($data as $item) { + $encoded[] = json_encode($item); + } $unique = array_values(array_unique($encoded)); - // Original code - not passing Codacy - // $decoded = array_map( - // fn($item) => json_decode((string) $item, true), - // $unique - // ); - // Experiment 1 - // $decoded = array_map( - // 'json_decode', - // $unique, - // array_fill(0, count($unique), true) // force decoding as associative array - // ); - - // Experiment 2 - // $decoded = []; - // array_walk($unique, function($item) use (&$decoded) { - // $decoded[] = json_decode((string) $item, true); - // }); - - // Experiment 3 + $decoded = []; foreach ($unique as $item) { $decoded[] = json_decode((string) $item, true); From edb97ed8a2035c9617cb8242c4a1539e01950f34 Mon Sep 17 00:00:00 2001 From: Eric Wright Date: Tue, 12 Aug 2025 17:28:13 -0400 Subject: [PATCH 7/7] MAGE-1383 Replace array_filter with explicit foreach loop --- Helper/ArrayDeduplicator.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Helper/ArrayDeduplicator.php b/Helper/ArrayDeduplicator.php index 63c62636e..457d34191 100644 --- a/Helper/ArrayDeduplicator.php +++ b/Helper/ArrayDeduplicator.php @@ -18,10 +18,14 @@ public function dedupeSpecificSettings(array $settingNames, array $settings): ar : null; } - return array_filter( - $processedSettings, - fn($val) => $val !== null - ); + $filteredSettings = []; + foreach ($processedSettings as $key => $value) { + if ($value !== null) { + $filteredSettings[$key] = $value; + } + } + + return $filteredSettings; } /**