From 025dd8fcb76edc3e8c113feb2c4abc9cade0bbfe Mon Sep 17 00:00:00 2001 From: Dominic Tubach Date: Tue, 30 Jul 2024 15:53:07 +0200 Subject: [PATCH] Execute delete and submit actions in database transaction --- .../AbstractProfileEntityActionsHandler.php | 47 +++++- ...moteActionsHandlerTransactionDecorator.php | 68 +++++++++ .../Database/TransactionFactory.php | 31 ++++ .../Compiler/ActionHandlerPass.php | 13 ++ .../Compiler/Traits/DecorateServiceTrait.php | 28 ++++ .../AbstractRemoteEntityProfile.php | 49 ++++++ .../AbstractRemoteEntityProfileDecorator.php | 52 +++++++ services/remote-tools.php | 3 + ...ActionsHandlerTransactionDecoratorTest.php | 142 ++++++++++++++++++ 9 files changed, 428 insertions(+), 5 deletions(-) create mode 100644 Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecorator.php create mode 100644 Civi/RemoteTools/Database/TransactionFactory.php create mode 100644 Civi/RemoteTools/DependencyInjection/Compiler/Traits/DecorateServiceTrait.php create mode 100644 tests/phpunit/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecoratorTest.php diff --git a/Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandler.php b/Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandler.php index be5db1d..adee6e6 100644 --- a/Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandler.php +++ b/Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandler.php @@ -217,15 +217,35 @@ public function submitCreateForm(RemoteSubmitCreateFormAction $action throw $validationResult->toException(); } + $entityValues = $formSpec->getDataTransformer()->toEntityValues( + ReadOnlyFieldsRemover::removeReadOnlyFields($formSpec, $action->getData()), + NULL, + $action->getResolvedContactId() + ); + + $this->profile->onPreCreate( + $action->getArguments(), + $entityValues, + $entityFields, + $formSpec, + $action->getResolvedContactId() + ); + $createdValues = $this->api4->createEntity( $this->profile->getEntityName(), - $formSpec->getDataTransformer()->toEntityValues( - ReadOnlyFieldsRemover::removeReadOnlyFields($formSpec, $action->getData()), - NULL, - $action->getResolvedContactId() - ), + $entityValues, ['checkPermissions' => $this->profile->isCheckApiPermissions($action->getResolvedContactId())], )->single(); + // Custom field values might not be part of $createdValues, so we add $entityValues + $createdValues += $entityValues; + + $this->profile->onPostCreate( + $action->getArguments(), + $createdValues, + $entityFields, + $formSpec, + $action->getResolvedContactId() + ); return $this->convertToSubmitFormActionResult( $createdValues, @@ -264,6 +284,15 @@ public function submitUpdateForm(RemoteSubmitUpdateFormAction $action $entityValues, $action->getResolvedContactId() ); + + $this->profile->onPreUpdate( + $newEntityValues, + $entityValues, + $entityFields, + $formSpec, + $action->getResolvedContactId() + ); + $updatedValues = $this->api4->updateEntity( $this->profile->getEntityName(), $action->getId(), @@ -274,6 +303,14 @@ public function submitUpdateForm(RemoteSubmitUpdateFormAction $action // ones. Fields updated by triggers might be outdated, though. $updatedValues += $entityValues; + $this->profile->onPostUpdate( + $updatedValues, + $entityValues, + $entityFields, + $formSpec, + $action->getResolvedContactId() + ); + return $this->convertToSubmitFormActionResult( $updatedValues, $entityValues, diff --git a/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecorator.php b/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecorator.php new file mode 100644 index 0000000..068bdce --- /dev/null +++ b/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecorator.php @@ -0,0 +1,68 @@ +transactionFactory = $transactionFactory; + } + + public function delete(RemoteDeleteAction $action): array { + $transaction = $this->transactionFactory->createTransaction(); + try { + return parent::delete($action); + } + // @phpstan-ignore-next-line Dead catch clause. + catch (\Throwable $e) { + $transaction->rollback(); + + throw $e; + } + finally { + $transaction->commit(); + } + } + + public function submitCreateForm(RemoteSubmitCreateFormAction $action): array { + $transaction = $this->transactionFactory->createTransaction(); + try { + return parent::submitCreateForm($action); + } + // @phpstan-ignore-next-line Dead catch clause. + catch (\Throwable $e) { + $transaction->rollback(); + + throw $e; + } + finally { + $transaction->commit(); + } + } + + public function submitUpdateForm(RemoteSubmitUpdateFormAction $action): array { + $transaction = $this->transactionFactory->createTransaction(); + try { + return parent::submitUpdateForm($action); + } + // @phpstan-ignore-next-line Dead catch clause. + catch (\Throwable $e) { + $transaction->rollback(); + + throw $e; + } + finally { + $transaction->commit(); + } + } + +} diff --git a/Civi/RemoteTools/Database/TransactionFactory.php b/Civi/RemoteTools/Database/TransactionFactory.php new file mode 100644 index 0000000..c367c0a --- /dev/null +++ b/Civi/RemoteTools/Database/TransactionFactory.php @@ -0,0 +1,31 @@ +. + */ + +declare(strict_types = 1); + +namespace Civi\RemoteTools\Database; + +/** + * @codeCoverageIgnore + */ +class TransactionFactory { + + public function createTransaction(): \CRM_Core_Transaction { + return \CRM_Core_Transaction::create(); + } + +} diff --git a/Civi/RemoteTools/DependencyInjection/Compiler/ActionHandlerPass.php b/Civi/RemoteTools/DependencyInjection/Compiler/ActionHandlerPass.php index 74d2560..107214b 100644 --- a/Civi/RemoteTools/DependencyInjection/Compiler/ActionHandlerPass.php +++ b/Civi/RemoteTools/DependencyInjection/Compiler/ActionHandlerPass.php @@ -22,6 +22,8 @@ use Civi\Api4\Generic\AbstractAction; use Civi\RemoteTools\ActionHandler\ActionHandlerInterface; use Civi\RemoteTools\ActionHandler\ActionHandlerProvider; +use Civi\RemoteTools\ActionHandler\RemoteActionsHandlerTransactionDecorator; +use Civi\RemoteTools\DependencyInjection\Compiler\Traits\DecorateServiceTrait; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -33,6 +35,8 @@ */ final class ActionHandlerPass implements CompilerPassInterface { + use DecorateServiceTrait; + public static function buildHandlerKey(string $entityName, string $actionName, ?string $profileName): string { if (NULL === $profileName) { return $entityName . '.' . $actionName; @@ -79,6 +83,15 @@ public function process(ContainerBuilder $container): void { } } + foreach ($handlerServices as $handlerKey => $handlerService) { + $this->decorateService( + $container, + $handlerService->__toString(), + RemoteActionsHandlerTransactionDecorator::class, + $handlerKey + ); + } + $container->getDefinition(ActionHandlerProvider::class) ->addArgument(ServiceLocatorTagPass::register($container, $handlerServices)); } diff --git a/Civi/RemoteTools/DependencyInjection/Compiler/Traits/DecorateServiceTrait.php b/Civi/RemoteTools/DependencyInjection/Compiler/Traits/DecorateServiceTrait.php new file mode 100644 index 0000000..2895ec6 --- /dev/null +++ b/Civi/RemoteTools/DependencyInjection/Compiler/Traits/DecorateServiceTrait.php @@ -0,0 +1,28 @@ +autowire($decoratorId, $decoratorClass) + ->setDecoratedService($id) + ->setArgument($argumentKey, new Reference($decoratorId . '.inner')); + } + +} diff --git a/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfile.php b/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfile.php index 4f87514..edb5fe2 100644 --- a/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfile.php +++ b/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfile.php @@ -22,6 +22,7 @@ use Civi\RemoteTools\Api4\Query\Comparison; use Civi\RemoteTools\Api4\Query\ConditionInterface; use Civi\RemoteTools\EntityProfile\Authorization\GrantResult; +use Civi\RemoteTools\Form\FormSpec\FormSpec; use CRM_Remotetools_ExtensionUtil as E; /** @@ -118,4 +119,52 @@ public function getSaveSuccessMessage( return E::ts('Saved successfully'); } + /** + * @inheritDoc + */ + public function onPreCreate( + array $arguments, + array &$entityValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + } + + /** + * @inheritDoc + */ + public function onPostCreate( + array $arguments, + array $entityValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + } + + /** + * @inheritDoc + */ + public function onPreUpdate( + array &$newValues, + array $oldValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + } + + /** + * @inheritDoc + */ + public function onPostUpdate( + array $newValues, + array $oldValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + } + } diff --git a/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfileDecorator.php b/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfileDecorator.php index b162131..480748f 100644 --- a/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfileDecorator.php +++ b/Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfileDecorator.php @@ -166,4 +166,56 @@ public function getSaveSuccessMessage( return $this->profile->getSaveSuccessMessage($newValues, $oldValues, $formData, $contactId); } + /** + * @inheritDoc + */ + public function onPreCreate( + array $arguments, + array &$entityValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + $this->profile->onPreCreate($arguments, $entityValues, $entityFields, $formSpec, $contactId); + } + + /** + * @inheritDoc + */ + public function onPostCreate( + array $arguments, + array $entityValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + $this->profile->onPostCreate($arguments, $entityValues, $entityFields, $formSpec, $contactId); + } + + /** + * @inheritDoc + */ + public function onPreUpdate( + array &$newValues, + array $oldValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + $this->profile->onPreUpdate($newValues, $oldValues, $entityFields, $formSpec, $contactId); + } + + /** + * @inheritDoc + */ + public function onPostUpdate( + array $newValues, + array $oldValues, + array $entityFields, + FormSpec $formSpec, + ?int $contactId + ): void { + $this->profile->onPostUpdate($newValues, $oldValues, $entityFields, $formSpec, $contactId); + } + } diff --git a/services/remote-tools.php b/services/remote-tools.php index 6272f4e..2bd17f2 100644 --- a/services/remote-tools.php +++ b/services/remote-tools.php @@ -33,6 +33,7 @@ use Civi\RemoteTools\Contact\RemoteContactIdResolverInterface; use Civi\RemoteTools\Contact\RemoteContactIdResolverProvider; use Civi\RemoteTools\Contact\RemoteContactIdResolverProviderInterface; +use Civi\RemoteTools\Database\TransactionFactory; use Civi\RemoteTools\DependencyInjection\Compiler\ActionHandlerPass; use Civi\RemoteTools\DependencyInjection\Compiler\RemoteEntityProfilePass; use Civi\RemoteTools\EntityProfile\Helper\ProfileEntityDeleter; @@ -58,6 +59,8 @@ $container->autowire(RequestContextInterface::class, RequestContext::class) ->setPublic(TRUE); +$container->autowire(TransactionFactory::class); + $container->autowire(ActionHandlerProviderInterface::class, ActionHandlerProviderCollection::class) ->addArgument(new TaggedIteratorArgument(ActionHandlerProviderInterface::SERVICE_TAG)); diff --git a/tests/phpunit/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecoratorTest.php b/tests/phpunit/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecoratorTest.php new file mode 100644 index 0000000..ac8c93e --- /dev/null +++ b/tests/phpunit/Civi/RemoteTools/ActionHandler/RemoteActionsHandlerTransactionDecoratorTest.php @@ -0,0 +1,142 @@ +handlerMock = $this->createMock(RemoteEntityActionsHandlerInterface::class); + $this->transactionFactoryMock = $this->createMock(TransactionFactory::class); + $this->decorator = new RemoteActionsHandlerTransactionDecorator( + $this->handlerMock, + $this->transactionFactoryMock + ); + } + + public function testDelete(): void { + $action = $this->createApi4ActionMock(RemoteDeleteAction::class, 'RemoteEntity', 'delete'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('delete') + ->with($action) + ->willReturn(['foo' => 'bar']); + + static::assertSame(['foo' => 'bar'], $this->decorator->delete($action)); + } + + public function testDeleteRollback(): void { + $action = $this->createApi4ActionMock(RemoteDeleteAction::class, 'RemoteEntity', 'delete'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $exception = new \Exception('test'); + $transactionMock->expects(static::once())->method('rollback'); + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('delete') + ->with($action) + ->willThrowException($exception); + + static::expectExceptionObject($exception); + $this->decorator->delete($action); + } + + public function testSubmitCreateForm(): void { + $action = $this->createApi4ActionMock(RemoteSubmitCreateFormAction::class, 'RemoteEntity', 'submitCreateForm'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('submitCreateForm') + ->with($action) + ->willReturn(['foo' => 'bar']); + + static::assertSame(['foo' => 'bar'], $this->decorator->submitCreateForm($action)); + } + + public function testSubmitCreateFormRollback(): void { + $action = $this->createApi4ActionMock(RemoteSubmitCreateFormAction::class, 'RemoteEntity', 'submitCreateForm'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $exception = new \Exception('test'); + $transactionMock->expects(static::once())->method('rollback'); + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('submitCreateForm') + ->with($action) + ->willThrowException($exception); + + static::expectExceptionObject($exception); + $this->decorator->submitCreateForm($action); + } + + public function testSubmitUpdateForm(): void { + $action = $this->createApi4ActionMock(RemoteSubmitUpdateFormAction::class, 'RemoteEntity', 'submitUpdateForm'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('submitUpdateForm') + ->with($action) + ->willReturn(['foo' => 'bar']); + + static::assertSame(['foo' => 'bar'], $this->decorator->submitUpdateForm($action)); + } + + public function testSubmitUpdateFormRollback(): void { + $action = $this->createApi4ActionMock(RemoteSubmitUpdateFormAction::class, 'RemoteEntity', 'submitUpdateForm'); + + $transactionMock = $this->createMock(\CRM_Core_Transaction::class); + $this->transactionFactoryMock->method('createTransaction') + ->willReturn($transactionMock); + + $exception = new \Exception('test'); + $transactionMock->expects(static::once())->method('rollback'); + $transactionMock->expects(static::once())->method('commit'); + $this->handlerMock->method('submitUpdateForm') + ->with($action) + ->willThrowException($exception); + + static::expectExceptionObject($exception); + $this->decorator->submitUpdateForm($action); + } + +}