diff --git a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php index 255e6d94d741f..b55b33ddfa54b 100644 --- a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php +++ b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php @@ -13,6 +13,7 @@ /** * Abstract resource model * + * phpcs:disable Magento2.Classes.AbstractApi * @api */ abstract class AbstractResource @@ -81,6 +82,7 @@ public function addCommitCallback($callback) /** * Commit resource transaction * + * @deprecated see \Magento\Framework\Model\ExecuteCommitCallbacks::afterCommit * @return $this * @api */ @@ -92,14 +94,15 @@ public function commit() */ if ($this->getConnection()->getTransactionLevel() === 0) { $callbacks = CallbackPool::get(spl_object_hash($this->getConnection())); - try { - foreach ($callbacks as $callback) { + foreach ($callbacks as $callback) { + try { call_user_func($callback); + } catch (\Exception $e) { + $this->getLogger()->critical($e); } - } catch (\Exception $e) { - $this->getLogger()->critical($e); } } + return $this; } diff --git a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceTest.php b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceTest.php index ee0d087abdbb2..f3b8bb74737b7 100644 --- a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceTest.php +++ b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceTest.php @@ -15,12 +15,15 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +/** + * Test for \Magento\Framework\Model\ResourceModel\AbstractResource. + */ class AbstractResourceTest extends TestCase { /** * @var AbstractResourceStub */ - private $abstractResource; + private $model; /** * @var Json|MockObject @@ -32,43 +35,51 @@ class AbstractResourceTest extends TestCase */ private $loggerMock; + /** + * @inheritdoc + */ protected function setUp(): void { $objectManager = new ObjectManager($this); $this->serializerMock = $this->createMock(Json::class); $this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class); - $this->abstractResource = $objectManager->getObject(AbstractResourceStub::class); - $objectManager->setBackwardCompatibleProperty($this->abstractResource, 'serializer', $this->serializerMock); - $objectManager->setBackwardCompatibleProperty($this->abstractResource, '_logger', $this->loggerMock); + $this->model = $objectManager->getObject(AbstractResourceStub::class); + $objectManager->setBackwardCompatibleProperty($this->model, 'serializer', $this->serializerMock); + $objectManager->setBackwardCompatibleProperty($this->model, '_logger', $this->loggerMock); } /** + * Test fields serialize + * * @param array $arguments - * @param string $expected + * @param string|null $expected * @param array|string|int $serializeCalledWith * @param int $numSerializeCalled + * @return void * @dataProvider serializeFieldsDataProvider */ public function testSerializeFields( array $arguments, - $expected, + ?string $expected, $serializeCalledWith, - $numSerializeCalled = 1 - ) { + int $numSerializeCalled = 1 + ): void { /** @var DataObject $dataObject */ - list($dataObject, $field, $defaultValue, $unsetEmpty) = $arguments; + [$dataObject, $field, $defaultValue, $unsetEmpty] = $arguments; $this->serializerMock->expects($this->exactly($numSerializeCalled)) ->method('serialize') ->with($serializeCalledWith) ->willReturn($expected); - $this->abstractResource->_serializeField($dataObject, $field, $defaultValue, $unsetEmpty); + $this->model->_serializeField($dataObject, $field, $defaultValue, $unsetEmpty); $this->assertEquals($expected, $dataObject->getData($field)); } /** + * DataProvider for testSerializeFields() + * * @return array */ - public function serializeFieldsDataProvider() + public function serializeFieldsDataProvider(): array { $array = ['a', 'b', 'c']; $string = 'i am string'; @@ -80,60 +91,66 @@ public function serializeFieldsDataProvider() 'string' => $string, 'integer' => $integer, 'empty' => $empty, - 'empty_with_default' => '' + 'empty_with_default' => '', ] ); + return [ [ [$dataObject, 'array', null, false], '["a","b","c"]', - $array + $array, ], [ [$dataObject, 'string', null, false], '"i am string"', - $string + $string, ], [ [$dataObject, 'integer', null, false], '969', - $integer + $integer, ], [ [$dataObject, 'empty', null, true], null, $empty, - 0 + 0, ], [ [$dataObject, 'empty_with_default', 'default', false], '"default"', - 'default' - ] + 'default', + ], ]; } /** + * Test fields unserialize + * * @param array $arguments * @param array|string|int|boolean $expected + * @return void * @dataProvider unserializeFieldsDataProvider */ - public function testUnserializeFields(array $arguments, $expected) + public function testUnserializeFields(array $arguments, $expected): void { /** @var DataObject $dataObject */ - list($dataObject, $field, $defaultValue) = $arguments; + [$dataObject, $field, $defaultValue] = $arguments; $this->serializerMock->expects($this->once()) ->method('unserialize') ->with($dataObject->getData($field)) ->willReturn($expected); - $this->abstractResource->_unserializeField($dataObject, $field, $defaultValue); + $this->model->_unserializeField($dataObject, $field, $defaultValue); $this->assertEquals($expected, $dataObject->getData($field)); } /** + * DataProvider for testUnserializeFields() + * * @return array */ - public function unserializeFieldsDataProvider() + public function unserializeFieldsDataProvider(): array { $dataObject = new DataObject( [ @@ -142,38 +159,44 @@ public function unserializeFieldsDataProvider() 'integer' => '969', 'empty_with_default' => '""', 'not_serialized_string' => 'i am string', - 'serialized_boolean_false' => 'false' + 'serialized_boolean_false' => 'false', ] ); + return [ [ [$dataObject, 'array', null], - ['a', 'b', 'c'] + ['a', 'b', 'c'], ], [ [$dataObject, 'string', null], - 'i am string' + 'i am string', ], [ [$dataObject, 'integer', null], - 969 + 969, ], [ [$dataObject, 'empty_with_default', 'default', false], - 'default' + 'default', ], [ [$dataObject, 'not_serialized_string', null], - 'i am string' + 'i am string', ], [ [$dataObject, 'serialized_boolean_false', null], false, - ] + ], ]; } - public function testCommitZeroLevel() + /** + * Commit zero level + * + * @return void + */ + public function testCommitZeroLevel(): void { /** @var AdapterInterface|MockObject $connection */ $connection = $this->getMockForAbstractClass(AdapterInterface::class); @@ -182,14 +205,14 @@ public function testCommitZeroLevel() ->disableOriginalConstructor() ->getMock(); - $this->abstractResource->setConnection($connection); - $this->abstractResource->addCommitCallback( + $this->model->setConnection($connection); + $this->model->addCommitCallback( function () use ($closureExpectation) { $closureExpectation->setData(1); } ); - $this->abstractResource->addCommitCallback( + $this->model->addCommitCallback( function () use ($closureExpectation) { $closureExpectation->getData(); } @@ -206,16 +229,21 @@ function () use ($closureExpectation) { $closureExpectation->expects($this->once()) ->method('getData'); - $this->abstractResource->commit(); + $this->model->commit(); } - public function testCommitZeroLevelCallbackException() + /** + * Commit zero level callback with exception + * + * @return void + */ + public function testCommitZeroLevelCallbackException(): void { /** @var AdapterInterface|MockObject $connection */ $connection = $this->getMockForAbstractClass(AdapterInterface::class); - $this->abstractResource->setConnection($connection); - $this->abstractResource->addCommitCallback( + $this->model->setConnection($connection); + $this->model->addCommitCallback( function () { throw new \Exception(); } @@ -229,10 +257,15 @@ function () { $this->loggerMock->expects($this->once()) ->method('critical'); - $this->abstractResource->commit(); + $this->model->commit(); } - public function testCommitNotCompletedTransaction() + /** + * Commit of transactions that have not been completed + * + * @return void + */ + public function testCommitNotCompletedTransaction(): void { /** @var AdapterInterface|MockObject $connection */ $connection = $this->getMockForAbstractClass(AdapterInterface::class); @@ -241,8 +274,8 @@ public function testCommitNotCompletedTransaction() ->disableOriginalConstructor() ->getMock(); - $this->abstractResource->setConnection($connection); - $this->abstractResource->addCommitCallback( + $this->model->setConnection($connection); + $this->model->addCommitCallback( function () use ($closureExpectation) { $closureExpectation->setData(1); } @@ -258,6 +291,47 @@ function () use ($closureExpectation) { ->method('setData') ->with(1); - $this->abstractResource->commit(); + $this->model->commit(); + } + + /** + * Test commit case when first callback throws an exception but other callbacks will be called + * + * @return void + */ + public function testCommitFewCallbacksWithException(): void + { + /** @var AdapterInterface|MockObject $connection */ + $connection = $this->createMock(AdapterInterface::class); + + /** @var DataObject|MockObject $closureExpectation */ + $closureExpectation = $this->getMockBuilder(DataObject::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model->setConnection($connection); + $this->model->addCommitCallback( + function () { + throw new \Exception(); + } + ); + + $this->model->addCommitCallback( + function () use ($closureExpectation) { + $closureExpectation->getData(); + } + ); + + $connection->expects($this->once()) + ->method('commit'); + $connection->expects($this->once()) + ->method('getTransactionLevel') + ->willReturn(0); + $this->loggerMock->expects($this->once()) + ->method('critical'); + $closureExpectation->expects($this->once()) + ->method('getData'); + + $this->model->commit(); } }