Skip to content

Commit 98d99a8

Browse files
author
Dale Sikkema
committed
MAGETWO-37848: Generate data fails when table prefix exceeds length of 5
 - refactor trigger name logic
1 parent 0a72226 commit 98d99a8

File tree

8 files changed

+103
-114
lines changed

8 files changed

+103
-114
lines changed

‎lib/internal/Magento/Framework/App/Resource.php

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public function __construct(
8585
*
8686
* @param string $resourceName
8787
* @return \Magento\Framework\DB\Adapter\AdapterInterface|false
88+
* @codeCoverageIgnore
8889
*/
8990
public function getConnection($resourceName)
9091
{
@@ -152,12 +153,27 @@ public function getTableName($modelEntity, $connectionName = self::DEFAULT_READ_
152153
return $this->getConnection($connectionName)->getTableName($tableName);
153154
}
154155

156+
/**
157+
* Build a trigger name
158+
*
159+
* @param string $tableName The table that is the subject of the trigger
160+
* @param string $time Either "before" or "after"
161+
* @param string $event The DB level event which activates the trigger, i.e. "update" or "insert"
162+
* @return string
163+
* @codeCoverageIgnore
164+
*/
165+
public function getTriggerName($tableName, $time, $event)
166+
{
167+
return $this->getConnectionByName(self::DEFAULT_READ_RESOURCE)->getTriggerName($tableName, $time, $event);
168+
}
169+
155170
/**
156171
* Set mapped table name
157172
*
158173
* @param string $tableName
159174
* @param string $mappedName
160175
* @return $this
176+
* @codeCoverageIgnore
161177
*/
162178
public function setMappedTableName($tableName, $mappedName)
163179
{
@@ -193,13 +209,12 @@ public function getIdxName(
193209
$fields,
194210
$indexType = \Magento\Framework\DB\Adapter\AdapterInterface::INDEX_TYPE_INDEX
195211
) {
196-
return $this->getConnection(
197-
self::DEFAULT_READ_RESOURCE
198-
)->getIndexName(
199-
$this->getTableName($tableName),
200-
$fields,
201-
$indexType
202-
);
212+
return $this->getConnection(self::DEFAULT_READ_RESOURCE)
213+
->getIndexName(
214+
$this->getTableName($tableName),
215+
$fields,
216+
$indexType
217+
);
203218
}
204219

205220
/**

‎lib/internal/Magento/Framework/DB/Adapter/AdapterInterface.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,17 @@ public function getDateExtractSql($date, $unit);
918918
*/
919919
public function getTableName($tableName);
920920

921+
922+
/**
923+
* Build a trigger name based on table name and trigger details
924+
*
925+
* @param string $tableName The table that is the subject of the trigger
926+
* @param string $time Either "before" or "after"
927+
* @param string $event The DB level event which activates the trigger, i.e. "update" or "insert"
928+
* @return string
929+
*/
930+
public function getTriggerName($tableName, $time, $event);
931+
921932
/**
922933
* Retrieve valid index name
923934
* Check index name length and allowed symbols

‎lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3131,15 +3131,19 @@ public function getTableName($tableName)
31313131
}
31323132

31333133
/**
3134-
* Returns a compressed version of the trigger name if it is too long
3134+
* Build a trigger name based on table name and trigger details
31353135
*
3136-
* @param string $triggerName
3136+
* @param string $tableName The table which is the subject of the trigger
3137+
* @param string $time Either "before" or "after"
3138+
* @param string $event The DB level event which activates the trigger, i.e. "update" or "insert"
31373139
* @return string
31383140
* @codeCoverageIgnore Covered by tests on ExpressionConverter
31393141
*/
3140-
public function getTriggerName($triggerName)
3142+
3143+
public function getTriggerName($tableName, $time, $event)
31413144
{
3142-
return ExpressionConverter::shortenEntityName($triggerName, Subscription::TRIGGER_NAME_QUALIFIER);
3145+
$triggerName = 'trg_' . $tableName . '_' . $time . '_' . $event;
3146+
return ExpressionConverter::shortenEntityName($triggerName, 'trg_');
31433147
}
31443148

31453149
/**

‎lib/internal/Magento/Framework/DB/ExpressionConverter.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public static function shortenEntityName($tableName, $prefix)
101101
if (strlen($shortName) > self::MYSQL_IDENTIFIER_LEN) {
102102
$hash = md5($tableName);
103103
if (strlen($prefix . $hash) > self::MYSQL_IDENTIFIER_LEN) {
104-
$tableName = self::_minusSuperfluous($hash, $prefix, self::MYSQL_IDENTIFIER_LEN);
104+
$tableName = self::trimHash($hash, $prefix, self::MYSQL_IDENTIFIER_LEN);
105105
} else {
106106
$tableName = $prefix . $hash;
107107
}
@@ -113,14 +113,14 @@ public static function shortenEntityName($tableName, $prefix)
113113
}
114114

115115
/**
116-
* Minus superfluous characters from hash.
116+
* Remove superfluous characters from hash.
117117
*
118118
* @param string $hash
119119
* @param string $prefix
120120
* @param int $maxCharacters
121121
* @return string
122122
*/
123-
protected static function _minusSuperfluous($hash, $prefix, $maxCharacters)
123+
private static function trimHash($hash, $prefix, $maxCharacters)
124124
{
125125
$diff = strlen($hash) + strlen($prefix) - $maxCharacters;
126126
$superfluous = $diff / 2;

‎lib/internal/Magento/Framework/DB/Test/Unit/Adapter/Pdo/MysqlTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,9 +557,10 @@ public function addColumnDataProvider()
557557
*/
558558
public function testGetIndexName($name, $fields, $indexType, $expectedName)
559559
{
560+
$resultIndexName = $this->_mockAdapter->getIndexName($name, $fields, $indexType);
560561
$this->assertTrue(
561-
strpos($this->_mockAdapter->getIndexName($name, $fields, $indexType), $expectedName) === 0,
562-
'Index name did not start with expected prefix'
562+
strpos($resultIndexName, $expectedName) === 0,
563+
"Index name '$resultIndexName' did not begin with expected value '$expectedName'"
563564
);
564565
}
565566

‎lib/internal/Magento/Framework/DB/Test/Unit/ExpressionConverterTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ class ExpressionConverterTest extends \PHPUnit_Framework_TestCase
1616
*/
1717
public function testShortenEntityName($in, $prefix, $expectedOut)
1818
{
19+
$resultEntityName = ExpressionConverter::shortenEntityName($in, $prefix);
1920
$this->assertTrue(
20-
strpos(ExpressionConverter::shortenEntityName($in, $prefix), $expectedOut) === 0,
21-
'Entity name did not start with expected prefix'
21+
strpos($resultEntityName, $expectedOut) === 0,
22+
"Entity name '$resultEntityName' did not begin with expected value '$expectedOut'"
2223
);
2324
}
2425

‎lib/internal/Magento/Framework/Mview/Test/Unit/View/SubscriptionTest.php

Lines changed: 34 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -12,46 +12,44 @@
1212

1313
class SubscriptionTest extends \PHPUnit_Framework_TestCase
1414
{
15-
/**
16-
* @var \Magento\Framework\Mview\View\Subscription
17-
*/
18-
protected $model;
19-
2015
/**
2116
* Mysql PDO DB adapter mock
2217
*
2318
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Adapter\Pdo\Mysql
2419
*/
2520
protected $connectionMock;
2621

27-
/**
28-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\Resource
29-
*/
22+
/** @var \Magento\Framework\Mview\View\Subscription */
23+
protected $model;
24+
25+
/** @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\Resource */
3026
protected $resourceMock;
3127

32-
/**
33-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Ddl\TriggerFactory
34-
*/
28+
/** @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Ddl\TriggerFactory */
3529
protected $triggerFactoryMock;
3630

37-
/**
38-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Mview\View\CollectionInterface
39-
*/
31+
/** @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Mview\View\CollectionInterface */
4032
protected $viewCollectionMock;
4133

42-
/**
43-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Mview\ViewInterface
44-
*/
34+
/** @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Mview\ViewInterface */
4535
protected $viewMock;
4636

37+
/** @var string */
38+
private $tableName;
39+
4740
protected function setUp()
4841
{
4942
$this->connectionMock = $this->getMock('Magento\Framework\DB\Adapter\Pdo\Mysql', [], [], '', false);
43+
$this->resourceMock = $this->getMock('Magento\Framework\App\Resource', [], [], '', false, false);
44+
45+
$this->connectionMock->expects($this->any())
46+
->method('quoteIdentifier')
47+
->will($this->returnArgument(0));
48+
49+
$this->resourceMock->expects($this->atLeastOnce())
50+
->method('getConnection')
51+
->willReturn($this->connectionMock);
5052

51-
$this->resourceMock = $this->getMock(
52-
'Magento\Framework\App\Resource', ['getConnection', 'getTableName'], [], '', false, false
53-
);
54-
$this->mockGetConnection($this->connectionMock);
5553
$this->triggerFactoryMock = $this->getMock(
5654
'Magento\Framework\DB\Ddl\TriggerFactory', [], [], '', false, false
5755
);
@@ -62,16 +60,16 @@ protected function setUp()
6260
'Magento\Framework\Mview\ViewInterface', [], '', false, false, true, []
6361
);
6462

65-
$this->connectionMock->expects($this->any())
66-
->method('quoteIdentifier')
67-
->will($this->returnArgument(0));
63+
$this->resourceMock->expects($this->any())
64+
->method('getTableName')
65+
->willReturn($this->tableName);
6866

6967
$this->model = new Subscription(
7068
$this->resourceMock,
7169
$this->triggerFactoryMock,
7270
$this->viewCollectionMock,
7371
$this->viewMock,
74-
'tableName',
72+
$this->tableName,
7573
'columnName'
7674
);
7775
}
@@ -83,7 +81,7 @@ public function testGetView()
8381

8482
public function testGetTableName()
8583
{
86-
$this->assertEquals('tableName', $this->model->getTableName());
84+
$this->assertEquals($this->tableName, $this->model->getTableName());
8785
}
8886

8987
public function testGetColumnName()
@@ -93,13 +91,14 @@ public function testGetColumnName()
9391

9492
public function testCreate()
9593
{
96-
$this->mockGetTableName();
97-
$shortTriggerName = 'short_trigger_name';
98-
$this->connectionMock->expects($this->atLeastOnce())->method('getTriggerName')->willReturn($shortTriggerName);
99-
$triggerMock = $this->getMock('Magento\Framework\DB\Ddl\Trigger', [], [], '', false, false);
94+
$triggerName = 'trigger_name';
95+
$this->resourceMock->expects($this->atLeastOnce())->method('getTriggerName')->willReturn($triggerName);
96+
$triggerMock = $this->getMockBuilder('Magento\Framework\DB\Ddl\Trigger')
97+
->disableOriginalConstructor()
98+
->getMock();
10099
$triggerMock->expects($this->exactly(3))
101100
->method('setName')
102-
->with($shortTriggerName)
101+
->with($triggerName)
103102
->will($this->returnSelf());
104103
$triggerMock->expects($this->exactly(3))
105104
->method('getName')
@@ -113,7 +112,7 @@ public function testCreate()
113112
->will($this->returnSelf());
114113
$triggerMock->expects($this->exactly(3))
115114
->method('setTable')
116-
->with('tableName')
115+
->with($this->tableName)
117116
->will($this->returnSelf());
118117
$triggerMock->expects($this->exactly(6))
119118
->method('addStatement')
@@ -155,7 +154,7 @@ public function testCreate()
155154
->will($this->returnValue('other_id'));
156155
$otherViewMock->expects($this->exactly(1))
157156
->method('getSubscriptions')
158-
->will($this->returnValue([['name' => 'tableName'], ['name' => 'otherTableName']]));
157+
->will($this->returnValue([['name' => $this->tableName], ['name' => 'otherTableName']]));
159158
$otherViewMock->expects($this->exactly(3))
160159
->method('getChangelog')
161160
->will($this->returnValue($otherChangelogMock));
@@ -184,8 +183,6 @@ public function testCreate()
184183

185184
public function testRemove()
186185
{
187-
$this->mockGetTableName();
188-
189186
$triggerMock = $this->getMock('Magento\Framework\DB\Ddl\Trigger', [], [], '', false, false);
190187
$triggerMock->expects($this->exactly(3))
191188
->method('setName')
@@ -202,7 +199,7 @@ public function testRemove()
202199
->will($this->returnSelf());
203200
$triggerMock->expects($this->exactly(3))
204201
->method('setTable')
205-
->with('tableName')
202+
->with($this->tableName)
206203
->will($this->returnSelf());
207204
$triggerMock->expects($this->exactly(3))
208205
->method('addStatement')
@@ -230,7 +227,7 @@ public function testRemove()
230227
->will($this->returnValue('other_id'));
231228
$otherViewMock->expects($this->exactly(1))
232229
->method('getSubscriptions')
233-
->will($this->returnValue([['name' => 'tableName'], ['name' => 'otherTableName']]));
230+
->will($this->returnValue([['name' => $this->tableName], ['name' => 'otherTableName']]));
234231
$otherViewMock->expects($this->exactly(3))
235232
->method('getChangelog')
236233
->will($this->returnValue($otherChangelogMock));
@@ -261,25 +258,4 @@ public function testRemove()
261258

262259
$this->model->remove();
263260
}
264-
265-
/**
266-
* @param $connection
267-
*/
268-
protected function mockGetConnection($connection)
269-
{
270-
$this->resourceMock->expects($this->atLeastOnce())
271-
->method('getConnection')
272-
->will($this->returnValue($connection));
273-
}
274-
275-
protected function mockGetTableName()
276-
{
277-
$this->resourceMock->expects($this->any())
278-
->method('getTableName')
279-
->will($this->returnArgument(0));
280-
}
281-
282-
public function testGetTriggerName()
283-
{
284-
}
285261
}

0 commit comments

Comments
 (0)