Skip to content

Mview Indexers getList should return integer values of id's and not strings #22920

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

Merged
merged 13 commits into from
May 31, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

namespace Magento\Framework\Mview\Test\Unit\View;

/**
* Test Coverage for Changelog View.
*
* @see \Magento\Framework\Mview\View\Changelog
*/
class ChangelogTest extends \PHPUnit\Framework\TestCase
{
/**
Expand Down Expand Up @@ -45,7 +50,7 @@ public function testInstanceOf()
}

/**
* @expectedException \Exception
* @expectedException \Magento\Framework\DB\Adapter\ConnectionException
* @expectedExceptionMessage The write connection to the database isn't available. Please try again later.
*/
public function testCheckConnectionException()
Expand Down Expand Up @@ -74,7 +79,7 @@ public function testGetViewId()
}

/**
* @expectedException \Exception
* @expectedException \DomainException
* @expectedExceptionMessage View's identifier is not set
*/
public function testGetNameWithException()
Expand All @@ -101,6 +106,10 @@ public function testGetVersion()
$this->assertEquals(10, $this->model->getVersion());
}

/**
* @expectedException \Magento\Framework\Exception\RuntimeException
* @expectedExceptionMessage Table status for viewIdtest_cl is incorrect. Can`t fetch version id.
*/
public function testGetVersionWithExceptionNoAutoincrement()
{
$changelogTableName = 'viewIdtest_cl';
Expand All @@ -111,8 +120,6 @@ public function testGetVersionWithExceptionNoAutoincrement()
->method('fetchRow')
->will($this->returnValue([]));

$this->expectException('Exception');
$this->expectExceptionMessage("Table status for `{$changelogTableName}` is incorrect. Can`t fetch version id.");
$this->model->setViewId('viewIdtest');
$this->model->getVersion();
}
Expand Down Expand Up @@ -215,10 +222,10 @@ public function testGetList()
$this->connectionMock->expects($this->once())
->method('fetchCol')
->with($selectMock)
->will($this->returnValue(['some_data']));
->will($this->returnValue([1]));

$this->model->setViewId('viewIdtest');
$this->assertEquals(['some_data'], $this->model->getList(1, 2));
$this->assertEquals([1], $this->model->getList(1, 2));
}

public function testGetListWithException()
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/Magento/Framework/Mview/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public function update()
for ($vsFrom = $lastVersionId; $vsFrom < $currentVersionId; $vsFrom += $versionBatchSize) {
// Don't go past the current version for atomicy.
$versionTo = min($currentVersionId, $vsFrom + $versionBatchSize);
$ids = array_map('intval', $this->getChangelog()->getList($vsFrom, $versionTo));
$ids = $this->getChangelog()->getList($vsFrom, $versionTo);

// We run the actual indexer in batches.
// Chunked AFTER loading to avoid duplicates in separate chunks.
Expand Down
29 changes: 22 additions & 7 deletions lib/internal/Magento/Framework/Mview/View/Changelog.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@

namespace Magento\Framework\Mview\View;

use Magento\Framework\DB\Adapter\ConnectionException;
use Magento\Framework\Exception\RuntimeException;
use Magento\Framework\Phrase;

/**
* Class Changelog for manipulations with the mview_state table.
*/
class Changelog implements ChangelogInterface
{
/**
Expand Down Expand Up @@ -41,6 +46,7 @@ class Changelog implements ChangelogInterface

/**
* @param \Magento\Framework\App\ResourceConnection $resource
* @throws ConnectionException
*/
public function __construct(\Magento\Framework\App\ResourceConnection $resource)
{
Expand All @@ -53,12 +59,14 @@ public function __construct(\Magento\Framework\App\ResourceConnection $resource)
* Check DB connection
*
* @return void
* @throws \Exception
* @throws ConnectionException
*/
protected function checkConnection()
{
if (!$this->connection) {
throw new \Exception("The write connection to the database isn't available. Please try again later.");
throw new ConnectionException(
new Phrase("The write connection to the database isn't available. Please try again later.")
);
}
}

Expand Down Expand Up @@ -154,14 +162,15 @@ public function getList($fromVersionId, $toVersionId)
(int)$toVersionId
);

return $this->connection->fetchCol($select);
return array_map('intval', $this->connection->fetchCol($select));
}

/**
* Get maximum version_id from changelog
*
* @return int
* @throws ChangelogTableNotExistsException
* @throws \Exception
* @throws RuntimeException
*/
public function getVersion()
{
Expand All @@ -173,7 +182,9 @@ public function getVersion()
if (isset($row['Auto_increment'])) {
return (int)$row['Auto_increment'] - 1;
} else {
throw new \Exception("Table status for `{$changelogTableName}` is incorrect. Can`t fetch version id.");
throw new RuntimeException(
new Phrase("Table status for %1 is incorrect. Can`t fetch version id.", [$changelogTableName])
);
}
}

Expand All @@ -182,13 +193,15 @@ public function getVersion()
*
* Build a changelog name by concatenating view identifier and changelog name suffix.
*
* @throws \Exception
* @throws \DomainException
* @return string
*/
public function getName()
{
if (strlen($this->viewId) == 0) {
throw new \Exception("View's identifier is not set");
throw new \DomainException(
new Phrase("View's identifier is not set")
);
}
return $this->viewId . '_' . self::NAME_SUFFIX;
}
Expand Down Expand Up @@ -216,6 +229,8 @@ public function setViewId($viewId)
}

/**
* Get view's identifier
*
* @return string
*/
public function getViewId()
Expand Down