fix #87 NoSuchEntityException on Export

thomas-kl1 · thomas-kl1 · commit f3f5c6c5ae49 · 2021-03-17T22:53:56.000+01:00
diff --git a/Api/ExportEntityManagementInterface.php b/Api/ExportEntityManagementInterface.php
@@ -11,6 +11,7 @@
 use Magento\Framework\Exception\CouldNotDeleteException;
 use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Opengento\Gdpr\Api\Data\ExportEntityInterface;
 
 /**
@@ -37,6 +38,7 @@ public function create(int $entityId, string $entityType, ?string $fileName = nu
      * @param ExportEntityInterface $exportEntity
      * @return ExportEntityInterface
      * @throws CouldNotSaveException
+     * @throws NoSuchEntityException
      * @throws LocalizedException
      */
     public function export(ExportEntityInterface $exportEntity): ExportEntityInterface;
diff --git a/Controller/Adminhtml/Privacy/MassExport.php b/Controller/Adminhtml/Privacy/MassExport.php
@@ -24,7 +24,7 @@
 use Opengento\Gdpr\Model\Action\ArgumentReader;
 use Opengento\Gdpr\Model\Action\ContextBuilder;
 use Opengento\Gdpr\Model\Action\Export\ArgumentReader as ExportArgumentReader;
-use Opengento\Gdpr\Model\Archive\MoveToArchive;
+use Opengento\Gdpr\Model\Archive\ArchiveManager;
 
 class MassExport extends AbstractMassAction
 {
@@ -36,9 +36,9 @@ class MassExport extends AbstractMassAction
     private $fileFactory;
 
     /**
-     * @var MoveToArchive
+     * @var ArchiveManager
      */
-    private $moveToArchive;
+    private $archiveManager;
 
     /**
      * @var ActionInterface
@@ -55,12 +55,12 @@ public function __construct(
         Filter $filter,
         CollectionFactory $collectionFactory,
         FileFactory $fileFactory,
-        MoveToArchive $moveToArchive,
+        ArchiveManager $archiveManager,
         ActionInterface $action,
         ContextBuilder $actionContextBuilder
     ) {
         $this->fileFactory = $fileFactory;
-        $this->moveToArchive = $moveToArchive;
+        $this->archiveManager = $archiveManager;
         $this->action = $action;
         $this->actionContextBuilder = $actionContextBuilder;
         parent::__construct($context, $filter, $collectionFactory);
@@ -79,7 +79,7 @@ protected function massAction(AbstractCollection $collection)
                 $result = $this->action->execute($this->actionContextBuilder->create())->getResult();
                 /** @var ExportEntityInterface $exportEntity */
                 $exportEntity = $result[ExportArgumentReader::EXPORT_ENTITY];
-                $this->moveToArchive->prepareArchive($exportEntity->getFilePath(), $archiveFileName, false);
+                $this->archiveManager->addToArchive($exportEntity->getFilePath(), $archiveFileName, false);
             }
 
             return $this->fileFactory->create(
diff --git a/Cron/ExportEntity.php b/Cron/ExportEntity.php
@@ -9,6 +9,7 @@
 
 use Exception;
 use Magento\Framework\Api\SearchCriteriaBuilder;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Opengento\Gdpr\Api\Data\ExportEntityInterface;
 use Opengento\Gdpr\Api\ExportEntityManagementInterface;
 use Opengento\Gdpr\Api\ExportEntityRepositoryInterface;
@@ -69,10 +70,15 @@ public function execute(): void
                 $exportList = $this->exportRepository->getList($this->criteriaBuilder->create());
 
                 foreach ($exportList->getItems() as $exportEntity) {
-                    $this->exportManagement->export($exportEntity);
+                    try {
+                        $this->exportManagement->export($exportEntity);
+                    } catch (NoSuchEntityException $e) {
+                        $this->logger->error($e->getLogMessage(), $e->getTrace());
+                        $this->exportRepository->delete($exportEntity);
+                    }
                 }
             } catch (Exception $e) {
-                $this->logger->error($e->getMessage(), $e->getTrace());
+                $this->logger->critical($e->getMessage(), $e->getTrace());
             }
         }
     }
diff --git a/Model/Action/Export/ExportAction.php b/Model/Action/Export/ExportAction.php
@@ -8,24 +8,33 @@
 namespace Opengento\Gdpr\Model\Action\Export;
 
 use Magento\Framework\Exception\InputException;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Opengento\Gdpr\Api\Data\ActionContextInterface;
 use Opengento\Gdpr\Api\Data\ActionResultInterface;
 use Opengento\Gdpr\Api\ExportEntityManagementInterface;
+use Opengento\Gdpr\Api\ExportEntityRepositoryInterface;
 use Opengento\Gdpr\Model\Action\AbstractAction;
 use Opengento\Gdpr\Model\Action\Export\ArgumentReader as ExportArgumentReader;
 use Opengento\Gdpr\Model\Action\ResultBuilder;
 
 final class ExportAction extends AbstractAction
 {
+    /**
+     * @var ExportEntityRepositoryInterface
+     */
+    private $exportRepository;
+
     /**
      * @var ExportEntityManagementInterface
      */
     private $exportManagement;
 
     public function __construct(
         ResultBuilder $resultBuilder,
+        ExportEntityRepositoryInterface $exportRepository,
         ExportEntityManagementInterface $exportManagement
     ) {
+        $this->exportRepository = $exportRepository;
         $this->exportManagement = $exportManagement;
         parent::__construct($resultBuilder);
     }
@@ -38,8 +47,16 @@ public function execute(ActionContextInterface $actionContext): ActionResultInte
             throw InputException::requiredField('entity');
         }
 
+        try {
+            $exportEntity = $this->exportManagement->export($exportEntity);
+        } catch (NoSuchEntityException $e) {
+            $this->exportRepository->delete($exportEntity);
+
+            throw $e;
+        }
+
         return $this->createActionResult(
-            [ExportArgumentReader::EXPORT_ENTITY => $this->exportManagement->export($exportEntity)]
+            [ExportArgumentReader::EXPORT_ENTITY => $exportEntity]
         );
     }
 }
diff --git a/Model/Archive/ArchiveManager.php b/Model/Archive/ArchiveManager.php
@@ -14,7 +14,7 @@
 use Magento\Framework\Filesystem;
 use Magento\Framework\Phrase;
 
-final class MoveToArchive
+final class ArchiveManager
 {
     /**
      * @var ArchiveInterface
@@ -42,7 +42,7 @@ public function __construct(
      * @throws FileSystemException
      * @throws NotFoundException
      */
-    public function prepareArchive(string $source, string $destination, bool $remove = true): string
+    public function addToArchive(string $source, string $destination, bool $remove = true): string
     {
         $tmpWrite = $this->filesystem->getDirectoryWrite(DirectoryList::TMP);
         $fileDriver = $tmpWrite->getDriver();
diff --git a/Model/Export/ExportEntityData.php b/Model/Export/ExportEntityData.php
@@ -64,8 +64,16 @@ public function export(int $entityId, string $entityType): ExportEntityInterface
             $exportEntity = $this->exportManagement->create($entityId, $entityType);
         }
 
-        return $this->exportEntityChecker->isExported($entityId, $entityType)
-            ? $exportEntity
-            : $this->exportManagement->export($exportEntity);
+        if (!$this->exportEntityChecker->isExported($entityId, $entityType)) {
+            try {
+                $exportEntity = $this->exportManagement->export($exportEntity);
+            } catch (NoSuchEntityException $e) {
+                $this->exportRepository->delete($exportEntity);
+
+                throw $e;
+            }
+        }
+
+        return $exportEntity;
     }
 }
diff --git a/Model/Export/ExportToFile.php b/Model/Export/ExportToFile.php
@@ -9,10 +9,11 @@
 
 use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\Exception\FileSystemException;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Exception\NotFoundException;
 use Magento\Store\Model\ScopeInterface;
 use Opengento\Gdpr\Api\Data\ExportEntityInterface;
-use Opengento\Gdpr\Model\Archive\MoveToArchive;
+use Opengento\Gdpr\Model\Archive\ArchiveManager;
 use Opengento\Gdpr\Service\Export\ProcessorFactory;
 use Opengento\Gdpr\Service\Export\RendererFactory;
 use function explode;
@@ -34,9 +35,9 @@ final class ExportToFile
     private $rendererFactory;
 
     /**
-     * @var MoveToArchive
+     * @var ArchiveManager
      */
-    private $archive;
+    private $archiveManager;
 
     /**
      * @var ScopeConfigInterface
@@ -46,12 +47,12 @@ final class ExportToFile
     public function __construct(
         ProcessorFactory $processorFactory,
         RendererFactory $rendererFactory,
-        MoveToArchive $archive,
+        ArchiveManager $archiveManager,
         ScopeConfigInterface $scopeConfig
     ) {
         $this->processorFactory = $processorFactory;
         $this->rendererFactory = $rendererFactory;
-        $this->archive = $archive;
+        $this->archiveManager = $archiveManager;
         $this->scopeConfig = $scopeConfig;
     }
 
@@ -60,20 +61,22 @@ public function __construct(
      * @return string|null
      * @throws FileSystemException
      * @throws NotFoundException
+     * @throws NoSuchEntityException
      */
-    public function export(ExportEntityInterface $exportEntity): ?string
+    public function export(ExportEntityInterface $exportEntity): string
     {
-        $exporter = $this->processorFactory->get($exportEntity->getEntityType());
         $fileName = $this->prepareFileName($exportEntity);
-        $data = $exporter->execute($exportEntity->getEntityId(), []);
+        $archiveFileName = $fileName . 'zip';
+        $data = $this->processorFactory->get($exportEntity->getEntityType())->execute($exportEntity->getEntityId(), []);
+
         foreach ($this->resolveExportRendererCodes() as $rendererCode) {
-            $filePath = $this->archive->prepareArchive(
+            $this->archiveManager->addToArchive(
                 $this->rendererFactory->get($rendererCode)->saveData($fileName, $data),
-                $fileName . '.zip'
+                $archiveFileName
             );
         }
 
-        return $filePath ?? null;
+        return $archiveFileName;
     }
 
     public function resolveExportRendererCodes(): array
diff --git a/Model/ExportEntityManagement.php b/Model/ExportEntityManagement.php
@@ -11,6 +11,7 @@
 use Exception;
 use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\Exception\AlreadyExistsException;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Phrase;
 use Magento\Framework\Stdlib\DateTime as DateTimeFormat;
 use Magento\Store\Model\ScopeInterface;
diff --git a/Observer/DeleteExport.php b/Observer/DeleteExport.php
@@ -0,0 +1,119 @@
+<?php
+/**
+ * Copyright © OpenGento, All rights reserved.
+ * See LICENSE bundled with this library for license details.
+ */
+declare(strict_types=1);
+
+namespace Opengento\Gdpr\Observer;
+
+use Exception;
+use Magento\Framework\Api\Filter;
+use Magento\Framework\Api\FilterBuilder;
+use Magento\Framework\Api\SearchCriteriaBuilder;
+use Magento\Framework\Api\SearchResultsInterface;
+use Magento\Framework\Event\Observer;
+use Magento\Framework\Event\ObserverInterface;
+use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\Model\AbstractModel;
+use Opengento\Gdpr\Api\Data\ExportEntityInterface;
+use Opengento\Gdpr\Api\Data\ExportEntitySearchResultsInterface;
+use Opengento\Gdpr\Api\ExportEntityRepositoryInterface;
+use Opengento\Gdpr\Model\Entity\EntityTypeResolver;
+use Psr\Log\LoggerInterface;
+
+final class DeleteExport implements ObserverInterface
+{
+    /**
+     * @var ExportEntityRepositoryInterface
+     */
+    private $exportRepository;
+
+    /**
+     * @var SearchCriteriaBuilder
+     */
+    private $criteriaBuilder;
+
+    /**
+     * @var FilterBuilder
+     */
+    private $filterBuilder;
+
+    /**
+     * @var EntityTypeResolver
+     */
+    private $entityTypeResolver;
+
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(
+        ExportEntityRepositoryInterface $exportRepository,
+        SearchCriteriaBuilder $criteriaBuilder,
+        FilterBuilder $filterBuilder,
+        EntityTypeResolver $entityTypeResolver,
+        LoggerInterface $logger
+    ) {
+        $this->exportRepository = $exportRepository;
+        $this->criteriaBuilder = $criteriaBuilder;
+        $this->filterBuilder = $filterBuilder;
+        $this->entityTypeResolver = $entityTypeResolver;
+        $this->logger = $logger;
+    }
+
+    public function execute(Observer $observer): void
+    {
+        /** @var AbstractModel $entity */
+        $entity = $observer->getData('data_object');
+
+        try {
+            foreach ($this->fetchExportEntities($entity)->getItems() as $exportEntity) {
+                $this->exportRepository->delete($exportEntity);
+            }
+        } catch (LocalizedException $e) {
+            $this->logger->error($e->getLogMessage(), $e->getTrace());
+        } catch (Exception $e) {
+            $this->logger->error($e->getMessage(), $e->getTrace());
+        }
+    }
+
+    /**
+     * @param AbstractModel $entity
+     * @return ExportEntitySearchResultsInterface
+     * @throws LocalizedException
+     * @throws Exception
+     */
+    private function fetchExportEntities(AbstractModel $entity): SearchResultsInterface
+    {
+        $entityTypes = $this->entityTypeResolver->resolve($entity);
+
+        foreach ($entityTypes as $entityType => $idFieldName) {
+            $this->criteriaBuilder->addFilters([
+                $this->createEntityIdFilter((int) $entity->getData($idFieldName)),
+                $this->createEntityTypeFilter($entityType)
+            ]);
+        }
+
+        return $this->exportRepository->getList($this->criteriaBuilder->create());
+    }
+
+    private function createEntityIdFilter(int $entityId): Filter
+    {
+        $this->filterBuilder->setField(ExportEntityInterface::ENTITY_ID);
+        $this->filterBuilder->setValue($entityId);
+        $this->filterBuilder->setConditionType('eq');
+
+        return $this->filterBuilder->create();
+    }
+
+    private function createEntityTypeFilter(string $entityType): Filter
+    {
+        $this->filterBuilder->setField(ExportEntityInterface::ENTITY_TYPE);
+        $this->filterBuilder->setValue($entityType);
+        $this->filterBuilder->setConditionType('eq');
+
+        return $this->filterBuilder->create();
+    }
+}
diff --git a/Observer/InvalidateExport.php b/Observer/InvalidateExport.php
@@ -103,6 +103,8 @@ private function fetchExportEntities(AbstractModel $entity): SearchResultsInterf
                 $this->createEntityTypeFilter($entityType)
             ]);
         }
+        $this->criteriaBuilder->addFilter(ExportEntityInterface::EXPORTED_AT, true, 'notnull');
+        $this->criteriaBuilder->addFilter(ExportEntityInterface::FILE_PATH, true, 'notnull');
 
         return $this->exportRepository->getList($this->criteriaBuilder->create());
     }
diff --git a/Service/Export/ProcessorInterface.php b/Service/Export/ProcessorInterface.php
diff --git a/etc/events.xml b/etc/events.xml
diff --git a/view/base/page_layout/customer_privacy_export.xml b/view/base/page_layout/customer_privacy_export.xml

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@`
`9`	`9`
`10`	`10`	`use Exception;`
`11`	`11`	`use Magento\Framework\Api\SearchCriteriaBuilder;`
	`12`	`+use Magento\Framework\Exception\NoSuchEntityException;`
`12`	`13`	`use Opengento\Gdpr\Api\Data\ExportEntityInterface;`
`13`	`14`	`use Opengento\Gdpr\Api\ExportEntityManagementInterface;`
`14`	`15`	`use Opengento\Gdpr\Api\ExportEntityRepositoryInterface;`
`@@ -69,10 +70,15 @@ public function execute(): void`
`69`	`70`	`$exportList = $this->exportRepository->getList($this->criteriaBuilder->create());`
`70`	`71`
`71`	`72`	`foreach ($exportList->getItems() as $exportEntity) {`
`72`		`- $this->exportManagement->export($exportEntity);`
	`73`	`+ try {`
	`74`	`+ $this->exportManagement->export($exportEntity);`
	`75`	`+ } catch (NoSuchEntityException $e) {`
	`76`	`+ $this->logger->error($e->getLogMessage(), $e->getTrace());`
	`77`	`+ $this->exportRepository->delete($exportEntity);`
	`78`	`+ }`
`73`	`79`	`}`
`74`	`80`	`} catch (Exception $e) {`
`75`		`- $this->logger->error($e->getMessage(), $e->getTrace());`
	`81`	`+ $this->logger->critical($e->getMessage(), $e->getTrace());`
`76`	`82`	`}`
`77`	`83`	`}`
`78`	`84`	`}`
Original file line number	Diff line number	Diff line change
`@@ -64,8 +64,16 @@ public function export(int $entityId, string $entityType): ExportEntityInterface`
`64`	`64`	`$exportEntity = $this->exportManagement->create($entityId, $entityType);`
`65`	`65`	`}`
`66`	`66`
`67`		`- return $this->exportEntityChecker->isExported($entityId, $entityType)`
`68`		`- ? $exportEntity`
`69`		`- : $this->exportManagement->export($exportEntity);`
	`67`	`+ if (!$this->exportEntityChecker->isExported($entityId, $entityType)) {`
	`68`	`+ try {`
	`69`	`+ $exportEntity = $this->exportManagement->export($exportEntity);`
	`70`	`+ } catch (NoSuchEntityException $e) {`
	`71`	`+ $this->exportRepository->delete($exportEntity);`
	`72`	`+`
	`73`	`+ throw $e;`
	`74`	`+ }`
	`75`	`+ }`
	`76`	`+`
	`77`	`+ return $exportEntity;`
`70`	`78`	`}`
`71`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,6 +103,8 @@ private function fetchExportEntities(AbstractModel $entity): SearchResultsInterf`
`103`	`103`	`$this->createEntityTypeFilter($entityType)`
`104`	`104`	`]);`
`105`	`105`	`}`
	`106`	`+ $this->criteriaBuilder->addFilter(ExportEntityInterface::EXPORTED_AT, true, 'notnull');`
	`107`	`+ $this->criteriaBuilder->addFilter(ExportEntityInterface::FILE_PATH, true, 'notnull');`
`106`	`108`
`107`	`109`	`return $this->exportRepository->getList($this->criteriaBuilder->create());`
`108`	`110`	`}`