Fix bug where some original indexes do not have a mapping to new indexes.

Author: JetTheCat

src/Actions/SelectJsonDiffsForOriginalKeysWithMinimalChangesAction.php

@@ -19,11 +19,14 @@ public function execute(Collection $diffs): Collection
         $originalToNewIndexMapping = collect();
 
         do {
-            $addedMapping = false;
+            $modifiedMapping = false;
 
-            $diffs->each(function (Collection $diffMappings) use (&$originalToNewIndexMapping, &$keysProcessedInNewArray, &$addedMapping): void {
-                $diffMappings->each(function (DiffMapping $diffMapping) use (&$originalToNewIndexMapping, &$keysProcessedInNewArray, &$addedMapping): void {
-                    // If we find a diff mapping with lesser changes than an existing mapping, replace it
+            $diffs->each(function (Collection $diffMappings) use (&$originalToNewIndexMapping, &$keysProcessedInNewArray, &$modifiedMapping): void {
+                $diffMappings->each(function (DiffMapping $diffMapping) use (&$originalToNewIndexMapping, &$keysProcessedInNewArray, &$modifiedMapping): void {
+                    /*
+                     * If this diff mapping's new index has already been mapped, and it has fewer changes
+                     * skip this diff mapping.
+                     */
                     if (
                         $keysProcessedInNewArray->has($diffMapping->getNewIndex())
                         && $diffMapping
@@ -37,8 +40,10 @@ public function execute(Collection $diffs): Collection
                         return;
                     }
 
-                    // If we have an existing diff mapping to the original index with lesser changes than the current
-                    // diff mapping, skip replacing the mapping with the current index
+                    /*
+                     * If this diff mapping's original index has already been mapped, and it has fewer changes
+                     * skip this diff mapping.
+                     */
                     if (
                         $originalToNewIndexMapping->has($diffMapping->getOriginalIndex())
                         && $diffMapping
@@ -52,25 +57,38 @@ public function execute(Collection $diffs): Collection
                         return;
                     }
 
+                    // Changes will happen so we need to keep looping
+                    $modifiedMapping = true;
+
                     // Add the mapping
                     $keysProcessedInNewArray
                         ->offsetSet(
                             $diffMapping->getNewIndex(),
                             $diffMapping
                         );
 
-                    // Add an original index to new index mapping to track if an original index has been mapped to a new
-                    // index already
+                    /*
+                     * Add an original index to new index mapping to track if an original index has been mapped to a new
+                     * index already
+                     */
                     $originalToNewIndexMapping
                         ->offsetSet(
                             $diffMapping->getOriginalIndex(),
                             $diffMapping->getNewIndex()
                         );
 
-                    $addedMapping = true;
+                    // Unmap any other original indexes that may have been mapped to this diff mapping's new index
+                    $originalToNewIndexMapping
+                        ->filter(function ($newIndex, $originalIndex) use ($diffMapping) {
+                            return $originalIndex !== $diffMapping->getOriginalIndex() && $newIndex === $diffMapping->getNewIndex();
+                        })
+                        ->keys()
+                        ->each(function ($originalIndex) use (&$originalToNewIndexMapping): void {
+                            $originalToNewIndexMapping->offsetUnset($originalIndex);
+                        });
                 });
             });
-        } while ($addedMapping);
+        } while ($modifiedMapping);
 
         // For each original index, return the found diff mapping
         return $keysProcessedInNewArray->mapWithKeys(function (DiffMapping $diffMapping) {

tests/Unit/Actions/SelectJsonDiffsForOriginalKeysWithMinimalChangesTest.php

@@ -0,0 +1,78 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Jet\Tests\Unit\Actions;
+
+use Carbon\Carbon;
+use Illuminate\Container\Container;
+use Jet\JsonDiff\Actions\CalculateAllDifferencesRespectiveToOriginalAction;
+use Jet\JsonDiff\Actions\SelectJsonDiffsForOriginalKeysWithMinimalChangesAction;
+use Jet\JsonDiff\Actions\SortDifferencesByNumberOfChangesAction;
+use Jet\Tests\TestCase;
+
+class SelectJsonDiffsForOriginalKeysWithMinimalChangesTest extends TestCase
+{
+    /**
+     * Test the action will create a mapping for each original array element to an element in the new array if the
+     * new array has greater or equal number of array elements to the original array.
+     *
+     * Note:
+     * The new array element mapped to the original array element will always have the least amount of changes in
+     * respect to the original element.
+     */
+    public function test_each_original_keys_have_a_mapping_to_a_new_key_if_new_array_has_greater_or_equal_elements_to_the_original_array(): void
+    {
+        $originalArray = [
+            [
+                'label' => 'post1',
+                'description' => null,
+                'meta' => [
+                    'created_at' => Carbon::now(),
+                ],
+            ],
+            [
+                'label' => 'post2',
+                'description' => 'description for test post 2',
+                'meta' => null,
+            ],
+        ];
+
+        $newArray = [
+            [
+                'label' => 'post1',
+                'description' => 'description for test post 1',
+                'meta' => [
+                    'created_at' => Carbon::now(),
+                    'comment_count' => 25,
+                ],
+            ],
+            [
+                'label' => 'post2',
+                'description' => 'description for test post 2',
+                'meta' => null,
+            ],
+            [
+                'label' => 'post3',
+                'description' => 'description for test post 3',
+                'meta' => null,
+            ],
+        ];
+
+        $diffMappings = $this->container(CalculateAllDifferencesRespectiveToOriginalAction::class)->execute($originalArray, $newArray, '');
+        $diffMappings = $this->container(SortDifferencesByNumberOfChangesAction::class)->execute($diffMappings);
+        $diffMappings = $this->container(SelectJsonDiffsForOriginalKeysWithMinimalChangesAction::class)->execute($diffMappings);
+
+        // Assert that both original array elements have a mapping to the new array elements
+        $this->assertSame(2, $diffMappings->count());
+    }
+
+    private function container($abstract = null, array $parameters = [])
+    {
+        if (null === $abstract) {
+            return Container::getInstance();
+        }
+
+        return Container::getInstance()->make($abstract, $parameters);
+    }
+}