Skip to content

Commit

Permalink
[BUGFIX] Catch API exception to prevent Scheduler error (#20)
Browse files Browse the repository at this point in the history
Adds a new status "remote is deleted" field for sys_file table, so need to Analyze Database.
If exception is thrown when fetching media from API, this field will set to true.
This will prevent file from beeing processed in any of the conslole commands.
Also sync buttons in backend module will be removed.

Fixes #18
  • Loading branch information
pixelmatseriks authored Jul 2, 2021
1 parent b8f8711 commit f28d7e6
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 21 deletions.
18 changes: 16 additions & 2 deletions Classes/Command/UpdateQbankFileStatusCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
use Pixelant\Qbank\Repository\MediaRepository;
use Pixelant\Qbank\Repository\QbankFileRepository;
use Pixelant\Qbank\Service\QbankService;
use Pixelant\Qbank\Utility\QbankUtility;
use QBNK\QBank\API\Exception\RequestException;
use QBNK\QBank\API\Model\MediaResponse;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
Expand Down Expand Up @@ -103,8 +105,20 @@ protected function execute(InputInterface $input, OutputInterface $output)
$mediaRepository = GeneralUtility::makeInstance(MediaRepository::class);

foreach ($updateQueue as $file) {
/** @var MediaResponse $media */
$media = $mediaRepository->findById($file['tx_qbank_id']);
try {
/** @var MediaResponse $media */
$media = $mediaRepository->findById($file['tx_qbank_id']);
} catch (RequestException $re) {
$io->writeln('QBank file was not found: ' . $file['tx_qbank_id']);

if (QbankUtility::qbankRequestExceptionStatesMediaIsDeleted($re)) {
$io->writeln('Update sys_file, set tx_qbank_remote_is_deleted to 1: ' . $file['uid']);
$qbankService->updateFileRemoteIsDeleted($file['uid']);

continue;
}
}

$remoteUpdate = (int)$media->getUpdated()->getTimestamp();
$remoteReplacedBy = (int)$media->getReplacedBy();
$message = '%s sys_file, set remote change to "%s", replaced: "%s" on sys_file with uid "%s".';
Expand Down
21 changes: 20 additions & 1 deletion Classes/Controller/ManagementController.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,18 @@ private function generateButtons($action): void
->setModuleName('file_qbank')
->setGetVariables(['action', 'extension'])
->setDisplayName($this->shortcutName);

$buttonBar->addButton($shortcutButton);

$reloadButton = $buttonBar->makeLinkButton()
->setHref($this->request->getAttribute('normalizedParams')->getRequestUri())
->setTitle(
$this->getLanguageService()
->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.reload')
)
->setIcon($this->iconFactory->getIcon('actions-refresh', Icon::SIZE_SMALL));

$buttonBar->addButton($reloadButton, ButtonBar::BUTTON_POSITION_RIGHT);
}

/**
Expand Down Expand Up @@ -265,7 +276,15 @@ public function synchronizeMetadataAction(): void
continue;
}

$this->qbankService->synchronizeMetadata($file);
try {
$this->qbankService->synchronizeMetadata($file);
} catch (\Pixelant\Qbank\Exception\MediaPermanentlyDeletedException $th) {
$this->moduleTemplate->addFlashMessage(
$th->getMessage(),
'',
FlashMessage::ERROR
);
}
}

$this->moduleTemplate->addFlashMessage(
Expand Down
12 changes: 12 additions & 0 deletions Classes/Exception/MediaPermanentlyDeletedException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace Pixelant\Qbank\Exception;

/**
* Exception thrown if a QBank media is permanently deleted.
*/
class MediaPermanentlyDeletedException extends \RuntimeException
{
}
13 changes: 13 additions & 0 deletions Classes/Repository/QbankFileRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public function findAll(): array
'sys_file.tx_qbank_status_updated_timestamp',
'sys_file.tx_qbank_remote_replaced_by',
'sys_file.tx_qbank_remote_is_replaced',
'sys_file.tx_qbank_remote_is_deleted',
'sys_file_metadata.uid AS metadata_uid',
];

Expand Down Expand Up @@ -91,6 +92,12 @@ public function fetchStatusUpdateQueue(int $limit, int $interval): array
$queryBuilder->createNamedParameter(time() - $interval, \PDO::PARAM_INT)
)
)
->andWhere(
$queryBuilder->expr()->eq(
'sys_file.tx_qbank_remote_is_deleted',
$queryBuilder->createNamedParameter(0, \PDO::PARAM_INT)
)
)
->setMaxResults($limit)
->orderBy('sys_file.tx_qbank_status_updated_timestamp')
->execute();
Expand Down Expand Up @@ -121,6 +128,12 @@ public function fetchFilesToUpdate(int $limit): array
$queryBuilder->createNamedParameter(0, \PDO::PARAM_INT)
)
)
->andWhere(
$queryBuilder->expr()->eq(
'sys_file.tx_qbank_remote_is_deleted',
$queryBuilder->createNamedParameter(0, \PDO::PARAM_INT)
)
)
->andWhere(
$queryBuilder->expr()->orX(
$queryBuilder->expr()->gt(
Expand Down
39 changes: 37 additions & 2 deletions Classes/Service/QbankService.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
use Pixelant\Qbank\Service\Event\FileReferenceUrlEvent;
use Pixelant\Qbank\Service\Event\ResolvePageTitleEvent;
use Pixelant\Qbank\Utility\PropertyUtility;
use Pixelant\Qbank\Utility\QbankUtility;
use QBNK\QBank\API\Exception\RequestException;
use QBNK\QBank\API\Model\MediaUsage;
use QBNK\QBank\API\Model\PropertyType;
use TYPO3\CMS\Backend\Utility\BackendUtility;
Expand Down Expand Up @@ -224,7 +226,7 @@ public function removeMediaUsageInFileReference(int $fileReferenceId): void
* Synchronize metadata for a particular file UID.
*
* @param int $fileId The FAL file UID
* @throws \TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException
* @throws \Pixelant\Qbank\Exception\MediaPermanentlyDeletedException
*/
public function synchronizeMetadata(int $fileId): void
{
Expand All @@ -234,7 +236,19 @@ public function synchronizeMetadata(int $fileId): void
return;
}

$qbankRecord = $this->mediaRepository->findById($qbankId);
try {
/** @var MediaResponse $qbankRecord */
$qbankRecord = $this->mediaRepository->findById($qbankId);
} catch (RequestException $re) {
if (QbankUtility::qbankRequestExceptionStatesMediaIsDeleted($re)) {
$this->updateFileRemoteIsDeleted($fileId);

throw new \Pixelant\Qbank\Exception\MediaPermanentlyDeletedException(
'QBank Media is permanently deleted',
1625149218
);
}
}

$metaDataMappings = GeneralUtility::makeInstance(MappingRepository::class)->findAllAsKeyValuePairs();

Expand Down Expand Up @@ -381,6 +395,27 @@ public function updateFileRemoteChange(int $fileUid, int $remoteChangeTimeStamp,
->execute();
}

/**
* Update a sys_file record if QBank remote file is reported as permanently deleted.
*
* @param int $fileUid The local file UID
* @param bool $deleted Optional to set as not deleted.
*/
public function updateFileRemoteIsDeleted(int $fileUid, bool $deleted = true): void
{
$queryBuilder = $this->getFileQueryBuilder();
$queryBuilder->update('sys_file');

$queryBuilder->set(
'tx_qbank_remote_is_deleted',
$deleted ? 1 : 0
);

$queryBuilder
->where($queryBuilder->expr()->eq('uid', $queryBuilder->createNamedParameter($fileUid, \PDO::PARAM_INT)))
->execute();
}

/**
* Returns true if the sys_file uid suppplied in $fileId is a QBank file.
*
Expand Down
16 changes: 16 additions & 0 deletions Classes/Utility/QbankUtility.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Pixelant\Qbank\Configuration\ExtensionConfigurationManager;
use QBNK\QBank\API\Credentials;
use QBNK\QBank\API\Exception\RequestException;
use QBNK\QBank\API\QBankApi;
use TYPO3\CMS\Core\Resource\Exception\FolderDoesNotExistException;
use TYPO3\CMS\Core\Resource\Exception\InsufficientFolderAccessPermissionsException;
Expand Down Expand Up @@ -164,4 +165,19 @@ public static function getAutoUpdateOption(): int

return $extensionConfigurationManager->getAutoUpdate();
}

/**
* Returns true if request exception seems to be due to that the media is permanently deleted in QBank.
*
* @param RequestException $re
* @return bool
*/
public static function qbankRequestExceptionStatesMediaIsDeleted(RequestException $re): bool
{
if (strpos(strtolower($re->getMessage()), 'media is permanently deleted') > 0) {
return true;
}

return false;
}
}
3 changes: 3 additions & 0 deletions Resources/Private/Language/locallang.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@
<trans-unit id="be.local_media_replaced" resname="be.local_media_replaced">
<source>Local media is replaced</source>
</trans-unit>
<trans-unit id="be.remote_deleted" resname="be.remote_deleted">
<source>Remote media is deleted</source>
</trans-unit>
</body>
</file>
</xliff>
42 changes: 26 additions & 16 deletions Resources/Private/Templates/Management/List.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ <h1><f:translate key="be.list.title" /></h1>
<f:for each="{qbankFiles}" as="qbankFile">
<tr>
<td>
<f:if condition="{qbankFile.tx_qbank_remote_is_deleted}">
<f:then><core:icon identifier="tx-qbank-logo" overlay="overlay-missing" size="small" /></f:then>
<f:else><core:icon identifier="tx-qbank-logo" size="small" /></f:else>
</f:if>
<be:link.editRecord returnUrl="{returnUrl}" table="sys_file" uid="{qbankFile.uid}" title="{f:translate(key: 'LLL:EXT:core/Resources/Private/Language/locallang_mod_web_list.xlf:edit')}: {qbankFile.name} [{qbankFile.uid}, {qbankFile.tx_qbank_id}]">
{qbankFile.name}
</be:link.editRecord>
Expand Down Expand Up @@ -86,23 +90,29 @@ <h1><f:translate key="be.list.title" /></h1>
<core:icon identifier="actions-document-info" />
</a>

<f:be.link
route="file_qbank"
parameters="{action: 'synchronizeMetadata', file: qbankFile.uid}"
title="{f:translate(key: 'be.action.update-metadata')}"
class="btn btn-success"
>
<core:icon identifier="actions-database-reload" /> {f:translate(key: 'be.action.update-metadata')}
</f:be.link>
<f:if condition="{qbankFile.tx_qbank_remote_is_deleted}">
<f:then>
<core:icon identifier="actions-delete" />
<f:translate key="LLL:EXT:qbank/Resources/Private/Language/locallang.xlf:be.remote_deleted"/> ({qbankFile.tx_qbank_id})
</f:then>
<f:else>
<f:be.link
route="file_qbank"
parameters="{action: 'synchronizeMetadata', file: qbankFile.uid}"
title="{f:translate(key: 'be.action.update-metadata')}"
class="btn btn-success">
<core:icon identifier="actions-database-reload" /> {f:translate(key: 'be.action.update-metadata')}
</f:be.link>

<f:be.link
route="file_qbank"
parameters="{action: 'replaceLocalMedia', file: qbankFile.uid}"
title="{f:translate(key: 'be.action.update-file')}"
class="btn btn-warning {f:if(condition: '{qbankFile.tx_qbank_remote_replaced_by}', then: '', else: 'disabled')} {f:if(condition: '{qbankFile.tx_qbank_remote_is_replaced}', then: 'disabled')}"
>
<core:icon identifier="actions-database-reload" /> {f:translate(key: 'be.action.update-file')}
</f:be.link>
<f:be.link
route="file_qbank"
parameters="{action: 'replaceLocalMedia', file: qbankFile.uid}"
title="{f:translate(key: 'be.action.update-file')}"
class="btn btn-warning {f:if(condition: '{qbankFile.tx_qbank_remote_replaced_by}', then: '', else: 'disabled')} {f:if(condition: '{qbankFile.tx_qbank_remote_is_replaced}', then: 'disabled')}">
<core:icon identifier="actions-database-reload" /> {f:translate(key: 'be.action.update-file')}
</f:be.link>
</f:else>
</f:if>
</td>
</tr>
</f:for>
Expand Down
25 changes: 25 additions & 0 deletions Tests/Unit/Utility/QbankUtilityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Nimut\TestingFramework\TestCase\UnitTestCase;
use Pixelant\Qbank\Utility\QbankUtility;
use QBNK\QBank\API\Exception\RequestException;

/**
* Test case.
Expand Down Expand Up @@ -57,4 +58,28 @@ public function qbankDateStringToUnixTimestampReturnsCorrectTimeStamp(): void
$testDateTime->getTimestamp()
);
}

/**
* @test
*/
public function qbankRequestExceptionStatesMediaIsDeletedReturnsTrueWhenMessageContainsCertainText(): void
{
$re = new RequestException('Bad Request: Media is permanently deleted.', 400, new \Exception());

self::assertTrue(
QbankUtility::qbankRequestExceptionStatesMediaIsDeleted($re)
);
}

/**
* @test
*/
public function qbankRequestExceptionStatesMediaIsDeletedReturnsFalseWhenMessageDoesntContainCertainText(): void
{
$re = new RequestException('Bad Request: Media could not be found.', 400, new \Exception());

self::assertFalse(
QbankUtility::qbankRequestExceptionStatesMediaIsDeleted($re)
);
}
}
1 change: 1 addition & 0 deletions ext_tables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ CREATE TABLE sys_file (
tx_qbank_remote_change_timestamp int(11) unsigned DEFAULT 0 NOT NULL,
tx_qbank_remote_replaced_by int(11) unsigned DEFAULT 0 NOT NULL,
tx_qbank_remote_is_replaced tinyint(3) unsigned DEFAULT 0 NOT NULL,
tx_qbank_remote_is_deleted tinyint(3) unsigned DEFAULT 0 NOT NULL,

KEY qbank (tx_qbank_id),
);
Expand Down

0 comments on commit f28d7e6

Please sign in to comment.