Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3 - _getCacheInternalId() is too generic #6

Merged
merged 3 commits into from
Jun 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/Adapter/DbSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
),
];
}
froschdesign marked this conversation as resolved.
Show resolved Hide resolved
}
10 changes: 8 additions & 2 deletions src/Paginator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -861,10 +862,15 @@ protected function _getCacheId($page = null)
// @codingStandardsIgnoreStart
protected function _getCacheInternalId()
{
$adapter = $this->getAdapter();
$adapterToSerialize = $adapter instanceof DbSelect
? $adapter->getArrayCopy()
: $adapter;
froschdesign marked this conversation as resolved.
Show resolved Hide resolved

// @codingStandardsIgnoreEnd
return md5(
get_class($this->getAdapter())
. json_encode($this->getAdapter())
get_class($adapter)
. json_encode($adapterToSerialize)
. $this->getItemCountPerPage()
);
}
Expand Down
17 changes: 17 additions & 0 deletions test/Adapter/DbSelectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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())
);
}
}
52 changes: 51 additions & 1 deletion test/PaginatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down
30 changes: 30 additions & 0 deletions test/TestAsset/TestDbSelectAdapter.php
Original file line number Diff line number Diff line change
@@ -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);
}
}