feat: Refactor form sync to run as a background job with retry

Signed-off-by: ailkiv <a.ilkiv.ye@gmail.com>
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
This commit is contained in:
ailkiv 2024-11-13 17:29:03 +02:00 коммит произвёл Kostiantyn Miakshyn
Родитель 270fc5a363
Коммит a2bd127ffc
4 изменённых файлов: 341 добавлений и 20 удалений

Просмотреть файл

@ -0,0 +1,108 @@
<?php
/**
* @copyright Copyright (c) 2024 Andrii Ilkiv <ailkiv@users.noreply.github.com>
*
* @author Andrii Ilkiv <ailkiv@users.noreply.github.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Forms\BackgroundJob;
use OCA\Forms\Db\FormMapper;
use OCA\Forms\Service\FormsService;
use OCA\Forms\Service\SubmissionService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\BackgroundJob\QueuedJob;
use OCP\Files\NotFoundException;
use OCP\IUserManager;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
use Throwable;
class SyncSubmissionsWithLinkedFileJob extends QueuedJob {
public const MAX_ATTEMPTS = 10;
public function __construct(
ITimeFactory $time,
private FormMapper $formMapper,
private FormsService $formsService,
private SubmissionService $submissionService,
private LoggerInterface $logger,
private IUserManager $userManager,
private IUserSession $userSession,
private IJobList $jobList,
) {
parent::__construct($time);
}
/**
* @param array $argument
*/
public function run($argument): void {
$oldUser = $this->userSession->getUser();
$formId = $argument['form_id'];
$attempt = $argument['attempt'] ?: 1;
try {
$form = $this->formMapper->findById($formId);
$ownerId = $form->getOwnerId();
$user = $this->userManager->get($ownerId);
$this->userSession->setUser($user);
$fileFormat = $form->getFileFormat();
$filePath = $this->formsService->getFilePath($form);
$this->submissionService->writeFileToCloud($form, $filePath, $fileFormat, $ownerId);
} catch (NotFoundException $e) {
$this->logger->notice('Form {formId} linked to a file that doesn\'t exist anymore', [
'formId' => $formId
]);
} catch (Throwable $e) {
$this->logger->warning(
'Failed to synchronize form {formId} with the file (attempt {attempt} of {maxAttempts}), reason: {message}',
[
'formId' => $formId,
'message' => $e->getMessage(),
'attempt' => $attempt,
'maxAttempts' => self::MAX_ATTEMPTS,
]
);
if ($attempt < self::MAX_ATTEMPTS) {
$this->jobList->scheduleAfter(
SyncSubmissionsWithLinkedFileJob::class,
$this->nextAttempt($attempt),
['form_id' => $formId, 'attempt' => $attempt + 1]
);
}
} finally {
$this->userSession->setUser($oldUser);
}
}
/**
* Calculates exponential delay (cubic growth) in seconds.
*/
private function nextAttempt(int $numberOfAttempt): int {
return $this->time->getTime() + pow($numberOfAttempt, 3) * 60;
}
}

Просмотреть файл

@ -30,6 +30,7 @@
namespace OCA\Forms\Controller;
use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob;
use OCA\Forms\Constants;
use OCA\Forms\Db\Answer;
use OCA\Forms\Db\AnswerMapper;
@ -57,9 +58,9 @@ use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;
use OCP\BackgroundJob\IJobList;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
@ -90,6 +91,7 @@ class ApiController extends OCSController {
private IRootFolder $rootFolder,
private UploadedFileMapper $uploadedFileMapper,
private IMimeTypeDetector $mimeTypeDetector,
private IJobList $jobList,
) {
parent::__construct($appName, $request);
$this->currentUser = $userSession->getUser();
@ -1180,17 +1182,7 @@ class ApiController extends OCSController {
$this->formsService->notifyNewSubmission($form, $submission);
if ($form->getFileId() !== null) {
try {
$filePath = $this->formsService->getFilePath($form);
$fileFormat = $form->getFileFormat();
$ownerId = $form->getOwnerId();
$this->submissionService->writeFileToCloud($form, $filePath, $fileFormat, $ownerId);
} catch (NotFoundException $e) {
$this->logger->notice('Form {formId} linked to a file that doesn\'t exist anymore', [
'formId' => $formId
]);
}
$this->jobList->add(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => $form->getId()]);
}
return new DataResponse();
@ -2469,7 +2461,7 @@ class ApiController extends OCSController {
$ownerId = $form->getOwnerId();
$this->submissionService->writeFileToCloud($form, $filePath, $fileFormat, $ownerId);
} catch (NotFoundException $e) {
} catch (OCSNotFoundException $e) {
$this->logger->notice('Form {formId} linked to a file that doesn\'t exist anymore', ['formId' => $formId]);
}
}

Просмотреть файл

@ -0,0 +1,217 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2024 Andrii Ilkiv <ailkiv@users.noreply.github.com>
*
* @author Andrii Ilkiv <ailkiv@users.noreply.github.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Forms\Tests\Unit\BackgroundJob;
use Exception;
use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob;
use OCA\Forms\Db\Form;
use OCA\Forms\Db\FormMapper;
use OCA\Forms\Service\FormsService;
use OCA\Forms\Service\SubmissionService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\Files\NotFoundException;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
use Test\TestCase;
class SyncSubmissionsWithLinkedFileJobTest extends TestCase {
private SyncSubmissionsWithLinkedFileJob $job;
/** @var FormMapper|MockObject */
private $formMapper;
/** @var FormsService|MockObject */
private $formsService;
/** @var SubmissionService|MockObject */
private $submissionService;
/** @var ITimeFactory|MockObject */
private $timeFactory;
/** @var LoggerInterface|MockObject */
private $logger;
/** @var IUserManager|MockObject */
private $userManager;
/** @var IUserSession|MockObject */
private $userSession;
/** @var IJobList|MockObject */
private $jobList;
protected function setUp(): void {
parent::setUp();
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->formMapper = $this->createMock(FormMapper::class);
$this->formsService = $this->createMock(FormsService::class);
$this->submissionService = $this->createMock(SubmissionService::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->userSession = $this->createMock(IUserSession::class);
$this->jobList = $this->createMock(IJobList::class);
$this->job = new SyncSubmissionsWithLinkedFileJob(
$this->timeFactory,
$this->formMapper,
$this->formsService,
$this->submissionService,
$this->logger,
$this->userManager,
$this->userSession,
$this->jobList
);
}
public function testRunSuccessfulSync(): void {
$formId = 1;
$argument = ['form_id' => $formId, 'attempt' => 1];
$form = $this->getForm($formId);
$this->formMapper->expects($this->once())
->method('findById')
->with($formId)
->willReturn($form);
$this->formsService->expects($this->once())
->method('getFilePath')
->with($form)
->willReturn('some/file/path');
$user = $this->createMock(IUser::class);
$this->userManager->expects($this->once())
->method('get')
->with('owner_name')
->willReturn($user);
$this->userSession->expects($this->once())
->method('getUser')
->willReturn(null);
$this->submissionService->expects($this->once())
->method('writeFileToCloud')
->with($form, 'some/file/path', $this->anything(), $this->anything());
$this->job->run($argument);
}
public function testRunNotFoundException(): void {
$formId = 1;
$argument = ['form_id' => $formId, 'attempt' => 1];
$this->formMapper->expects($this->once())
->method('findById')
->with($formId)
->willThrowException(new NotFoundException('Test exception'));
$this->logger->expects($this->once())
->method('notice')
->with('Form {formId} linked to a file that doesn\'t exist anymore', ['formId' => $formId]);
$this->job->run($argument);
}
public function testRunThrowableException(): void {
$formId = 1;
$argument = ['form_id' => $formId, 'attempt' => 1];
$form = $this->getForm($formId);
$this->formMapper->expects($this->once())
->method('findById')
->with($formId)
->willReturn($form);
$this->formsService->expects($this->once())
->method('getFilePath')
->willReturn('some/file/path');
$this->submissionService->expects($this->once())
->method('writeFileToCloud')
->willThrowException(new Exception('Test exception'));
$this->logger->expects($this->once())
->method('warning')
->with(
'Failed to synchronize form {formId} with the file (attempt {attempt} of {maxAttempts}), reason: {message}',
[
'formId' => $formId,
'message' => 'Test exception',
'attempt' => 1,
'maxAttempts' => SyncSubmissionsWithLinkedFileJob::MAX_ATTEMPTS
]
);
$this->jobList->expects($this->once())
->method('scheduleAfter')
->with(
SyncSubmissionsWithLinkedFileJob::class,
$this->anything(),
['form_id' => $formId, 'attempt' => 2]
);
$this->job->run($argument);
}
public function testMaxAttemptsReached(): void {
$formId = 1;
$argument = ['form_id' => $formId, 'attempt' => SyncSubmissionsWithLinkedFileJob::MAX_ATTEMPTS];
$form = $this->getForm($formId);
$this->formMapper->expects($this->once())
->method('findById')
->with($formId)
->willReturn($form);
$this->formsService->expects($this->once())
->method('getFilePath')
->willReturn('some/file/path');
$this->submissionService->expects($this->once())
->method('writeFileToCloud')
->willThrowException(new Exception('Test exception'));
$this->jobList->expects($this->never())->method('add');
$this->job->run($argument);
}
private function getForm(int $formId): Form {
$form = new Form();
$form->setId($formId);
$form->setFileFormat('csv');
$form->setOwnerId('owner_name');
return $form;
}
}

Просмотреть файл

@ -47,6 +47,7 @@ function is_uploaded_file(string|bool|null $filename) {
namespace OCA\Forms\Tests\Unit\Controller;
use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob;
use OCA\Forms\Constants;
use OCA\Forms\Controller\ApiController;
use OCA\Forms\Db\AnswerMapper;
@ -69,6 +70,7 @@ use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\BackgroundJob\IJobList;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\IMimeTypeDetector;
@ -121,6 +123,8 @@ class ApiControllerTest extends TestCase {
private $uploadedFileMapper;
/** @var IMimeTypeDetector|MockObject */
private $mimeTypeDetector;
/** @var IJobList|MockObject */
private $jobList;
public function setUp(): void {
$this->answerMapper = $this->createMock(AnswerMapper::class);
@ -145,6 +149,7 @@ class ApiControllerTest extends TestCase {
$this->storage = $this->createMock(IRootFolder::class);
$this->uploadedFileMapper = $this->createMock(UploadedFileMapper::class);
$this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class);
$this->jobList = $this->createMock(IJobList::class);
$this->apiController = new ApiController(
'forms',
@ -165,6 +170,7 @@ class ApiControllerTest extends TestCase {
$this->storage,
$this->uploadedFileMapper,
$this->mimeTypeDetector,
$this->jobList,
);
}
@ -453,6 +459,7 @@ class ApiControllerTest extends TestCase {
$this->storage,
$this->uploadedFileMapper,
$this->mimeTypeDetector,
$this->jobList,
])->getMock();
$this->configService->expects($this->once())
@ -614,6 +621,7 @@ class ApiControllerTest extends TestCase {
$this->storage,
$this->uploadedFileMapper,
$this->mimeTypeDetector,
$this->jobList,
])
->getMock();
@ -827,13 +835,9 @@ class ApiControllerTest extends TestCase {
$this->formsService->expects($this->once())
->method('notifyNewSubmission');
$this->formsService->expects($this->once())
->method('getFilePath')
->willReturn('foo/bar');
$this->submissionService->expects($this->once())
->method('writeFileToCloud')
->with($form, 'foo/bar', 'xlsx', 'admin');
$this->jobList->expects($this->once())
->method('add')
->with(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => 1]);
$userFolder = $this->createMock(Folder::class);
$userFolder->expects($this->once())