diff --git a/Model/ConflictException.php b/Model/ConflictException.php new file mode 100644 index 0000000..2e7d284 --- /dev/null +++ b/Model/ConflictException.php @@ -0,0 +1,7 @@ +request = $request; $this->response = $response; $this->lockService = $lockService; $this->resourceConnection = $resourceConnection; - $this->modificationTimeRepo = $resourceTimestampRepository; + $this->messageGroupRepository = $messageGroupRepository; $this->errorProcessor = $errorProcessor; } @@ -47,7 +47,7 @@ public function aroundDispatch( $messageGroupId = $this->request->getHeader('X-Message-Group-ID', $default = false); $messageTimestamp = $this->request->getHeader('X-Message-Timestamp', $default = false); - if ($messageGroupId === false) { + if ($messageGroupId === false || $messageTimestamp === false) { return $proceed($request); } @@ -57,17 +57,13 @@ public function aroundDispatch( } try { - if ($messageTimestamp !== false && - $lastModificationTime = $this->modificationTimeRepo->getLastModificationTime($messageGroupId) - ) { - if (!$this->isUnmodifiedSince($lastModificationTime, $messageTimestamp)) { - $this->response->setStatusCode(412); - return $this->response; - } - $updateTimestamp = $messageTimestamp; - } else { - $lastModificationTime = null; - $updateTimestamp = \time(); + $messageGroup = $this->messageGroupRepository->getMessageGroup($messageGroupId); + $lastTimestamp = $messageGroup['timestamp'] ?? null; + $lastVersion = $messageGroup['version'] ?? null; + + if ($lastTimestamp !== null && $messageTimestamp < $lastTimestamp) { + $this->response->setStatusCode(412); + return $this->response; } $connection = $this->resourceConnection->getConnection(); @@ -83,10 +79,11 @@ public function aroundDispatch( return $response; } - $this->modificationTimeRepo->updateModificationTime( + $this->messageGroupRepository->updateModificationTime( $messageGroupId, - $updateTimestamp, - $lastModificationTime + $messageTimestamp, + $lastVersion, + $lastTimestamp ); $connection->commit(); return $response; @@ -94,6 +91,9 @@ public function aroundDispatch( $connection->rollBack(); throw $e; } + } catch (ConflictException $e) { + $this->response->setStatusCode(409); + return $this->response; } catch (\Exception $e) { $e = $this->errorProcessor->maskException($e); $this->response->setStatusCode($e->getHttpCode()); @@ -105,9 +105,4 @@ public function aroundDispatch( $this->lockService->releaseLock("idempotent_api.$messageGroupId"); } } - - private function isUnmodifiedSince(int $modificationTime, int $expectedTime) - { - return $modificationTime <= $expectedTime; - } } diff --git a/Model/ResourceModificationTimeRepository.php b/Model/ResourceModificationTimeRepository.php deleted file mode 100644 index ed06b62..0000000 --- a/Model/ResourceModificationTimeRepository.php +++ /dev/null @@ -1,54 +0,0 @@ -dbConnection = $dbContext->getResources()->getConnection($connectionName); - } - - public function getLastModificationTime(string $identifier) - { - $identifier = md5($identifier); - $select = $this->dbConnection->select() - ->from(['t' => $this->dbConnection->getTableName('webapi_resource_modification_log')], 'timestamp') - ->where('t.identifier = ?', $identifier); - $result = $this->dbConnection->fetchOne($select); - - return $result ? (int) $result : null; - } - - public function updateModificationTime( - string $identifier, - string $modificationTime, - string $expectedModificationTime = null - ) { - $identifier = md5($identifier); - - if ($expectedModificationTime !== null) { - $rowsAffected = $this->dbConnection->update( - $this->dbConnection->getTableName('webapi_resource_modification_log'), - [ - 'timestamp' => $modificationTime - ], - ['identifier = ?' => $identifier, 'timestamp = ?' => $expectedModificationTime] - ); - } else { - //new resource that has never been modified - $rowsAffected = $this->dbConnection->insert( - $this->dbConnection->getTableName('webapi_resource_modification_log'), - ['identifier' => $identifier, 'timestamp' => $modificationTime] - ); - } - - if ($rowsAffected === 0 && ($modificationTime !== $expectedModificationTime)) { - throw new \RuntimeException('There has been a conflict updating the web api resource'); - } - } -} diff --git a/Model/WebApiMessageGroupRepository.php b/Model/WebApiMessageGroupRepository.php new file mode 100644 index 0000000..9f553d2 --- /dev/null +++ b/Model/WebApiMessageGroupRepository.php @@ -0,0 +1,55 @@ +dbConnection = $dbContext->getResources()->getConnection($connectionName); + } + + public function getMessageGroup(string $id) + { + $id = md5($id); + $select = $this->dbConnection->select() + ->from(['t' => $this->dbConnection->getTableName('webapi_message_group')], ['timestamp', 'version']) + ->where('t.id = ?', $id); + $result = $this->dbConnection->fetchAssoc($select); + + return array_shift($result); + } + + public function updateModificationTime( + string $id, + string $modificationTime, + int $expectedVersion = null, + string $expectedModificationTime = null + ) { + $id = md5($id); + + if ($expectedModificationTime !== null) { + $rowsAffected = $this->dbConnection->update( + $this->dbConnection->getTableName('webapi_message_group'), + [ + 'timestamp' => $modificationTime, + 'version' => $expectedVersion + 1 + ], + ['id = ?' => $id, 'timestamp = ?' => $expectedModificationTime, 'version = ?' => $expectedVersion] + ); + } else { + $rowsAffected = $this->dbConnection->insert( + $this->dbConnection->getTableName('webapi_message_group'), + ['id' => $id, 'timestamp' => $modificationTime, 'version' => 1] + ); + } + + if ($rowsAffected === 0) { + throw new ConflictException; + } + } +} diff --git a/Setup/InstallSchema.php b/Setup/InstallSchema.php index 31b2286..76b038b 100644 --- a/Setup/InstallSchema.php +++ b/Setup/InstallSchema.php @@ -22,19 +22,25 @@ public function install(SchemaSetupInterface $setup, ModuleContextInterface $con $installer = $setup; $installer->startSetup(); $table = $installer->getConnection()->newTable( - $installer->getTable('webapi_resource_modification_log') + $installer->getTable('webapi_message_group') )->addColumn( - 'identifier', + 'id', Table::TYPE_TEXT, 255, ['primary' => true, 'nullable' => false], - 'Resource Identifier' + 'Message Group ID' )->addColumn( 'timestamp', - Table::TYPE_DECIMAL, - null, + Table::TYPE_TEXT, + 255, ['unsigned' => true, 'nullable' => false], - 'Timestamp' + 'Message Timestamp' + )->addColumn( + 'version', + Table::TYPE_INTEGER, + 255, + ['unsigned' => true, 'nullable' => false, 'default' => 1], + 'Version' ); $installer->getConnection()->createTable($table); $installer->endSetup(); diff --git a/Test/Unit/Model/DispatchPluginTest.php b/Test/Unit/Model/DispatchPluginTest.php index 4637abf..69a6221 100644 --- a/Test/Unit/Model/DispatchPluginTest.php +++ b/Test/Unit/Model/DispatchPluginTest.php @@ -11,7 +11,7 @@ use PHPUnit_Framework_MockObject_MockObject; use PHPUnit_Framework_TestCase; use SnowIO\IdempotentAPI\Model\DispatchPlugin; -use SnowIO\IdempotentAPI\Model\ResourceModificationTimeRepository; +use SnowIO\IdempotentAPI\Model\WebApiMessageGroupRepository; use SnowIO\Lock\Api\LockService; use Magento\Framework\Webapi\Response; use \Magento\Framework\App\RequestInterface; @@ -20,7 +20,7 @@ class DispatchPluginTest extends PHPUnit_Framework_TestCase { - /** @var ResourceModificationTimeRepository | PHPUnit_Framework_MockObject_MockObject */ + /** @var WebApiMessageGroupRepository | PHPUnit_Framework_MockObject_MockObject */ private $mockResourceTimestampRespository; /** @var LockService | PHPUnit_Framework_MockObject_MockObject */ @@ -56,7 +56,7 @@ public function __construct($name = null, array $data = array(), $dataName = '') public function setUp() { - $this->mockResourceTimestampRespository = $this->getMockBuilder(ResourceModificationTimeRepository::class) + $this->mockResourceTimestampRespository = $this->getMockBuilder(WebApiMessageGroupRepository::class) ->disableOriginalConstructor()->setMethods([ 'getLastModificationTime', 'updateModificationTime'