From 0a933015f6fe31f86dcede862bbe458afc6b28a5 Mon Sep 17 00:00:00 2001
From: Owen Leibman <eclipsechasers2@yahoo.com>
Date: Sat, 29 May 2021 23:20:23 -0700
Subject: [PATCH] Improve Coverage for HashTable, Fix Clone

Add unit tests to cover all of HashTable. I was hoping to do this without source changes, but this class does require a deep clone, and, as the new unit tests revealed, the existing code did not fill the bill - it cloned objects, but not arrays which contained objects, and all the object variables in this class are arrays which can contain objects.
---
 src/PhpSpreadsheet/HashTable.php            | 11 ++-
 tests/PhpSpreadsheetTests/HashTableTest.php | 92 +++++++++++++++++++++
 2 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 tests/PhpSpreadsheetTests/HashTableTest.php

diff --git a/src/PhpSpreadsheet/HashTable.php b/src/PhpSpreadsheet/HashTable.php
index 5d4444e795..8cf8281e0f 100644
--- a/src/PhpSpreadsheet/HashTable.php
+++ b/src/PhpSpreadsheet/HashTable.php
@@ -170,8 +170,15 @@ public function __clone()
     {
         $vars = get_object_vars($this);
         foreach ($vars as $key => $value) {
-            if (is_object($value)) {
-                $this->$key = clone $value;
+            // each member of this class is an array
+            if (is_array($value)) {
+                $array1 = $value;
+                foreach ($array1 as $key1 => $value1) {
+                    if (is_object($value1)) {
+                        $array1[$key1] = clone $value1;
+                    }
+                }
+                $this->$key = $array1;
             }
         }
     }
diff --git a/tests/PhpSpreadsheetTests/HashTableTest.php b/tests/PhpSpreadsheetTests/HashTableTest.php
new file mode 100644
index 0000000000..09c2fc9283
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/HashTableTest.php
@@ -0,0 +1,92 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\Comment; // need Comparable object
+use PhpOffice\PhpSpreadsheet\HashTable;
+use PHPUnit\Framework\TestCase;
+
+class HashTableTest extends TestCase
+{
+    public static function createArray(): array
+    {
+        $comment1 = new Comment();
+        $comment1->setAuthor('Author1');
+        $comment2 = new Comment();
+        $comment2->setAuthor('Author2');
+
+        return [$comment1, $comment2];
+    }
+
+    /**
+     * @param mixed $comment
+     */
+    public static function getAuthor($comment): string
+    {
+        return ($comment instanceof Comment) ? $comment->getAuthor() : '';
+    }
+
+    public function testAddRemoveClear(): void
+    {
+        $array1 = self::createArray();
+        $hash1 = new HashTable($array1);
+        self::assertSame(2, $hash1->count());
+        $comment3 = new Comment();
+        $comment3->setAuthor('Author3');
+        $hash1->add($comment3);
+        $comment4 = new Comment();
+        $comment4->setAuthor('Author4');
+        $hash1->add($comment4);
+        $comment5 = new Comment();
+        $comment5->setAuthor('Author5');
+        // don't add comment5
+        self::assertSame(4, $hash1->count());
+        self::assertNull($hash1->getByIndex(10));
+        $comment = $hash1->getByIndex(2);
+        self::assertSame('Author3', self::getAuthor($comment));
+        $hash1->remove($comment3);
+        self::assertSame(3, $hash1->count());
+        $comment = $hash1->getByIndex(2);
+        self::assertSame('Author4', self::getAuthor($comment));
+        $hash1->remove($comment5);
+        self::assertSame(3, $hash1->count(), 'Remove non-hash member');
+        $comment = $hash1->getByIndex(2);
+        self::assertSame('Author4', self::getAuthor($comment));
+        self::assertNull($hash1->getByHashCode('xyz'));
+        $hash1->clear();
+        self::AssertSame(0, $hash1->count());
+    }
+
+    public function testToArray(): void
+    {
+        $array1 = self::createArray();
+        $count1 = count($array1);
+        $hash1 = new HashTable($array1);
+        $array2 = $hash1->toArray();
+        self::assertCount($count1, $array2);
+        $idx = 0;
+        foreach ($array2 as $key => $value) {
+            self::assertEquals($array1[$idx], $value, "Item $idx");
+            self::assertSame($idx, $hash1->getIndexForHashCode($key));
+            ++$idx;
+        }
+    }
+
+    public function testClone(): void
+    {
+        $array1 = self::createArray();
+        $hash1 = new HashTable($array1);
+        $hash2 = new HashTable();
+        self::assertSame(0, $hash2->count());
+        $hash2->addFromSource();
+        self::assertSame(0, $hash2->count());
+        $hash2->addFromSource($array1);
+        self::assertSame(2, $hash2->count());
+        self::assertEquals($hash1, $hash2, 'Add in constructor same as addFromSource');
+        $hash3 = clone $hash1;
+        self::assertEquals($hash1, $hash3, 'Clone equal to original');
+        self::assertSame($hash1->getByIndex(0), $hash2->getByIndex(0));
+        self::assertEquals($hash1->getByIndex(0), $hash3->getByIndex(0));
+        self::assertNotSame($hash1->getByIndex(0), $hash3->getByIndex(0));
+    }
+}