Skip to content

Commit

Permalink
Refactor form sync to run as a background job with retry
Browse files Browse the repository at this point in the history
Signed-off-by: ailkiv <[email protected]>
  • Loading branch information
AIlkiv committed Nov 13, 2024
1 parent ec712fc commit e08797f
Show file tree
Hide file tree
Showing 4 changed files with 305 additions and 19 deletions.
98 changes: 98 additions & 0 deletions lib/BackgroundJob/SyncSubmissionsWithLinkedFileJob.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php
/**
* @copyright Copyright (c) 2024 Andrii Ilkiv <[email protected]>
*
* @author Andrii Ilkiv <[email protected]>
*
* @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 Psr\Log\LoggerInterface;
use Throwable;

class SyncSubmissionsWithLinkedFileJob extends QueuedJob {
public const MAX_ATTEMPTS = 10;

public function __construct(
private FormMapper $formMapper,
private FormsService $formsService,
private SubmissionService $submissionService,
ITimeFactory $time,
private LoggerInterface $logger,
private IJobList $jobList,
) {
parent::__construct($time);
}

/**
* @param array $argument
*/
public function run($argument): void {
$formId = $argument['form_id'];
$attempt = $argument['attempt'] ?: 1;

try {
$form = $this->formMapper->findById($formId);

$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
]);
} 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]
);
}
}
}

/**
* Calculates exponential delay (cubic growth) in seconds.
*/
private function nextAttempt(int $numberOfAttempt): int {
return $this->time->getTime() + pow($numberOfAttempt, 3) * 60;
}
}
16 changes: 4 additions & 12 deletions lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -62,9 +63,9 @@
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;
Expand Down Expand Up @@ -95,6 +96,7 @@ public function __construct(
private IRootFolder $rootFolder,
private UploadedFileMapper $uploadedFileMapper,
private IMimeTypeDetector $mimeTypeDetector,
private IJobList $jobList,
) {
parent::__construct($appName, $request);
$this->currentUser = $userSession->getUser();
Expand Down Expand Up @@ -1194,17 +1196,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = '
$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();
Expand Down
194 changes: 194 additions & 0 deletions tests/Unit/BackgroundJob/SyncSubmissionsWithLinkedFileJobTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2024 Andrii Ilkiv <[email protected]>
*
* @author Andrii Ilkiv <[email protected]>
*
* @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 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 IJobList|MockObject */
private $jobList;

protected function setUp(): void {
parent::setUp();

$this->formMapper = $this->createMock(FormMapper::class);
$this->formsService = $this->createMock(FormsService::class);
$this->submissionService = $this->createMock(SubmissionService::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->jobList = $this->createMock(IJobList::class);

$this->job = new SyncSubmissionsWithLinkedFileJob(
$this->formMapper,
$this->formsService,
$this->submissionService,
$this->timeFactory,
$this->logger,
$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');

$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;
}
}
Loading

0 comments on commit e08797f

Please sign in to comment.