Refactor transaction logic in AuditDumpRequestHandler and optimize audit dumping process

- Removed unused `StoredObjectRepository` dependency and improved transaction management by replacing `beganTx` flag with `isTransactionActive` checks.
- Adjusted entity locking logic to separate `find()` and `lock()` calls for `StoredObject`.
- Simplified `AuditEventDumper` by removing redundant assertions and unused `AuditTrail` import.
- Standardized subject display configuration in `AuditEventDumper` and ensured consistent data formatting.
This commit is contained in:
2026-03-04 15:09:32 +01:00
parent 5494f227c4
commit 7bf619aeca
3 changed files with 40 additions and 30 deletions

View File

@@ -16,7 +16,6 @@ use Chill\DocStoreBundle\Service\StoredObjectManagerInterface;
use Chill\MainBundle\Audit\Exception\AuditDumpAlreadyGeneratedException;
use Chill\MainBundle\Audit\Exception\AuditDumpTooMuchLines;
use Chill\MainBundle\Repository\AuditTrailRepository;
use Chill\MainBundle\Entity\AuditTrail;
use Chill\MainBundle\Templating\Entity\UserRender;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
@@ -91,20 +90,16 @@ class AuditEventDumper
$i = 0;
foreach ($this->auditTrailRepository->findByCriteriaIterator($criteria) as $audit) {
\assert($audit instanceof AuditTrail);
$occurredAt = $formatter?->format($audit->getOccurredAt()) ?: $audit->getOccurredAt()->format('c');
$occurredAt = $formatter->format($audit->getOccurredAt());
$user = $audit->getUser();
$userId = $user?->getId();
$userLabel = $user ? $this->userRender->renderString($user, ['at_date' => $audit->getOccurredAt()]) : '';
$userLabel = null !== $user ? $this->userRender->renderString($user, ['at_date' => $audit->getOccurredAt()]) : '';
// Displayable name of the audit (main subject)
$mainSubjectDisplay = '';
$mainSubject = $audit->getMainSubject();
if (null !== $mainSubject) {
$mainSubjectDisplay = $this->converterManager->display(Subject::fromArray($mainSubject), 'string');
}
$mainSubjectDisplay = $this->converterManager->display(Subject::fromArray($mainSubject), 'string');
// Translated action
$action = $this->translator->trans('audit_trail.action.'.$audit->getAction(), [], null, $lang);
@@ -115,9 +110,9 @@ class AuditEventDumper
// Associated subjects, comma separated
$subjectsDisplay = '';
$subjects = $audit->getSubjects();
if (!empty($subjects)) {
if ([] !== $subjects) {
$subjectsDisplay = implode(', ', array_map(function (array $s): string {
return $this->converterManager->display(Subject::fromArray($s), 'fr');
return $this->converterManager->display(Subject::fromArray($s), 'string');
}, $subjects));
}
@@ -145,7 +140,7 @@ class AuditEventDumper
$storedObject->setStatus(StoredObject::STATUS_READY);
$this->entityManager->flush();
$content = file_get_contents($tmpPath) ?: '';
$content = file_get_contents($tmpPath);
$this->storedObjectManager->write($storedObject, $content, 'text/csv');
// Remove temp file from disk

View File

@@ -12,7 +12,6 @@ declare(strict_types=1);
namespace Chill\MainBundle\Audit\Messenger;
use Chill\DocStoreBundle\Entity\StoredObject;
use Chill\DocStoreBundle\Repository\StoredObjectRepositoryInterface;
use Chill\MainBundle\Audit\AuditEventDumper;
use Chill\MainBundle\Audit\Exception\AuditDumpAlreadyGeneratedException;
use Chill\MainBundle\Audit\Exception\AuditDumpTooMuchLines;
@@ -27,7 +26,6 @@ final readonly class AuditDumpRequestHandler implements MessageHandlerInterface
{
public function __construct(
private AuditEventDumper $auditEventDumper,
private StoredObjectRepositoryInterface $storedObjectRepository,
private EntityManagerInterface $entityManager,
private UserRepositoryInterface $userRepository,
) {}
@@ -35,24 +33,25 @@ final readonly class AuditDumpRequestHandler implements MessageHandlerInterface
public function __invoke(AuditDumpRequestMessage $message): void
{
$conn = $this->entityManager->getConnection();
$beganTx = false;
if (!$conn->isTransactionActive()) {
$conn->beginTransaction();
$beganTx = true;
}
try {
// Lock the StoredObject until the end of the process
$storedObject = $this->entityManager->find(StoredObject::class, $message->storedObjectId, LockMode::PESSIMISTIC_WRITE);
$storedObject = $this->entityManager->find(StoredObject::class, $message->storedObjectId);
if (!$storedObject instanceof StoredObject) {
// Nothing to do if stored object does not exist anymore
if ($beganTx) {
if ($conn->isTransactionActive()) {
$conn->commit();
}
return;
}
$this->entityManager->lock($storedObject, LockMode::PESSIMISTIC_WRITE);
// Build criteria expected by AuditTrailRepository
$criteria = [];
if (null !== $message->from) {
@@ -83,18 +82,18 @@ final readonly class AuditDumpRequestHandler implements MessageHandlerInterface
$this->auditEventDumper->dump($criteria, $message->lang, $storedObject);
$this->entityManager->flush();
if ($beganTx) {
if ($conn->isTransactionActive()) {
$conn->commit();
}
} catch (AuditDumpTooMuchLines|AuditDumpAlreadyGeneratedException $e) {
if ($beganTx) {
if ($conn->isTransactionActive()) {
$this->entityManager->flush();
$conn->commit();
}
throw new UnrecoverableMessageHandlingException(previous: $e);
} catch (\Throwable $e) {
if ($beganTx && $conn->isTransactionActive()) {
if ($conn->isTransactionActive()) {
$conn->rollBack();
}
throw $e;

View File

@@ -57,7 +57,6 @@ class AuditDumpRequestHandlerTest extends TestCase
$this->handler = new AuditDumpRequestHandler(
$this->auditEventDumper->reveal(),
$this->storedObjectRepository->reveal(),
$this->entityManager->reveal(),
$this->userRepository->reveal()
);
@@ -77,11 +76,17 @@ class AuditDumpRequestHandlerTest extends TestCase
$storedObject = new StoredObject();
$user = $this->prophesize(User::class)->reveal();
$this->connection->isTransactionActive()->willReturn(false);
$this->connection->beginTransaction()->shouldBeCalled();
$beganTx = false;
$this->connection->isTransactionActive()->will(function ($args) use (&$beganTx) {
return $beganTx;
});
$this->connection->beginTransaction()->will(function ($args) use (&$beganTx) {
$beganTx = true;
})->shouldBeCalled();
$this->entityManager->find(StoredObject::class, 123, LockMode::PESSIMISTIC_WRITE)
$this->entityManager->find(StoredObject::class, 123)
->willReturn($storedObject);
$this->entityManager->lock($storedObject, LockMode::PESSIMISTIC_WRITE)->shouldBeCalled();
$this->userRepository->find(1)->willReturn($user);
@@ -119,11 +124,17 @@ class AuditDumpRequestHandlerTest extends TestCase
$message = new AuditDumpRequestMessage('fr', 123);
$storedObject = new StoredObject();
$this->connection->isTransactionActive()->willReturn(false, true);
$this->connection->beginTransaction()->shouldBeCalled();
$beganTx = false;
$this->connection->isTransactionActive()->will(function ($args) use (&$beganTx) {
return $beganTx;
});
$this->connection->beginTransaction()->will(function ($args) use (&$beganTx) {
$beganTx = true;
})->shouldBeCalled();
$this->entityManager->find(StoredObject::class, 123, LockMode::PESSIMISTIC_WRITE)
$this->entityManager->find(StoredObject::class, 123)
->willReturn($storedObject);
$this->entityManager->lock($storedObject, LockMode::PESSIMISTIC_WRITE)->shouldBeCalled();
$this->auditEventDumper->dump(Argument::any(), 'fr', $storedObject)
->willThrow($exception);
@@ -153,10 +164,15 @@ class AuditDumpRequestHandlerTest extends TestCase
{
$message = new AuditDumpRequestMessage('fr', 123);
$this->connection->isTransactionActive()->willReturn(false);
$this->connection->beginTransaction()->shouldBeCalled();
$beganTx = false;
$this->connection->isTransactionActive()->will(function ($args) use (&$beganTx) {
return $beganTx;
});
$this->connection->beginTransaction()->will(function ($args) use (&$beganTx) {
$beganTx = true;
})->shouldBeCalled();
$this->entityManager->find(StoredObject::class, 123, LockMode::PESSIMISTIC_WRITE)
$this->entityManager->find(StoredObject::class, 123)
->willReturn(null);
$this->connection->commit()->shouldBeCalled();