Merge branch 'hotfix/6-cache-internal-id' into develop

Forward port #6 Conflicts: CHANGELOG.md
laminas · Jun 22, 2020 · ce834f0 · ce834f0
2 parents 2fb1d2a + ffc8ffc
commit ce834f0
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,7 +24,7 @@ All notable changes to this project will be documented in this file, in reverse
 
 - Nothing.
 
-## 2.8.3 - TBD
+## 2.8.3 - 2020-06-22
 
 ### Added
 
@@ -44,7 +44,7 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
-- Nothing.
+- [#6](https://github.com/laminas/laminas-paginator/pull/6) fixes an issue that occurs when generating a cache identifier for a `DbSelect` adapter. Previously, it was done such that the cache identifier was always unique or never unique, which would lead to unexpected behavior.
 
 ## 2.8.2 - 2019-08-21
 

diff --git a/src/Adapter/DbSelect.php b/src/Adapter/DbSelect.php
@@ -150,4 +150,19 @@ protected function getSelectCount()
 
         return $countSelect;
     }
+
+    /**
+     * @see https://github.com/laminas/laminas-paginator/issues/3 Reference for creating an internal cache ID
+     * @todo The next major version should rework the entire caching of a paginator.
+     * @internal
+     */
+    public function getArrayCopy()
+    {
+        return [
+            'select'       => $this->sql->buildSqlString($this->select),
+            'count_select' => $this->sql->buildSqlString(
+                $this->getSelectCount()
+            ),
+        ];
+    }
 }
diff --git a/src/Paginator.php b/src/Paginator.php
@@ -17,6 +17,7 @@
 use Laminas\Filter\FilterInterface;
 use Laminas\Json\Json;
 use Laminas\Paginator\Adapter\AdapterInterface;
+use Laminas\Paginator\Adapter\DbSelect;
 use Laminas\Paginator\ScrollingStyle\ScrollingStyleInterface;
 use Laminas\ServiceManager\ServiceManager;
 use Laminas\Stdlib\ArrayUtils;
@@ -861,10 +862,15 @@ protected function _getCacheId($page = null)
     // @codingStandardsIgnoreStart
     protected function _getCacheInternalId()
     {
+        $adapter            = $this->getAdapter();
+        $adapterToSerialize = $adapter instanceof DbSelect
+            ? $adapter->getArrayCopy()
+            : $adapter;
+
         // @codingStandardsIgnoreEnd
         return md5(
-            get_class($this->getAdapter())
-            . json_encode($this->getAdapter())
+            get_class($adapter)
+            . json_encode($adapterToSerialize)
             . $this->getItemCountPerPage()
         );
     }

diff --git a/test/Adapter/DbSelectTest.php b/test/Adapter/DbSelectTest.php
@@ -108,4 +108,21 @@ public function testReturnValueIsArray()
     {
         $this->assertInternalType('array', $this->dbSelect->getItems(0, 10));
     }
+
+    public function testGetArrayCopyShouldContainSelectItems()
+    {
+        $this->dbSelect = new DbSelect(
+            $this->mockSelect,
+            $this->mockSql,
+            null,
+            $this->mockSelectCount
+        );
+        $this->assertSame(
+            [
+                'select',
+                'count_select',
+            ],
+            array_keys($this->dbSelect->getArrayCopy())
+        );
+    }
 }
diff --git a/test/PaginatorTest.php b/test/PaginatorTest.php
@@ -500,7 +500,7 @@ public function testGetsItemsByPageHandleDbSelectAdapter()
             ->will($this->returnValue($mockStatement));
         $mockSelect = $this->createMock('Laminas\Db\Sql\Select');
 
-        $dbSelect = new DbSelect($mockSelect, $mockSql);
+        $dbSelect = new DbSelect($mockSelect, $mockSql, null, $mockSelect);
         $this->assertInstanceOf('ArrayIterator', $resultSet->getDataSource());
 
         $paginator = new Paginator\Paginator($dbSelect);
@@ -934,6 +934,56 @@ public function testGetCacheIdWithInheritedClass()
         $this->assertNotEquals($firstOutputGetCacheInternalId, $secondOutputGetCacheInternalId);
     }
 
+    public function testDbSelectAdapterShouldProduceValidCacheId()
+    {
+        // Create first interal cache ID
+        $paginator                    = new Paginator\Paginator(
+            new TestAsset\TestDbSelectAdapter(
+                (new Sql\Select('table1'))
+                    ->where('id = 1')
+                    ->where("foo = 'bar'"),
+                new DbAdapter\Adapter(
+                    new DbAdapter\Driver\Pdo\Pdo(
+                        new DbAdapter\Driver\Pdo\Connection([])
+                    )
+                )
+            )
+        );
+        $reflectionGetCacheInternalId = new ReflectionMethod(
+            $paginator,
+            '_getCacheInternalId'
+        );
+        $reflectionGetCacheInternalId->setAccessible(true);
+        $firstCacheId = $reflectionGetCacheInternalId->invoke(
+            $paginator
+        );
+
+        // Create second internal cache ID
+        $paginator                    = new Paginator\Paginator(
+            new TestAsset\TestDbSelectAdapter(
+                (new Sql\Select('table2'))
+                    ->where('id = 2')
+                    ->where("foo = 'bar'"),
+                new DbAdapter\Adapter(
+                    new DbAdapter\Driver\Pdo\Pdo(
+                        new DbAdapter\Driver\Pdo\Connection([])
+                    )
+                )
+            )
+        );
+        $reflectionGetCacheInternalId = new ReflectionMethod(
+            $paginator,
+            '_getCacheInternalId'
+        );
+        $reflectionGetCacheInternalId->setAccessible(true);
+        $secondCacheIde = $reflectionGetCacheInternalId->invoke(
+            $paginator
+        );
+
+        // Test
+        $this->assertNotEquals($firstCacheId, $secondCacheIde);
+    }
+
     public function testAcceptsComplexAdapters()
     {
         $paginator = new Paginator\Paginator(

diff --git a/test/TestAsset/TestDbSelectAdapter.php b/test/TestAsset/TestDbSelectAdapter.php
@@ -0,0 +1,30 @@
+<?php
+
+/**
+ * @see       https://github.com/laminas/laminas-paginator for the canonical source repository
+ * @copyright https://github.com/laminas/laminas-paginator/blob/master/COPYRIGHT.md
+ * @license   https://github.com/laminas/laminas-paginator/blob/master/LICENSE.md New BSD License
+ */
+
+namespace LaminasTest\Paginator\TestAsset;
+
+use Laminas\Paginator\Adapter\DbSelect;
+
+class TestDbSelectAdapter extends DbSelect
+{
+    /**
+     * @inheritDoc
+     */
+    public function count()
+    {
+        return 10;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function getItems($pageNumber, $itemCountPerPage)
+    {
+        return range(1, 10);
+    }
+}