From 5c43965aafb975f8f00ab42849b761b131a0d0fb Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:06:55 +1030 Subject: [PATCH 1/9] Add ability to download export data file --- .../Controller/Adminhtml/Exports/Data.php | 95 +++++++++++++++++++ .../Ui/Component/Listing/Column/Actions.php | 58 ++++++----- 2 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 src/export/Controller/Adminhtml/Exports/Data.php diff --git a/src/export/Controller/Adminhtml/Exports/Data.php b/src/export/Controller/Adminhtml/Exports/Data.php new file mode 100644 index 0000000..1c5fe1c --- /dev/null +++ b/src/export/Controller/Adminhtml/Exports/Data.php @@ -0,0 +1,95 @@ +getRequest()->getParam('id'); + if (empty($exportId)) { + $this->getMessageManager()->addErrorMessage(__('Export ID is required.')); + $this->_redirect('*/*/index'); + return $this->_response; + } + $isZip = (bool)$this->getRequest()->getParam('zip'); + /** @var Export $export */ + $export = $this->exportFactory->create(); + $this->exportResource->load($export, $exportId); + if ($export->isEmpty()) { + $this->getMessageManager()->addErrorMessage(__('Could not load export data.')); + $this->_redirect('*/*/index'); + return $this->_response; + } + $directory = $export->getDirectory(); + $filename = match ($export->getExportType()) { + ExportInterface::EXPORT_TYPE_INCREMENTAL => ExportInterface::ZIP_FILENAME_INCREMENTAL, + ExportInterface::EXPORT_TYPE_FULL => ExportInterface::ZIP_FILENAME_FULL, + ExportInterface::EXPORT_TYPE_SUGGEST => ExportInterface::ZIP_FILENAME_SUGGEST, + default => '' + }; + if (empty($filename)) { + $this->getMessageManager()->addErrorMessage(__('Could not download data file.')); + $this->_redirect('*/*/index'); + return $this->_response; + } + + $filename = $directory . DIRECTORY_SEPARATOR . $filename; + $name = $this->filesystem->getPathInfo($filename)['basename'] ?? $filename; + try { + return $this->fileFactory->create( + $name, + [ + 'type' => 'filename', + 'value' => $filename, + 'rm' => false + ], + DirectoryList::VAR_DIR, + 'application/zip' + ); + } catch (\Exception $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + $this->getMessageManager()->addErrorMessage(__('Could not download data file.')); + $this->_redirect('*/*/index'); + return $this->_response; + } + } +} diff --git a/src/export/Ui/Component/Listing/Column/Actions.php b/src/export/Ui/Component/Listing/Column/Actions.php index e342b74..d4a04e1 100644 --- a/src/export/Ui/Component/Listing/Column/Actions.php +++ b/src/export/Ui/Component/Listing/Column/Actions.php @@ -14,6 +14,7 @@ class Actions extends Column { private const string URL_PATH_REPORT = 'fredhopper/exports/report'; + private const string URL_PATH_DATA_FILE = 'fredhopper/exports/data'; /** * @param ContextInterface $context @@ -43,32 +44,44 @@ public function prepareDataSource(array $dataSource): array foreach ($dataSource['data']['items'] as &$item) { if (isset($item[ExportInterface::FIELD_EXPORT_ID])) { $exportId = (int)$item[ExportInterface::FIELD_EXPORT_ID]; + // give link to download export zip file + $actions = [ + 'download_data' => [ + 'href' => $this->urlBuilder->getUrl( + self::URL_PATH_DATA_FILE, + [ + 'id' => $item[ExportInterface::FIELD_EXPORT_ID] + ] + ), + 'label' => __('Download Data File'), + 'target' => '_blank' + ] + ]; // if the export is already complete, we can download a quality report if ($this->canDownloadReport($exportId)) { - $item[$this->getData('name')] = [ - 'download' => [ - 'href' => $this->urlBuilder->getUrl( - self::URL_PATH_REPORT, - [ - 'id' => $item[ExportInterface::FIELD_EXPORT_ID] - ] - ), - 'label' => __('Download Quality Report'), - 'target' => '_blank', - ], - 'download_zip' => [ - 'href' => $this->urlBuilder->getUrl( - self::URL_PATH_REPORT, - [ - 'id' => $item[ExportInterface::FIELD_EXPORT_ID], - 'zip' => true - ] - ), - 'label' => __('Download ZIP Report'), - 'target' => '_blank', - ] + $actions['download_report'] = [ + 'href' => $this->urlBuilder->getUrl( + self::URL_PATH_REPORT, + [ + 'id' => $item[ExportInterface::FIELD_EXPORT_ID] + ] + ), + 'label' => __('Download Quality Report'), + 'target' => '_blank' + ]; + $actions['download_report_zip'] = [ + 'href' => $this->urlBuilder->getUrl( + self::URL_PATH_REPORT, + [ + 'id' => $item[ExportInterface::FIELD_EXPORT_ID], + 'zip' => true + ] + ), + 'label' => __('Download ZIP Report'), + 'target' => '_blank', ]; } + $item[$this->getData('name')] = $actions; } } } @@ -99,5 +112,4 @@ private function canDownloadReport(int $exportId): bool } return true; } - } From 9c8e1a409922a06e081e88a77a00f431e0e0b95c Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:07:36 +1030 Subject: [PATCH 2/9] Increase frequency of cron jobs that update exports. --- src/export/etc/crontab.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/export/etc/crontab.xml b/src/export/etc/crontab.xml index d3785c9..2050f83 100644 --- a/src/export/etc/crontab.xml +++ b/src/export/etc/crontab.xml @@ -15,19 +15,19 @@ - */10 * * * * + */1 * * * * - */10 * * * * + */1 * * * * - */5 * * * * + */1 * * * * - */5 * * * * + */1 * * * * From 051bc26e12f8d14bebe7ca5f911fd10133ddd308 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:09:13 +1030 Subject: [PATCH 3/9] Fix bug with suggest export where directory is not set --- src/export/Cron/Generate/Suggest.php | 8 ++++++++ src/export/Model/GenerateSuggestExport.php | 5 +++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/export/Cron/Generate/Suggest.php b/src/export/Cron/Generate/Suggest.php index def8a2d..974f0ff 100644 --- a/src/export/Cron/Generate/Suggest.php +++ b/src/export/Cron/Generate/Suggest.php @@ -7,11 +7,19 @@ class Suggest { + /** + * @param GenerateSuggestExport $generateSuggestExport + */ public function __construct( private readonly GenerateSuggestExport $generateSuggestExport ) { } + /** + * Generate a suggest export + * + * @return void + */ public function execute(): void { $this->generateSuggestExport->execute(); diff --git a/src/export/Model/GenerateSuggestExport.php b/src/export/Model/GenerateSuggestExport.php index 3f69a2a..c3430bb 100644 --- a/src/export/Model/GenerateSuggestExport.php +++ b/src/export/Model/GenerateSuggestExport.php @@ -51,6 +51,7 @@ public function execute(): void // create directory $directory = $this->createDirectory->execute(ExportInterface::EXPORT_TYPE_SUGGEST); + $export->setDirectory($directory); // create all required files $files = []; @@ -74,13 +75,13 @@ public function execute(): void $files ); if (!$zipCreated) { - throw new LocalizedException(__('Error while creating ZIP file.')); + throw new LocalizedException(__(__METHOD__ . ': Error while creating ZIP file.')); } // save export $this->exportResource->save($export); } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); + $this->logger->error(__METHOD__ . ': ' . $e->getMessage(), ['exception' => $e]); } } } From 73679c6c5c6eea2594521f98ae23e26e160a587c Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:09:24 +1030 Subject: [PATCH 4/9] Add service class to indicate if an export is in progress --- src/export/Model/GetExportIsInProgress.php | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 src/export/Model/GetExportIsInProgress.php diff --git a/src/export/Model/GetExportIsInProgress.php b/src/export/Model/GetExportIsInProgress.php new file mode 100644 index 0000000..4ed263f --- /dev/null +++ b/src/export/Model/GetExportIsInProgress.php @@ -0,0 +1,41 @@ +collectionFactory->create(); + $statusesToCheck = $triggeredOnly ? [ExportInterface::STATUS_TRIGGERED] : self::IN_PROGRESS_STATUSES; + $collection->addFieldToFilter('status', ['in' => self::IN_PROGRESS_STATUSES]); + return $collection->getSize() > 0; + } +} From c8914bc6b423d8b843e878aa38cfe4c6dfcdc1f6 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:09:45 +1030 Subject: [PATCH 5/9] Move invalidating of exports to service class --- src/export/Cron/UpdateInvalidExports.php | 39 ++-------------- src/export/Model/InvalidateExports.php | 59 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 35 deletions(-) create mode 100644 src/export/Model/InvalidateExports.php diff --git a/src/export/Cron/UpdateInvalidExports.php b/src/export/Cron/UpdateInvalidExports.php index cb14f0a..cf18eae 100644 --- a/src/export/Cron/UpdateInvalidExports.php +++ b/src/export/Cron/UpdateInvalidExports.php @@ -3,27 +3,16 @@ declare(strict_types=1); namespace Aligent\FredhopperExport\Cron; -use Aligent\FredhopperExport\Api\Data\ExportInterface; -use Aligent\FredhopperExport\Model\Data\Export; -use Aligent\FredhopperExport\Model\Data\GetCurrentExportedVersion; -use Aligent\FredhopperExport\Model\ResourceModel\Data\Export as ExportResource; -use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\Collection; +use Aligent\FredhopperExport\Model\InvalidateExports; use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\CollectionFactory; -use Psr\Log\LoggerInterface; class UpdateInvalidExports { /** - * @param CollectionFactory $collectionFactory - * @param GetCurrentExportedVersion $getCurrentExportedVersion - * @param ExportResource $exportResource - * @param LoggerInterface $logger + * @param InvalidateExports $invalidateExports */ public function __construct( - private readonly CollectionFactory $collectionFactory, - private readonly GetCurrentExportedVersion $getCurrentExportedVersion, - private readonly ExportResource $exportResource, - private readonly LoggerInterface $logger + private readonly InvalidateExports $invalidateExports, ) { } @@ -34,26 +23,6 @@ public function __construct( */ public function execute(): void { - $currentExportedVersion = $this->getCurrentExportedVersion->execute(); - - /** @var Collection $collection */ - $collection = $this->collectionFactory->create(); - // need to check pending and uploaded exports only - $collection->addFieldToFilter(ExportInterface::FIELD_STATUS, ExportInterface::STATUS_PENDING); - $collection->addFieldToFilter(ExportInterface::FIELD_EXPORT_TYPE, ExportInterface::EXPORT_TYPE_INCREMENTAL); - /** @var Export[] $exports */ - $exports = $collection->getItems(); - foreach ($exports as $export) { - // mark incremental update as invalid if a more recent export has been uploaded already - if ($currentExportedVersion > $export->getVersionId()) { - $export->setStatus(ExportInterface::STATUS_INVALID); - try { - $this->exportResource->save($export); - } catch (\Exception $e) { - $message = sprintf('Error updating status of export %i', $export->getExportId()); - $this->logger->error($message, ['exception' => $e]); - } - } - } + $this->invalidateExports->execute(); } } diff --git a/src/export/Model/InvalidateExports.php b/src/export/Model/InvalidateExports.php new file mode 100644 index 0000000..c55f197 --- /dev/null +++ b/src/export/Model/InvalidateExports.php @@ -0,0 +1,59 @@ +getCurrentExportedVersion->execute(); + + /** @var Collection $collection */ + $collection = $this->collectionFactory->create(); + // need to check pending and uploaded exports only + $collection->addFieldToFilter(ExportInterface::FIELD_STATUS, ExportInterface::STATUS_PENDING); + $collection->addFieldToFilter(ExportInterface::FIELD_EXPORT_TYPE, ExportInterface::EXPORT_TYPE_INCREMENTAL); + /** @var Export[] $exports */ + $exports = $collection->getItems(); + foreach ($exports as $export) { + // mark incremental update as invalid if a more recent export has been uploaded already + if ($currentExportedVersion > $export->getVersionId()) { + $export->setStatus(ExportInterface::STATUS_INVALID); + try { + $this->exportResource->save($export); + } catch (\Exception $e) { + $message = sprintf('Error updating status of export %i', $export->getExportId()); + $this->logger->error($message, ['exception' => $e]); + } + } + } + } +} From f2ced60e2dcb449794209aedf2867a6771fa4bfe Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:10:15 +1030 Subject: [PATCH 6/9] Return early in the case of an error --- src/export/Model/DownloadQualityReport.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/export/Model/DownloadQualityReport.php b/src/export/Model/DownloadQualityReport.php index d6245f8..eca09c2 100644 --- a/src/export/Model/DownloadQualityReport.php +++ b/src/export/Model/DownloadQualityReport.php @@ -45,6 +45,7 @@ public function execute(string $directory, string $triggerId, bool $isSummary): } } catch (FileSystemException $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); + return null; } $qualityReportData = $this->dataIntegrationClient->getDataQualityReport($triggerId, $isSummary); From e5fa807610de4f0171548f5ba85efbedf1ebcde0 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:12:14 +1030 Subject: [PATCH 7/9] Restrict uploads to a single export - the latest one --- src/export/Cron/Upload.php | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/export/Cron/Upload.php b/src/export/Cron/Upload.php index e57f2bb..adbbdd6 100644 --- a/src/export/Cron/Upload.php +++ b/src/export/Cron/Upload.php @@ -5,6 +5,8 @@ use Aligent\FredhopperExport\Api\Data\ExportInterface; use Aligent\FredhopperExport\Model\Data\Export; +use Aligent\FredhopperExport\Model\Data\GetCurrentExportedVersion; +use Aligent\FredhopperExport\Model\GetExportIsInProgress; use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\Collection; use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\CollectionFactory; use Aligent\FredhopperExport\Model\UploadExport; @@ -14,10 +16,14 @@ class Upload /** * @param CollectionFactory $collectionFactory * @param UploadExport $uploadExport + * @param GetExportIsInProgress $getExportIsInProgress + * @param GetCurrentExportedVersion $getCurrentExportedVersion */ public function __construct( private readonly CollectionFactory $collectionFactory, - private readonly UploadExport $uploadExport + private readonly UploadExport $uploadExport, + private readonly GetExportIsInProgress $getExportIsInProgress, + private readonly GetCurrentExportedVersion $getCurrentExportedVersion ) { } @@ -28,13 +34,26 @@ public function __construct( */ public function execute(): void { + // don't upload any export when an export is in progress + if ($this->getExportIsInProgress->execute()) { + return; + } /** @var Collection $collection */ $collection = $this->collectionFactory->create(); - $collection->addFieldToFilter('status', ExportInterface::STATUS_PENDING); - /** @var Export[] $exports */ - $exports = $collection->getItems(); - foreach ($exports as $export) { - $this->uploadExport->execute($export); + $collection->addFieldToFilter(ExportInterface::FIELD_STATUS, ExportInterface::STATUS_PENDING); + // do not upload any export behind the current version + $currentVersion = $this->getCurrentExportedVersion->execute(); + if ($currentVersion > 0) { + $collection->addFieldToFilter(ExportInterface::FIELD_VERSION_ID, ['gt' => $currentVersion]); + } + // order by the version number descending to get the latest export + $collection->setOrder(ExportInterface::FIELD_VERSION_ID); + // we only ever want to upload a single export. Trying to upload multiple at once could prove troublesome + /** @var ExportInterface $firstExport */ + $firstExport = $collection->getFirstItem(); + if (!$firstExport->isEmpty()) { + /** @var Export $export */ + $this->uploadExport->execute($firstExport); } } } From 38bd6e74c8f6e3f7c6de8d2e8fbc71704c9e3c72 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:12:51 +1030 Subject: [PATCH 8/9] Only trigger an export if there is not already an export that has been triggered (but is not yet complete) --- src/export/Cron/Trigger.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/export/Cron/Trigger.php b/src/export/Cron/Trigger.php index 695faec..c25a50c 100644 --- a/src/export/Cron/Trigger.php +++ b/src/export/Cron/Trigger.php @@ -5,6 +5,7 @@ use Aligent\FredhopperExport\Api\Data\ExportInterface; use Aligent\FredhopperExport\Model\Data\Export; +use Aligent\FredhopperExport\Model\GetExportIsInProgress; use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\Collection; use Aligent\FredhopperExport\Model\ResourceModel\Data\Export\CollectionFactory; use Aligent\FredhopperExport\Model\TriggerDataLoad; @@ -14,10 +15,12 @@ class Trigger /** * @param CollectionFactory $exportCollectionFactory * @param TriggerDataLoad $triggerDataLoad + * @param GetExportIsInProgress $getExportIsInProgress */ public function __construct( private readonly CollectionFactory $exportCollectionFactory, - private readonly TriggerDataLoad $triggerDataLoad + private readonly TriggerDataLoad $triggerDataLoad, + private readonly GetExportIsInProgress $getExportIsInProgress ) { } @@ -28,6 +31,10 @@ public function __construct( */ public function execute(): void { + // don't trigger if an export has already been triggered + if ($this->getExportIsInProgress->execute(true)) { + return; + } /** @var Collection $collection */ $collection = $this->exportCollectionFactory->create(); $collection->addFieldToFilter(ExportInterface::FIELD_STATUS, ExportInterface::STATUS_UPLOADED); From cc6736bc8e4b83cd7f245951c8ec3f2d2212b085 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Thu, 21 Nov 2024 21:13:38 +1030 Subject: [PATCH 9/9] Invalidate exports after an update to the current version --- src/export/Model/UpdateExportDataStatus.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/export/Model/UpdateExportDataStatus.php b/src/export/Model/UpdateExportDataStatus.php index 5b44fa9..23a17eb 100644 --- a/src/export/Model/UpdateExportDataStatus.php +++ b/src/export/Model/UpdateExportDataStatus.php @@ -16,12 +16,14 @@ class UpdateExportDataStatus * @param DataIntegrationClient $dataIntegrationClient * @param ExportResource $exportResource * @param SetCurrentExport $setCurrentExport + * @param InvalidateExports $invalidateExports * @param LoggerInterface $logger */ public function __construct( private readonly DataIntegrationClient $dataIntegrationClient, private readonly ExportResource $exportResource, private readonly SetCurrentExport $setCurrentExport, + private readonly InvalidateExports $invalidateExports, private readonly LoggerInterface $logger ) { } @@ -68,6 +70,8 @@ public function execute(ExportInterface $export): void if ($dataStatus === ExportInterface::DATA_STATUS_SUCCESS) { $this->setCurrentExport->execute($export->getExportId()); + // invalidate any pending exports that have an earlier version than the current one + $this->invalidateExports->execute(); } } }