From 1d2444cb35b81dc8577c801cd51e402c4c90777e Mon Sep 17 00:00:00 2001 From: Serhii Voloshkov Date: Tue, 7 May 2019 12:57:08 +0300 Subject: [PATCH 1/2] MAGETWO-98146: Downloadable Product Controller|API save changes --- .../Helper/Plugin/Downloadable.php | 84 ++++++++++++++++-- .../Downloadable/Model/Link/Builder.php | 3 +- .../Model/Link/ContentValidator.php | 80 +++++++++++------ .../Downloadable/Model/LinkRepository.php | 88 ++++++++++++++----- .../Model/Sample/ContentValidator.php | 55 +++++++++--- .../Downloadable/Model/SampleRepository.php | 38 ++++---- .../Helper/Plugin/DownloadableTest.php | 22 ++++- .../Unit/Model/Link/ContentValidatorTest.php | 25 +++++- .../Test/Unit/Model/LinkRepositoryTest.php | 5 +- .../Model/Sample/ContentValidatorTest.php | 25 +++++- .../Test/Unit/Model/SampleRepositoryTest.php | 2 + .../Product/Form/Modifier/Data/Links.php | 20 ++++- .../Product/Form/Modifier/Data/Samples.php | 18 +++- .../Downloadable/Api/LinkRepositoryTest.php | 65 ++++++++++++++ .../Api/ProductRepositoryTest.php | 4 +- .../Downloadable/Api/SampleRepositoryTest.php | 29 ++++++ 16 files changed, 472 insertions(+), 91 deletions(-) diff --git a/app/code/Magento/Downloadable/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Downloadable.php b/app/code/Magento/Downloadable/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Downloadable.php index a283891afc406..e7d963329826a 100644 --- a/app/code/Magento/Downloadable/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Downloadable.php +++ b/app/code/Magento/Downloadable/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Downloadable.php @@ -5,11 +5,15 @@ */ namespace Magento\Downloadable\Controller\Adminhtml\Product\Initialization\Helper\Plugin; -use Magento\Framework\App\RequestInterface; +use Magento\Downloadable\Api\Data\LinkInterfaceFactory; +use Magento\Downloadable\Api\Data\SampleInterfaceFactory; +use Magento\Downloadable\Helper\Download; +use Magento\Downloadable\Model\Link; use Magento\Downloadable\Model\Link\Builder as LinkBuilder; +use Magento\Downloadable\Model\Product\Type; +use Magento\Downloadable\Model\ResourceModel\Sample\Collection; use Magento\Downloadable\Model\Sample\Builder as SampleBuilder; -use Magento\Downloadable\Api\Data\SampleInterfaceFactory; -use Magento\Downloadable\Api\Data\LinkInterfaceFactory; +use Magento\Framework\App\RequestInterface; /** * Class for initialization downloadable info from request. @@ -42,8 +46,6 @@ class Downloadable private $linkBuilder; /** - * Constructor - * * @param RequestInterface $request * @param LinkBuilder $linkBuilder * @param SampleBuilder $sampleBuilder @@ -79,14 +81,20 @@ public function afterInitialize( \Magento\Catalog\Model\Product $product ) { if ($downloadable = $this->request->getPost('downloadable')) { + $product->setTypeId(Type::TYPE_DOWNLOADABLE); $product->setDownloadableData($downloadable); $extension = $product->getExtensionAttributes(); + /** @var \Magento\Downloadable\Model\Product\Type $type */ + $type = $product->getTypeInstance(); + $productLinks = $type->getLinks($product); + $productSamples = $type->getSamples($product); if (isset($downloadable['link']) && is_array($downloadable['link'])) { $links = []; foreach ($downloadable['link'] as $linkData) { if (!$linkData || (isset($linkData['is_delete']) && $linkData['is_delete'])) { continue; } else { + $linkData = $this->processLink($linkData, $productLinks); $links[] = $this->linkBuilder->setData( $linkData )->build( @@ -104,6 +112,7 @@ public function afterInitialize( if (!$sampleData || (isset($sampleData['is_delete']) && (bool)$sampleData['is_delete'])) { continue; } else { + $sampleData = $this->processSample($sampleData, $productSamples); $samples[] = $this->sampleBuilder->setData( $sampleData )->build( @@ -124,4 +133,69 @@ public function afterInitialize( } return $product; } + + /** + * Check Links type and status. + * + * @param array $linkData + * @param Link[] $productLinks + * @return array + */ + private function processLink(array $linkData, array $productLinks): array + { + $linkId = $linkData['link_id'] ?? null; + if ($linkId && isset($productLinks[$linkId])) { + $linkData = $this->processFileStatus($linkData, $productLinks[$linkId]->getLinkFile()); + $linkData['sample'] = $this->processFileStatus( + $linkData['sample'] ?? [], + $productLinks[$linkId]->getSampleFile() + ); + } else { + $linkData = $this->processFileStatus($linkData, null); + $linkData['sample'] = $this->processFileStatus($linkData['sample'] ?? [], null); + } + + return $linkData; + } + + /** + * Check Sample type and status. + * + * @param array $sampleData + * @param Collection $productSamples + * @return array + */ + private function processSample(array $sampleData, Collection $productSamples): array + { + $sampleId = $sampleData['sample_id'] ?? null; + /** @var \Magento\Downloadable\Model\Sample $productSample */ + $productSample = $sampleId ? $productSamples->getItemById($sampleId) : null; + if ($sampleId && $productSample) { + $sampleData = $this->processFileStatus($sampleData, $productSample->getSampleFile()); + } else { + $sampleData = $this->processFileStatus($sampleData, null); + } + + return $sampleData; + } + + /** + * Compare file path from request with DB and set status. + * + * @param array $data + * @param string|null $file + * @return array + */ + private function processFileStatus(array $data, $file): array + { + if (isset($data['type']) && $data['type'] === Download::LINK_TYPE_FILE && isset($data['file']['0']['file'])) { + if ($data['file'][0]['file'] !== $file) { + $data['file'][0]['status'] = 'new'; + } else { + $data['file'][0]['status'] = 'old'; + } + } + + return $data; + } } diff --git a/app/code/Magento/Downloadable/Model/Link/Builder.php b/app/code/Magento/Downloadable/Model/Link/Builder.php index 83d01f76fe9cd..a977a4166f0fb 100644 --- a/app/code/Magento/Downloadable/Model/Link/Builder.php +++ b/app/code/Magento/Downloadable/Model/Link/Builder.php @@ -174,7 +174,8 @@ private function buildSample(\Magento\Downloadable\Api\Data\LinkInterface $link, ), \Magento\Downloadable\Api\Data\LinkInterface::class ); - if ($link->getSampleType() === \Magento\Downloadable\Helper\Download::LINK_TYPE_FILE) { + if ($link->getSampleType() === \Magento\Downloadable\Helper\Download::LINK_TYPE_FILE + && isset($sample['file'])) { $linkSampleFileName = $this->downloadableFile->moveFileFromTmp( $this->getComponent()->getBaseSampleTmpPath(), $this->getComponent()->getBaseSamplePath(), diff --git a/app/code/Magento/Downloadable/Model/Link/ContentValidator.php b/app/code/Magento/Downloadable/Model/Link/ContentValidator.php index 088356caefad0..8497bf7de6592 100644 --- a/app/code/Magento/Downloadable/Model/Link/ContentValidator.php +++ b/app/code/Magento/Downloadable/Model/Link/ContentValidator.php @@ -6,10 +6,16 @@ namespace Magento\Downloadable\Model\Link; use Magento\Downloadable\Api\Data\LinkInterface; +use Magento\Downloadable\Helper\File; use Magento\Downloadable\Model\File\ContentValidator as FileContentValidator; use Magento\Framework\Exception\InputException; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Exception\ValidatorException; use Magento\Framework\Url\Validator as UrlValidator; +/** + * Class to validate Link Content. + */ class ContentValidator { /** @@ -22,20 +28,28 @@ class ContentValidator */ protected $urlValidator; + /** + * @var File + */ + private $fileHelper; + /** * @param FileContentValidator $fileContentValidator * @param UrlValidator $urlValidator + * @param File|null $fileHelper */ public function __construct( FileContentValidator $fileContentValidator, - UrlValidator $urlValidator + UrlValidator $urlValidator, + File $fileHelper = null ) { $this->fileContentValidator = $fileContentValidator; $this->urlValidator = $urlValidator; + $this->fileHelper = $fileHelper ?? ObjectManager::getInstance()->get(File::class); } /** - * Check if link content is valid + * Check if link content is valid. * * @param LinkInterface $link * @param bool $validateLinkContent @@ -63,50 +77,66 @@ public function isValid(LinkInterface $link, $validateLinkContent = true, $valid if ($validateSampleContent) { $this->validateSampleResource($link); } + return true; } /** - * Validate link resource (file or URL) + * Validate link resource (file or URL). * * @param LinkInterface $link - * @throws InputException * @return void + * @throws InputException */ protected function validateLinkResource(LinkInterface $link) { - if ($link->getLinkType() == 'url' - && !$this->urlValidator->isValid($link->getLinkUrl()) - ) { - throw new InputException(__('Link URL must have valid format.')); - } - if ($link->getLinkType() == 'file' - && (!$link->getLinkFileContent() - || !$this->fileContentValidator->isValid($link->getLinkFileContent())) - ) { - throw new InputException(__('Provided file content must be valid base64 encoded data.')); + if ($link->getLinkType() === 'url') { + if (!$this->urlValidator->isValid($link->getLinkUrl())) { + throw new InputException(__('Link URL must have valid format.')); + } + } elseif ($link->getLinkFileContent()) { + if (!$this->fileContentValidator->isValid($link->getLinkFileContent())) { + throw new InputException(__('Provided file content must be valid base64 encoded data.')); + } + } elseif (!$this->isFileValid($link->getBasePath() . $link->getLinkFile())) { + throw new InputException(__('Link file not found. Please try again.')); } } /** - * Validate sample resource (file or URL) + * Validate sample resource (file or URL). * * @param LinkInterface $link - * @throws InputException * @return void + * @throws InputException */ protected function validateSampleResource(LinkInterface $link) { - if ($link->getSampleType() == 'url' - && !$this->urlValidator->isValid($link->getSampleUrl()) - ) { - throw new InputException(__('Sample URL must have valid format.')); + if ($link->getSampleType() === 'url') { + if (!$this->urlValidator->isValid($link->getSampleUrl())) { + throw new InputException(__('Sample URL must have valid format.')); + } + } elseif ($link->getSampleFileContent()) { + if (!$this->fileContentValidator->isValid($link->getSampleFileContent())) { + throw new InputException(__('Provided file content must be valid base64 encoded data.')); + } + } elseif (!$this->isFileValid($link->getBaseSamplePath() . $link->getSampleFile())) { + throw new InputException(__('Link sample file not found. Please try again.')); } - if ($link->getSampleType() == 'file' - && (!$link->getSampleFileContent() - || !$this->fileContentValidator->isValid($link->getSampleFileContent())) - ) { - throw new InputException(__('Provided file content must be valid base64 encoded data.')); + } + + /** + * Check that Links File or Sample is valid. + * + * @param string $file + * @return bool + */ + private function isFileValid(string $file): bool + { + try { + return $this->fileHelper->ensureFileInFilesystem($file); + } catch (ValidatorException $e) { + return false; } } } diff --git a/app/code/Magento/Downloadable/Model/LinkRepository.php b/app/code/Magento/Downloadable/Model/LinkRepository.php index b84a8284c9083..31a5b990ae426 100644 --- a/app/code/Magento/Downloadable/Model/LinkRepository.php +++ b/app/code/Magento/Downloadable/Model/LinkRepository.php @@ -165,9 +165,11 @@ protected function setBasicFields($resourceData, $dataObject) } /** - * {@inheritdoc} + * @inheritdoc + * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * @throws InputException */ public function save($sku, LinkInterface $link, $isGlobalScopeContent = true) { @@ -175,27 +177,26 @@ public function save($sku, LinkInterface $link, $isGlobalScopeContent = true) if ($link->getId() !== null) { return $this->updateLink($product, $link, $isGlobalScopeContent); } else { - if ($product->getTypeId() !== \Magento\Downloadable\Model\Product\Type::TYPE_DOWNLOADABLE) { + if ($product->getTypeId() !== Type::TYPE_DOWNLOADABLE) { throw new InputException(__('Provided product must be type \'downloadable\'.')); } - $validateLinkContent = !($link->getLinkType() === 'file' && $link->getLinkFile()); - $validateSampleContent = !($link->getSampleType() === 'file' && $link->getSampleFile()); - if (!$this->contentValidator->isValid($link, $validateLinkContent, $validateSampleContent)) { + $this->validateLinkType($link); + $this->validateSampleType($link); + if (!$this->contentValidator->isValid($link, true, $link->hasSampleType())) { throw new InputException(__('Provided link information is invalid.')); } - - if (!in_array($link->getLinkType(), ['url', 'file'], true)) { - throw new InputException(__('Invalid link type.')); - } $title = $link->getTitle(); if (empty($title)) { throw new InputException(__('Link title cannot be empty.')); } + return $this->saveLink($product, $link, $isGlobalScopeContent); } } /** + * Construct Data structure and Save it. + * * @param \Magento\Catalog\Api\Data\ProductInterface $product * @param LinkInterface $link * @param bool $isGlobalScopeContent @@ -217,7 +218,7 @@ protected function saveLink( 'is_shareable' => $link->getIsShareable(), ]; - if ($link->getLinkType() == 'file' && $link->getLinkFile() === null) { + if ($link->getLinkType() == 'file' && $link->getLinkFileContent() !== null) { $linkData['file'] = $this->jsonEncoder->encode( [ $this->fileContentUploader->upload($link->getLinkFileContent(), 'link_file'), @@ -239,7 +240,7 @@ protected function saveLink( if ($link->getSampleType() == 'file') { $linkData['sample']['type'] = 'file'; - if ($link->getSampleFile() === null) { + if ($link->getSampleFileContent() !== null) { $fileData = [ $this->fileContentUploader->upload($link->getSampleFileContent(), 'link_sample_file'), ]; @@ -266,6 +267,8 @@ protected function saveLink( } /** + * Update existing Link. + * * @param \Magento\Catalog\Api\Data\ProductInterface $product * @param LinkInterface $link * @param bool $isGlobalScopeContent @@ -291,9 +294,10 @@ protected function updateLink( if ($existingLink->getProductId() != $linkFieldValue) { throw new InputException(__('Provided downloadable link is not related to given product.')); } - $validateLinkContent = !($link->getLinkFileContent() === null); - $validateSampleContent = !($link->getSampleFileContent() === null); - if (!$this->contentValidator->isValid($link, $validateLinkContent, $validateSampleContent)) { + $this->validateLinkType($link); + $this->validateSampleType($link); + $validateSampleContent = $link->hasSampleType(); + if (!$this->contentValidator->isValid($link, true, $validateSampleContent)) { throw new InputException(__('Provided link information is invalid.')); } if ($isGlobalScopeContent) { @@ -305,15 +309,11 @@ protected function updateLink( throw new InputException(__('Link title cannot be empty.')); } } - - if ($link->getLinkType() == 'file' && $link->getLinkFileContent() === null && !$link->getLinkFile()) { - $link->setLinkFile($existingLink->getLinkFile()); - } - if ($link->getSampleType() == 'file' && $link->getSampleFileContent() === null && !$link->getSampleFile()) { - $link->setSampleFile($existingLink->getSampleFile()); + if (!$validateSampleContent) { + $this->resetLinkSampleContent($link, $existingLink); } - $this->saveLink($product, $link, $isGlobalScopeContent); + return $existingLink->getId(); } @@ -335,6 +335,52 @@ public function delete($id) return true; } + /** + * Check that Link type exist. + * + * @param LinkInterface $link + * @return void + * @throws InputException + */ + private function validateLinkType(LinkInterface $link) + { + if (!in_array($link->getLinkType(), ['url', 'file'], true)) { + throw new InputException(__('Invalid link type.')); + } + } + + /** + * Check that Link sample type exist. + * + * @param LinkInterface $link + * @return void + * @throws InputException + */ + private function validateSampleType(LinkInterface $link) + { + if ($link->hasSampleType() && !in_array($link->getSampleType(), ['url', 'file'], true)) { + throw new InputException(__('Invalid link sample type.')); + } + } + + /** + * Reset Sample type and file. + * + * @param LinkInterface $link + * @param LinkInterface $existingLink + * @return void + */ + private function resetLinkSampleContent(LinkInterface $link, LinkInterface $existingLink) + { + $existingType = $existingLink->getSampleType(); + $link->setSampleType($existingType); + if ($existingType === 'file') { + $link->setSampleFile($existingLink->getSampleFile()); + } else { + $link->setSampleUrl($existingLink->getSampleUrl()); + } + } + /** * Get MetadataPool instance * diff --git a/app/code/Magento/Downloadable/Model/Sample/ContentValidator.php b/app/code/Magento/Downloadable/Model/Sample/ContentValidator.php index 6a273bfe5d34e..7348b04793a8f 100644 --- a/app/code/Magento/Downloadable/Model/Sample/ContentValidator.php +++ b/app/code/Magento/Downloadable/Model/Sample/ContentValidator.php @@ -6,10 +6,16 @@ namespace Magento\Downloadable\Model\Sample; use Magento\Downloadable\Api\Data\SampleInterface; +use Magento\Downloadable\Helper\File; use Magento\Downloadable\Model\File\ContentValidator as FileContentValidator; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Exception\InputException; +use Magento\Framework\Exception\ValidatorException; use Magento\Framework\Url\Validator as UrlValidator; +/** + * Class to validate Sample Content. + */ class ContentValidator { /** @@ -22,20 +28,28 @@ class ContentValidator */ protected $fileContentValidator; + /** + * @var File + */ + private $fileHelper; + /** * @param FileContentValidator $fileContentValidator * @param UrlValidator $urlValidator + * @param File|null $fileHelper */ public function __construct( FileContentValidator $fileContentValidator, - UrlValidator $urlValidator + UrlValidator $urlValidator, + File $fileHelper = null ) { $this->fileContentValidator = $fileContentValidator; $this->urlValidator = $urlValidator; + $this->fileHelper = $fileHelper ?? ObjectManager::getInstance()->get(File::class); } /** - * Check if sample content is valid + * Check if sample content is valid. * * @param SampleInterface $sample * @param bool $validateSampleContent @@ -51,29 +65,44 @@ public function isValid(SampleInterface $sample, $validateSampleContent = true) if ($validateSampleContent) { $this->validateSampleResource($sample); } + return true; } /** - * Validate sample resource (file or URL) + * Validate sample resource (file or URL). * * @param SampleInterface $sample - * @throws InputException * @return void + * @throws InputException */ protected function validateSampleResource(SampleInterface $sample) { - $sampleFile = $sample->getSampleFileContent(); - if ($sample->getSampleType() == 'file' - && (!$sampleFile || !$this->fileContentValidator->isValid($sampleFile)) - ) { - throw new InputException(__('Provided file content must be valid base64 encoded data.')); + if ($sample->getSampleType() === 'url') { + if (!$this->urlValidator->isValid($sample->getSampleUrl())) { + throw new InputException(__('Sample URL must have valid format.')); + } + } elseif ($sample->getSampleFileContent()) { + if (!$this->fileContentValidator->isValid($sample->getSampleFileContent())) { + throw new InputException(__('Provided file content must be valid base64 encoded data.')); + } + } elseif (!$this->isFileValid($sample->getBasePath() . $sample->getSampleFile())) { + throw new InputException(__('Sample file not found. Please try again.')); } + } - if ($sample->getSampleType() == 'url' - && !$this->urlValidator->isValid($sample->getSampleUrl()) - ) { - throw new InputException(__('Sample URL must have valid format.')); + /** + * Check that Samples file is valid. + * + * @param string $file + * @return bool + */ + private function isFileValid(string $file): bool + { + try { + return $this->fileHelper->ensureFileInFilesystem($file); + } catch (ValidatorException $e) { + return false; } } } diff --git a/app/code/Magento/Downloadable/Model/SampleRepository.php b/app/code/Magento/Downloadable/Model/SampleRepository.php index 00f653e87b5ae..dfd26feab2d7f 100644 --- a/app/code/Magento/Downloadable/Model/SampleRepository.php +++ b/app/code/Magento/Downloadable/Model/SampleRepository.php @@ -185,15 +185,10 @@ public function save( if ($product->getTypeId() !== Type::TYPE_DOWNLOADABLE) { throw new InputException(__('Provided product must be type \'downloadable\'.')); } - $validateSampleContent = !($sample->getSampleType() === 'file' && $sample->getSampleFile()); - if (!$this->contentValidator->isValid($sample, $validateSampleContent)) { + $this->validateSampleType($sample); + if (!$this->contentValidator->isValid($sample, true)) { throw new InputException(__('Provided sample information is invalid.')); } - - if (!in_array($sample->getSampleType(), ['url', 'file'], true)) { - throw new InputException(__('Invalid sample type.')); - } - $title = $sample->getTitle(); if (empty($title)) { throw new InputException(__('Sample title cannot be empty.')); @@ -222,7 +217,7 @@ protected function saveSample( 'title' => $sample->getTitle(), ]; - if ($sample->getSampleType() === 'file' && $sample->getSampleFile() === null) { + if ($sample->getSampleType() === 'file' && $sample->getSampleFileContent() !== null) { $sampleData['file'] = $this->jsonEncoder->encode( [ $this->fileContentUploader->upload($sample->getSampleFileContent(), 'sample'), @@ -279,9 +274,8 @@ protected function updateSample( if ($existingSample->getProductId() != $linkFieldValue) { throw new InputException(__('Provided downloadable sample is not related to given product.')); } - - $validateFileContent = $sample->getSampleFileContent() === null ? false : true; - if (!$this->contentValidator->isValid($sample, $validateFileContent)) { + $this->validateSampleType($sample); + if (!$this->contentValidator->isValid($sample, true)) { throw new InputException(__('Provided sample information is invalid.')); } if ($isGlobalScopeContent) { @@ -298,14 +292,8 @@ protected function updateSample( } else { $existingSample->setTitle($sample->getTitle()); } - - if ($sample->getSampleType() === 'file' - && $sample->getSampleFileContent() === null - && $sample->getSampleFile() !== null - ) { - $existingSample->setSampleFile($sample->getSampleFile()); - } $this->saveSample($product, $sample, $isGlobalScopeContent); + return $existingSample->getId(); } @@ -327,6 +315,20 @@ public function delete($id) return true; } + /** + * Check that Sample type exist. + * + * @param SampleInterface $sample + * @throws InputException + * @return void + */ + private function validateSampleType(SampleInterface $sample) + { + if (!in_array($sample->getSampleType(), ['url', 'file'], true)) { + throw new InputException(__('Invalid sample type.')); + } + } + /** * Get MetadataPool instance * diff --git a/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/DownloadableTest.php b/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/DownloadableTest.php index 25a5d86b0385c..c2b9229753bc8 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/DownloadableTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/DownloadableTest.php @@ -7,6 +7,9 @@ use Magento\Catalog\Api\Data\ProductExtensionInterface; +/** + * Unit tests for \Magento\Downloadable\Controller\Adminhtml\Product\Initialization\Helper\Plugin\Downloadable. + */ class DownloadableTest extends \PHPUnit\Framework\TestCase { /** @@ -34,12 +37,20 @@ class DownloadableTest extends \PHPUnit\Framework\TestCase */ private $extensionAttributesMock; + /** + * @var \Magento\Downloadable\Model\Product\Type|\Magento\Catalog\Api\Data\ProductExtensionInterface + */ + private $downloadableProductTypeMock; + + /** + * @inheritdoc + */ protected function setUp() { $this->requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class); $this->productMock = $this->createPartialMock( \Magento\Catalog\Model\Product::class, - ['setDownloadableData', 'getExtensionAttributes', '__wakeup'] + ['setDownloadableData', 'getExtensionAttributes', '__wakeup', 'getTypeInstance'] ); $this->subjectMock = $this->createMock( \Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper::class @@ -62,6 +73,10 @@ protected function setUp() $sampleBuilderMock = $this->getMockBuilder(\Magento\Downloadable\Model\Sample\Builder::class) ->disableOriginalConstructor() ->getMock(); + $this->downloadableProductTypeMock = $this->createPartialMock( + \Magento\Downloadable\Model\Product\Type::class, + ['getLinks', 'getSamples'] + ); $this->downloadablePlugin = new \Magento\Downloadable\Controller\Adminhtml\Product\Initialization\Helper\Plugin\Downloadable( $this->requestMock, @@ -86,6 +101,11 @@ public function testAfterInitializeWithNoDataToSave($downloadable) $this->productMock->expects($this->once()) ->method('getExtensionAttributes') ->willReturn($this->extensionAttributesMock); + $this->productMock->expects($this->once()) + ->method('getTypeInstance') + ->willReturn($this->downloadableProductTypeMock); + $this->downloadableProductTypeMock->expects($this->once())->method('getLinks')->willReturn([]); + $this->downloadableProductTypeMock->expects($this->once())->method('getSamples')->willReturn([]); $this->extensionAttributesMock->expects($this->once()) ->method('setDownloadableProductLinks') ->with([]); diff --git a/app/code/Magento/Downloadable/Test/Unit/Model/Link/ContentValidatorTest.php b/app/code/Magento/Downloadable/Test/Unit/Model/Link/ContentValidatorTest.php index 2639c22ff2ca2..5484e39bd57fc 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Model/Link/ContentValidatorTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Model/Link/ContentValidatorTest.php @@ -5,8 +5,12 @@ */ namespace Magento\Downloadable\Test\Unit\Model\Link; +use Magento\Downloadable\Helper\File; use Magento\Downloadable\Model\Link\ContentValidator; +/** + * Unit tests for Magento\Downloadable\Model\Link\ContentValidator. + */ class ContentValidatorTest extends \PHPUnit\Framework\TestCase { /** @@ -34,13 +38,32 @@ class ContentValidatorTest extends \PHPUnit\Framework\TestCase */ protected $sampleFileMock; + /** + * @var File|\PHPUnit_Framework_MockObject_MockObject + */ + private $fileMock; + + /** + * @inheritdoc + */ protected function setUp() { + $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->fileValidatorMock = $this->createMock(\Magento\Downloadable\Model\File\ContentValidator::class); $this->urlValidatorMock = $this->createMock(\Magento\Framework\Url\Validator::class); $this->linkFileMock = $this->createMock(\Magento\Downloadable\Api\Data\File\ContentInterface::class); $this->sampleFileMock = $this->createMock(\Magento\Downloadable\Api\Data\File\ContentInterface::class); - $this->validator = new ContentValidator($this->fileValidatorMock, $this->urlValidatorMock); + $this->fileMock = $this->createMock(File::class); + + $this->validator = $objectManager->getObject( + ContentValidator::class, + [ + 'fileContentValidator' => $this->fileValidatorMock, + 'urlValidator' => $this->urlValidatorMock, + 'fileHelper' => $this->fileMock, + ] + ); } public function testIsValid() diff --git a/app/code/Magento/Downloadable/Test/Unit/Model/LinkRepositoryTest.php b/app/code/Magento/Downloadable/Test/Unit/Model/LinkRepositoryTest.php index 9ce917433ff1f..aa8bb8d1ad953 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Model/LinkRepositoryTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Model/LinkRepositoryTest.php @@ -161,7 +161,8 @@ protected function getLinkMock(array $linkData) 'getNumberOfDownloads', 'getIsShareable', 'getLinkUrl', - 'getLinkFile' + 'getLinkFile', + 'hasSampleType', ] ) ->getMockForAbstractClass(); @@ -435,6 +436,8 @@ public function testUpdateThrowsExceptionIfTitleIsEmptyAndScopeIsGlobal() 'price' => 10.1, 'number_of_downloads' => 100, 'is_shareable' => true, + 'link_type' => 'url', + 'link_url' => 'https://google.com', ]; $this->repositoryMock->expects($this->any())->method('get')->with($productSku, true) ->will($this->returnValue($this->productMock)); diff --git a/app/code/Magento/Downloadable/Test/Unit/Model/Sample/ContentValidatorTest.php b/app/code/Magento/Downloadable/Test/Unit/Model/Sample/ContentValidatorTest.php index c863fb7ad62ff..90bcfa2e39bef 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Model/Sample/ContentValidatorTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Model/Sample/ContentValidatorTest.php @@ -6,7 +6,11 @@ namespace Magento\Downloadable\Test\Unit\Model\Sample; use Magento\Downloadable\Model\Sample\ContentValidator; +use Magento\Downloadable\Helper\File; +/** + * Unit tests for Magento\Downloadable\Model\Sample\ContentValidator. + */ class ContentValidatorTest extends \PHPUnit\Framework\TestCase { /** @@ -34,12 +38,31 @@ class ContentValidatorTest extends \PHPUnit\Framework\TestCase */ protected $sampleFileMock; + /** + * @var File|\PHPUnit_Framework_MockObject_MockObject + */ + private $fileMock; + + /** + * @inheritdoc + */ protected function setUp() { + $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->fileValidatorMock = $this->createMock(\Magento\Downloadable\Model\File\ContentValidator::class); $this->urlValidatorMock = $this->createMock(\Magento\Framework\Url\Validator::class); $this->sampleFileMock = $this->createMock(\Magento\Downloadable\Api\Data\File\ContentInterface::class); - $this->validator = new ContentValidator($this->fileValidatorMock, $this->urlValidatorMock); + $this->fileMock = $this->createMock(File::class); + + $this->validator = $objectManager->getObject( + ContentValidator::class, + [ + 'fileContentValidator' => $this->fileValidatorMock, + 'urlValidator' => $this->urlValidatorMock, + 'fileHelper' => $this->fileMock, + ] + ); } public function testIsValid() diff --git a/app/code/Magento/Downloadable/Test/Unit/Model/SampleRepositoryTest.php b/app/code/Magento/Downloadable/Test/Unit/Model/SampleRepositoryTest.php index 79a314cee5423..b96aae749e21f 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Model/SampleRepositoryTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Model/SampleRepositoryTest.php @@ -352,6 +352,8 @@ public function testUpdateThrowsExceptionIfTitleIsEmptyAndScopeIsGlobal() 'id' => $sampleId, 'title' => '', 'sort_order' => 1, + 'sample_type' => 'url', + 'sample_url' => 'https://google.com', ]; $this->repositoryMock->expects($this->any())->method('get')->with($productSku, true) ->will($this->returnValue($this->productMock)); diff --git a/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Links.php b/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Links.php index 5b350071e03f3..aac277106046f 100644 --- a/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Links.php +++ b/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Links.php @@ -13,6 +13,7 @@ use Magento\Framework\UrlInterface; use Magento\Downloadable\Model\Link as LinkModel; use Magento\Downloadable\Api\Data\LinkInterface; +use Magento\Framework\Exception\ValidatorException; /** * Class Links @@ -155,7 +156,7 @@ protected function addSampleFile(array $linkData, LinkInterface $link) $sampleFile = $link->getSampleFile(); if ($sampleFile) { $file = $this->downloadableFile->getFilePath($this->linkModel->getBaseSamplePath(), $sampleFile); - if ($this->downloadableFile->ensureFileInFilesystem($file)) { + if ($this->isLinkFileValid($file)) { $linkData['sample']['file'][0] = [ 'file' => $sampleFile, 'name' => $this->downloadableFile->getFileFromPathFile($sampleFile), @@ -184,7 +185,7 @@ protected function addLinkFile(array $linkData, LinkInterface $link) $linkFile = $link->getLinkFile(); if ($linkFile) { $file = $this->downloadableFile->getFilePath($this->linkModel->getBasePath(), $linkFile); - if ($this->downloadableFile->ensureFileInFilesystem($file)) { + if ($this->isLinkFileValid($file)) { $linkData['file'][0] = [ 'file' => $linkFile, 'name' => $this->downloadableFile->getFileFromPathFile($linkFile), @@ -201,6 +202,21 @@ protected function addLinkFile(array $linkData, LinkInterface $link) return $linkData; } + /** + * Check that Links File or Sample is valid. + * + * @param string $file + * @return bool + */ + private function isLinkFileValid(string $file): bool + { + try { + return $this->downloadableFile->ensureFileInFilesystem($file); + } catch (ValidatorException $e) { + return false; + } + } + /** * Return formatted price with two digits after decimal point * diff --git a/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Samples.php b/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Samples.php index b000de487b775..988f429de1d87 100644 --- a/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Samples.php +++ b/app/code/Magento/Downloadable/Ui/DataProvider/Product/Form/Modifier/Data/Samples.php @@ -11,6 +11,7 @@ use Magento\Catalog\Model\Locator\LocatorInterface; use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Downloadable\Helper\File as DownloadableFile; +use Magento\Framework\Exception\ValidatorException; use Magento\Framework\UrlInterface; use Magento\Downloadable\Api\Data\SampleInterface; @@ -136,7 +137,7 @@ protected function addSampleFile(array $sampleData, SampleInterface $sample) $sampleFile = $sample->getSampleFile(); if ($sampleFile) { $file = $this->downloadableFile->getFilePath($this->sampleModel->getBasePath(), $sampleFile); - if ($this->downloadableFile->ensureFileInFilesystem($file)) { + if ($this->isSampleFileValid($file)) { $sampleData['file'][0] = [ 'file' => $sampleFile, 'name' => $this->downloadableFile->getFileFromPathFile($sampleFile), @@ -152,4 +153,19 @@ protected function addSampleFile(array $sampleData, SampleInterface $sample) return $sampleData; } + + /** + * Check that Sample file is valid. + * + * @param string $file + * @return bool + */ + private function isSampleFileValid(string $file): bool + { + try { + return $this->downloadableFile->ensureFileInFilesystem($file); + } catch (ValidatorException $e) { + return false; + } + } } diff --git a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/LinkRepositoryTest.php b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/LinkRepositoryTest.php index 2cc528af3e84e..339aaade18834 100644 --- a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/LinkRepositoryTest.php +++ b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/LinkRepositoryTest.php @@ -11,6 +11,9 @@ use Magento\TestFramework\Helper\Bootstrap; use Magento\TestFramework\TestCase\WebapiAbstract; +/** + * API tests for Magento\Downloadable\Model\LinkRepository. + */ class LinkRepositoryTest extends WebapiAbstract { /** @@ -291,6 +294,64 @@ public function testCreateThrowsExceptionIfLinkFileContentIsNotAValidBase64Encod $this->_webApiCall($this->createServiceInfo, $requestData); } + /** + * Check that error appears when link file not existing in filesystem. + * + * @magentoApiDataFixture Magento/Downloadable/_files/product_downloadable.php + * @expectedException \Exception + * @expectedExceptionMessage Link file not found. Please try again. + * @return void + */ + public function testCreateLinkWithMissingLinkFileThrowsException() + { + $requestData = [ + 'isGlobalScopeContent' => false, + 'sku' => 'downloadable-product', + 'link' => [ + 'title' => 'Link Title', + 'sort_order' => 1, + 'price' => 10, + 'is_shareable' => 1, + 'number_of_downloads' => 100, + 'link_type' => 'file', + 'link_file' => '/n/o/nexistfile.png', + 'sample_type' => 'url', + 'sample_file' => 'http://google.com', + ], + ]; + + $this->_webApiCall($this->createServiceInfo, $requestData); + } + + /** + * Check that error appears when link sample file not existing in filesystem. + * + * @magentoApiDataFixture Magento/Downloadable/_files/product_downloadable.php + * @expectedException \Exception + * @expectedExceptionMessage Link sample file not found. Please try again. + * @return void + */ + public function testCreateLinkWithMissingSampleFileThrowsException() + { + $requestData = [ + 'isGlobalScopeContent' => false, + 'sku' => 'downloadable-product', + 'link' => [ + 'title' => 'Link Title', + 'sort_order' => 1, + 'price' => 10, + 'is_shareable' => 1, + 'number_of_downloads' => 100, + 'link_type' => 'url', + 'link_url' => 'http://www.example.com/', + 'sample_type' => 'file', + 'sample_file' => '/n/o/nexistfile.png', + ], + ]; + + $this->_webApiCall($this->createServiceInfo, $requestData); + } + /** * @magentoApiDataFixture Magento/Downloadable/_files/product_downloadable.php * @expectedException \Exception @@ -609,7 +670,9 @@ public function testUpdate() 'is_shareable' => 0, 'number_of_downloads' => 50, 'link_type' => 'url', + 'link_url' => 'http://google.com', 'sample_type' => 'url', + 'sample_url' => 'http://google.com', ], ]; $this->assertEquals($linkId, $this->_webApiCall($this->updateServiceInfo, $requestData)); @@ -642,7 +705,9 @@ public function testUpdateSavesDataInGlobalScopeAndDoesNotAffectValuesStoredInSt 'is_shareable' => 0, 'number_of_downloads' => 50, 'link_type' => 'url', + 'link_url' => 'http://google.com', 'sample_type' => 'url', + 'sample_url' => 'http://google.com', ], ]; diff --git a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/ProductRepositoryTest.php b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/ProductRepositoryTest.php index 39a64c94067aa..6449265df22e8 100644 --- a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/ProductRepositoryTest.php +++ b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/ProductRepositoryTest.php @@ -222,7 +222,9 @@ public function testUpdateDownloadableProductLinks() 'price' => 5.0, 'number_of_downloads' => 999, 'link_type' => 'file', - 'sample_type' => 'file' + 'link_file' => $linkFile, + 'sample_type' => 'file', + 'sample_file' => $sampleFile, ]; $linkData = $this->getLinkData(); diff --git a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/SampleRepositoryTest.php b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/SampleRepositoryTest.php index c3f9473d8f094..3b0c17051d9c4 100644 --- a/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/SampleRepositoryTest.php +++ b/dev/tests/api-functional/testsuite/Magento/Downloadable/Api/SampleRepositoryTest.php @@ -10,6 +10,9 @@ use Magento\TestFramework\Helper\Bootstrap; use Magento\TestFramework\TestCase\WebapiAbstract; +/** + * API tests for Magento\Downloadable\Model\SampleRepository. + */ class SampleRepositoryTest extends WebapiAbstract { /** @@ -222,6 +225,30 @@ public function testCreateThrowsExceptionIfSampleTypeIsInvalid() $this->_webApiCall($this->createServiceInfo, $requestData); } + /** + * Check that error appears when sample file not existing in filesystem. + * + * @magentoApiDataFixture Magento/Downloadable/_files/product_downloadable.php + * @expectedException \Exception + * @expectedExceptionMessage Sample file not found. Please try again. + * @return void + */ + public function testCreateSampleWithMissingFileThrowsException() + { + $requestData = [ + 'isGlobalScopeContent' => false, + 'sku' => 'downloadable-product', + 'sample' => [ + 'title' => 'Link Title', + 'sort_order' => 1, + 'sample_type' => 'file', + 'sample_file' => '/n/o/nexistfile.png', + ], + ]; + + $this->_webApiCall($this->createServiceInfo, $requestData); + } + /** * @magentoApiDataFixture Magento/Downloadable/_files/product_downloadable.php * @expectedException \Exception @@ -379,6 +406,7 @@ public function testUpdate() 'title' => 'Updated Title', 'sort_order' => 2, 'sample_type' => 'url', + 'sample_url' => 'http://google.com', ], ]; @@ -407,6 +435,7 @@ public function testUpdateSavesDataInGlobalScopeAndDoesNotAffectValuesStoredInSt 'title' => 'Updated Title', 'sort_order' => 2, 'sample_type' => 'url', + 'sample_url' => 'http://google.com', ], ]; From 7d085791cea998cc88f8aa3c492b4593e149609c Mon Sep 17 00:00:00 2001 From: Serhii Voloshkov Date: Wed, 22 May 2019 12:59:23 +0300 Subject: [PATCH 2/2] MAGETWO-98146: Downloadable Product Controller|API save changes --- app/code/Magento/Downloadable/Model/Link/Builder.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Downloadable/Model/Link/Builder.php b/app/code/Magento/Downloadable/Model/Link/Builder.php index a977a4166f0fb..0533a634b0ea7 100644 --- a/app/code/Magento/Downloadable/Model/Link/Builder.php +++ b/app/code/Magento/Downloadable/Model/Link/Builder.php @@ -175,7 +175,8 @@ private function buildSample(\Magento\Downloadable\Api\Data\LinkInterface $link, \Magento\Downloadable\Api\Data\LinkInterface::class ); if ($link->getSampleType() === \Magento\Downloadable\Helper\Download::LINK_TYPE_FILE - && isset($sample['file'])) { + && isset($sample['file']) + ) { $linkSampleFileName = $this->downloadableFile->moveFileFromTmp( $this->getComponent()->getBaseSampleTmpPath(), $this->getComponent()->getBaseSamplePath(),