Minor CR fixes

PrestaShop · Oct 1, 2019 · 768b778 · 768b778
1 parent 6dc884c
commit 768b778
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 22 deletions.
diff --git a/src/Adapter/Attachment/CommandHandler/AbstractAttachmentHandler.php b/src/Adapter/Attachment/CommandHandler/AbstractAttachmentHandler.php
@@ -26,9 +26,11 @@
 
 namespace PrestaShop\PrestaShop\Adapter\Attachment\CommandHandler;
 
+use Attachment;
 use PrestaShop\PrestaShop\Core\ConstraintValidator\Constraints\CleanHtml;
 use PrestaShop\PrestaShop\Core\ConstraintValidator\Constraints\DefaultLanguage;
 use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentConstraintException;
+use PrestaShopException;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 
 /**
@@ -97,4 +99,19 @@ protected function getUniqueFileName(): string
 
         return $uniqueFileName;
     }
+
+    /**
+     * @param Attachment $attachment
+     * @throws AttachmentConstraintException
+     * @throws PrestaShopException
+     */
+    protected function assertValidFields(Attachment $attachment)
+    {
+        if (!$attachment->validateFields(false) && !$attachment->validateFieldsLang(false)) {
+            throw new AttachmentConstraintException(
+                'Attachment contains invalid field values',
+                AttachmentConstraintException::INVALID_FIELDS
+            );
+        }
+    }
 }
diff --git a/src/Adapter/Attachment/CommandHandler/AddAttachmentHandler.php b/src/Adapter/Attachment/CommandHandler/AddAttachmentHandler.php
@@ -77,22 +77,15 @@ public function handle(AddAttachmentCommand $command): AttachmentId
 
             $uniqueFileName = $this->getUniqueFileName();
 
-            $attachment->file_name = $command->getOriginalName();
-            $attachment->file = $uniqueFileName;
             $attachment->description = $command->getLocalizedDescriptions();
             $attachment->name = $command->getLocalizedNames();
+            $attachment->file_name = $command->getOriginalName();
+            $attachment->file = $uniqueFileName;
             $attachment->mime = $command->getMimeType();
 
-            if (!$attachment->validateFields(false) && !$attachment->validateFieldsLang(false)) {
-                throw new AttachmentConstraintException(
-                    'Attachment contains invalid field values',
-                    AttachmentConstraintException::INVALID_FIELDS
-                );
-            }
+            $this->assertValidFields($attachment);
 
-            if (null !== $command->getFilePathName()) {
-                $this->fileUploader->upload($command->getFilePathName(), $uniqueFileName, $command->getFileSize());
-            }
+            $this->fileUploader->upload($command->getFilePathName(), $uniqueFileName, $command->getFileSize());
 
             if (false === $attachment->add()) {
                 throw new CannotAddAttachmentException('Failed to add attachment');

diff --git a/src/Adapter/Attachment/CommandHandler/EditAttachmentHandler.php b/src/Adapter/Attachment/CommandHandler/EditAttachmentHandler.php
@@ -67,7 +67,6 @@ public function __construct(ValidatorInterface $validator, AttachmentFileUploade
      * @throws AttachmentException
      * @throws AttachmentNotFoundException
      * @throws CannotUpdateAttachmentException
-     * @throws FileNotFoundException
      */
     public function handle(EditAttachmentCommand $command)
     {
@@ -98,7 +97,6 @@ public function handle(EditAttachmentCommand $command)
      * @throws AttachmentException
      * @throws AttachmentNotFoundException
      * @throws CannotUpdateAttachmentException
-     * @throws FileNotFoundException
      */
     private function updateAttachmentFromCommandData(Attachment $attachment, EditAttachmentCommand $command)
     {
@@ -116,12 +114,7 @@ private function updateAttachmentFromCommandData(Attachment $attachment, EditAtt
             $attachment->description = $command->getLocalizedDescriptions();
             $attachment->name = $command->getLocalizedNames();
 
-            if (!$attachment->validateFields(false) && !$attachment->validateFieldsLang(false)) {
-                throw new AttachmentConstraintException(
-                    'Attachment contains invalid field values',
-                    AttachmentConstraintException::INVALID_FIELDS
-                );
-            }
+            $this->assertValidFields($attachment);
 
             if (null !== $command->getPathName()) {
                 $uniqueFileName = $this->getUniqueFileName();
@@ -130,6 +123,8 @@ private function updateAttachmentFromCommandData(Attachment $attachment, EditAtt
                 $attachment->file = $uniqueFileName;
                 $attachment->mime = $command->getMimeType();
 
+                $this->assertValidFields($attachment);
+
                 $this->fileUploader->upload(
                     $command->getPathName(),
                     $uniqueFileName,

diff --git a/src/Adapter/File/Uploader/AttachmentFileUploader.php b/src/Adapter/File/Uploader/AttachmentFileUploader.php
@@ -82,7 +82,7 @@ private function deleteOldFile(int $attachmentId): void
     {
         try {
             $attachment = new Attachment($attachmentId);
-            @unlink(_PS_DOWNLOAD_DIR_ . $attachment->file);
+            unlink(_PS_DOWNLOAD_DIR_ . $attachment->file);
         } catch (PrestaShopException $e) {
             throw new AttachmentNotFoundException(sprintf('Attachment with id "%s" was not found.', $attachmentId));
         }
@@ -104,13 +104,14 @@ private function uploadFile(string $filePath, string $uniqid, int $fileSize): vo
                     (string) ($this->configuration->get('PS_ATTACHMENT_MAXIMUM_SIZE') * 1024),
                     number_format(($fileSize / 1024), 2, '.', '')
                 ),
-             AttachmentConstraintException::INVALID_FILE_SIZE);
+                AttachmentConstraintException::INVALID_FILE_SIZE
+            );
         }
 
         try {
             move_uploaded_file($filePath, _PS_DOWNLOAD_DIR_ . $uniqid);
         } catch (FileException $e) {
-            throw new AttachmentUploadFailedException('Failed to copy the file.');
+            throw new AttachmentUploadFailedException(sprintf('Failed to copy the file %s.', $filePath));
         }
     }