Skip to content

Commit 918c89b

Browse files
slavvkaslavvka
authored
ENGCOM-7136: Handling exception on individual callback #27134 
2 parents 668ad8a + 4855a09 commit 918c89b

File tree

2 files changed

+123
-46
lines changed

2 files changed

+123
-46
lines changed

‎lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
/**
1414
* Abstract resource model
1515
*
16+
* phpcs:disable Magento2.Classes.AbstractApi
1617
* @api
1718
*/
1819
abstract class AbstractResource
@@ -81,6 +82,7 @@ public function addCommitCallback($callback)
8182
/**
8283
* Commit resource transaction
8384
*
85+
* @deprecated see \Magento\Framework\Model\ExecuteCommitCallbacks::afterCommit
8486
* @return $this
8587
* @api
8688
*/
@@ -92,14 +94,15 @@ public function commit()
9294
*/
9395
if ($this->getConnection()->getTransactionLevel() === 0) {
9496
$callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
95-
try {
96-
foreach ($callbacks as $callback) {
97+
foreach ($callbacks as $callback) {
98+
try {
9799
call_user_func($callback);
100+
} catch (\Exception $e) {
101+
$this->getLogger()->critical($e);
98102
}
99-
} catch (\Exception $e) {
100-
$this->getLogger()->critical($e);
101103
}
102104
}
105+
103106
return $this;
104107
}
105108

‎lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceTest.php

Lines changed: 116 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
use PHPUnit\Framework\TestCase;
1616
use Psr\Log\LoggerInterface;
1717

18+
/**
19+
* Test for \Magento\Framework\Model\ResourceModel\AbstractResource.
20+
*/
1821
class AbstractResourceTest extends TestCase
1922
{
2023
/**
2124
* @var AbstractResourceStub
2225
*/
23-
private $abstractResource;
26+
private $model;
2427

2528
/**
2629
* @var Json|MockObject
@@ -32,43 +35,51 @@ class AbstractResourceTest extends TestCase
3235
*/
3336
private $loggerMock;
3437

38+
/**
39+
* @inheritdoc
40+
*/
3541
protected function setUp(): void
3642
{
3743
$objectManager = new ObjectManager($this);
3844
$this->serializerMock = $this->createMock(Json::class);
3945
$this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class);
40-
$this->abstractResource = $objectManager->getObject(AbstractResourceStub::class);
41-
$objectManager->setBackwardCompatibleProperty($this->abstractResource, 'serializer', $this->serializerMock);
42-
$objectManager->setBackwardCompatibleProperty($this->abstractResource, '_logger', $this->loggerMock);
46+
$this->model = $objectManager->getObject(AbstractResourceStub::class);
47+
$objectManager->setBackwardCompatibleProperty($this->model, 'serializer', $this->serializerMock);
48+
$objectManager->setBackwardCompatibleProperty($this->model, '_logger', $this->loggerMock);
4349
}
4450

4551
/**
52+
* Test fields serialize
53+
*
4654
* @param array $arguments
47-
* @param string $expected
55+
* @param string|null $expected
4856
* @param array|string|int $serializeCalledWith
4957
* @param int $numSerializeCalled
58+
* @return void
5059
* @dataProvider serializeFieldsDataProvider
5160
*/
5261
public function testSerializeFields(
5362
array $arguments,
54-
$expected,
63+
?string $expected,
5564
$serializeCalledWith,
56-
$numSerializeCalled = 1
57-
) {
65+
int $numSerializeCalled = 1
66+
): void {
5867
/** @var DataObject $dataObject */
59-
list($dataObject, $field, $defaultValue, $unsetEmpty) = $arguments;
68+
[$dataObject, $field, $defaultValue, $unsetEmpty] = $arguments;
6069
$this->serializerMock->expects($this->exactly($numSerializeCalled))
6170
->method('serialize')
6271
->with($serializeCalledWith)
6372
->willReturn($expected);
64-
$this->abstractResource->_serializeField($dataObject, $field, $defaultValue, $unsetEmpty);
73+
$this->model->_serializeField($dataObject, $field, $defaultValue, $unsetEmpty);
6574
$this->assertEquals($expected, $dataObject->getData($field));
6675
}
6776

6877
/**
78+
* DataProvider for testSerializeFields()
79+
*
6980
* @return array
7081
*/
71-
public function serializeFieldsDataProvider()
82+
public function serializeFieldsDataProvider(): array
7283
{
7384
$array = ['a', 'b', 'c'];
7485
$string = 'i am string';
@@ -80,60 +91,66 @@ public function serializeFieldsDataProvider()
8091
'string' => $string,
8192
'integer' => $integer,
8293
'empty' => $empty,
83-
'empty_with_default' => ''
94+
'empty_with_default' => '',
8495
]
8596
);
97+
8698
return [
8799
[
88100
[$dataObject, 'array', null, false],
89101
'["a","b","c"]',
90-
$array
102+
$array,
91103
],
92104
[
93105
[$dataObject, 'string', null, false],
94106
'"i am string"',
95-
$string
107+
$string,
96108
],
97109
[
98110
[$dataObject, 'integer', null, false],
99111
'969',
100-
$integer
112+
$integer,
101113
],
102114
[
103115
[$dataObject, 'empty', null, true],
104116
null,
105117
$empty,
106-
0
118+
0,
107119
],
108120
[
109121
[$dataObject, 'empty_with_default', 'default', false],
110122
'"default"',
111-
'default'
112-
]
123+
'default',
124+
],
113125
];
114126
}
115127

116128
/**
129+
* Test fields unserialize
130+
*
117131
* @param array $arguments
118132
* @param array|string|int|boolean $expected
133+
* @return void
119134
* @dataProvider unserializeFieldsDataProvider
120135
*/
121-
public function testUnserializeFields(array $arguments, $expected)
136+
public function testUnserializeFields(array $arguments, $expected): void
122137
{
123138
/** @var DataObject $dataObject */
124-
list($dataObject, $field, $defaultValue) = $arguments;
139+
[$dataObject, $field, $defaultValue] = $arguments;
125140
$this->serializerMock->expects($this->once())
126141
->method('unserialize')
127142
->with($dataObject->getData($field))
128143
->willReturn($expected);
129-
$this->abstractResource->_unserializeField($dataObject, $field, $defaultValue);
144+
$this->model->_unserializeField($dataObject, $field, $defaultValue);
130145
$this->assertEquals($expected, $dataObject->getData($field));
131146
}
132147

133148
/**
149+
* DataProvider for testUnserializeFields()
150+
*
134151
* @return array
135152
*/
136-
public function unserializeFieldsDataProvider()
153+
public function unserializeFieldsDataProvider(): array
137154
{
138155
$dataObject = new DataObject(
139156
[
@@ -142,38 +159,44 @@ public function unserializeFieldsDataProvider()
142159
'integer' => '969',
143160
'empty_with_default' => '""',
144161
'not_serialized_string' => 'i am string',
145-
'serialized_boolean_false' => 'false'
162+
'serialized_boolean_false' => 'false',
146163
]
147164
);
165+
148166
return [
149167
[
150168
[$dataObject, 'array', null],
151-
['a', 'b', 'c']
169+
['a', 'b', 'c'],
152170
],
153171
[
154172
[$dataObject, 'string', null],
155-
'i am string'
173+
'i am string',
156174
],
157175
[
158176
[$dataObject, 'integer', null],
159-
969
177+
969,
160178
],
161179
[
162180
[$dataObject, 'empty_with_default', 'default', false],
163-
'default'
181+
'default',
164182
],
165183
[
166184
[$dataObject, 'not_serialized_string', null],
167-
'i am string'
185+
'i am string',
168186
],
169187
[
170188
[$dataObject, 'serialized_boolean_false', null],
171189
false,
172-
]
190+
],
173191
];
174192
}
175193

176-
public function testCommitZeroLevel()
194+
/**
195+
* Commit zero level
196+
*
197+
* @return void
198+
*/
199+
public function testCommitZeroLevel(): void
177200
{
178201
/** @var AdapterInterface|MockObject $connection */
179202
$connection = $this->getMockForAbstractClass(AdapterInterface::class);
@@ -182,14 +205,14 @@ public function testCommitZeroLevel()
182205
->disableOriginalConstructor()
183206
->getMock();
184207

185-
$this->abstractResource->setConnection($connection);
186-
$this->abstractResource->addCommitCallback(
208+
$this->model->setConnection($connection);
209+
$this->model->addCommitCallback(
187210
function () use ($closureExpectation) {
188211
$closureExpectation->setData(1);
189212
}
190213
);
191214

192-
$this->abstractResource->addCommitCallback(
215+
$this->model->addCommitCallback(
193216
function () use ($closureExpectation) {
194217
$closureExpectation->getData();
195218
}
@@ -206,16 +229,21 @@ function () use ($closureExpectation) {
206229
$closureExpectation->expects($this->once())
207230
->method('getData');
208231

209-
$this->abstractResource->commit();
232+
$this->model->commit();
210233
}
211234

212-
public function testCommitZeroLevelCallbackException()
235+
/**
236+
* Commit zero level callback with exception
237+
*
238+
* @return void
239+
*/
240+
public function testCommitZeroLevelCallbackException(): void
213241
{
214242
/** @var AdapterInterface|MockObject $connection */
215243
$connection = $this->getMockForAbstractClass(AdapterInterface::class);
216244

217-
$this->abstractResource->setConnection($connection);
218-
$this->abstractResource->addCommitCallback(
245+
$this->model->setConnection($connection);
246+
$this->model->addCommitCallback(
219247
function () {
220248
throw new \Exception();
221249
}
@@ -229,10 +257,15 @@ function () {
229257
$this->loggerMock->expects($this->once())
230258
->method('critical');
231259

232-
$this->abstractResource->commit();
260+
$this->model->commit();
233261
}
234262

235-
public function testCommitNotCompletedTransaction()
263+
/**
264+
* Commit of transactions that have not been completed
265+
*
266+
* @return void
267+
*/
268+
public function testCommitNotCompletedTransaction(): void
236269
{
237270
/** @var AdapterInterface|MockObject $connection */
238271
$connection = $this->getMockForAbstractClass(AdapterInterface::class);
@@ -241,8 +274,8 @@ public function testCommitNotCompletedTransaction()
241274
->disableOriginalConstructor()
242275
->getMock();
243276

244-
$this->abstractResource->setConnection($connection);
245-
$this->abstractResource->addCommitCallback(
277+
$this->model->setConnection($connection);
278+
$this->model->addCommitCallback(
246279
function () use ($closureExpectation) {
247280
$closureExpectation->setData(1);
248281
}
@@ -258,6 +291,47 @@ function () use ($closureExpectation) {
258291
->method('setData')
259292
->with(1);
260293

261-
$this->abstractResource->commit();
294+
$this->model->commit();
295+
}
296+
297+
/**
298+
* Test commit case when first callback throws an exception but other callbacks will be called
299+
*
300+
* @return void
301+
*/
302+
public function testCommitFewCallbacksWithException(): void
303+
{
304+
/** @var AdapterInterface|MockObject $connection */
305+
$connection = $this->createMock(AdapterInterface::class);
306+
307+
/** @var DataObject|MockObject $closureExpectation */
308+
$closureExpectation = $this->getMockBuilder(DataObject::class)
309+
->disableOriginalConstructor()
310+
->getMock();
311+
312+
$this->model->setConnection($connection);
313+
$this->model->addCommitCallback(
314+
function () {
315+
throw new \Exception();
316+
}
317+
);
318+
319+
$this->model->addCommitCallback(
320+
function () use ($closureExpectation) {
321+
$closureExpectation->getData();
322+
}
323+
);
324+
325+
$connection->expects($this->once())
326+
->method('commit');
327+
$connection->expects($this->once())
328+
->method('getTransactionLevel')
329+
->willReturn(0);
330+
$this->loggerMock->expects($this->once())
331+
->method('critical');
332+
$closureExpectation->expects($this->once())
333+
->method('getData');
334+
335+
$this->model->commit();
262336
}
263337
}

0 commit comments

Comments
 (0)