diff --git a/app/code/Magento/Sitemap/Model/Sitemap.php b/app/code/Magento/Sitemap/Model/Sitemap.php index 2eb289e2e07d6..887dac2a598c0 100644 --- a/app/code/Magento/Sitemap/Model/Sitemap.php +++ b/app/code/Magento/Sitemap/Model/Sitemap.php @@ -1,9 +1,8 @@ _sitemapData = $sitemapData; $this->filesystem = $filesystem; $this->_directory = $filesystem->getDirectoryWrite(DirectoryList::PUB); + $this->tmpDirectory = $filesystem->getDirectoryWrite(DirectoryList::SYS_TMP); $this->_categoryFactory = $categoryFactory; $this->_productFactory = $productFactory; $this->_cmsFactory = $cmsFactory; @@ -482,11 +479,9 @@ public function generateXml() if ($this->_sitemapIncrement == 1) { // In case when only one increment file was created use it as default sitemap - $sitemapPath = $this->getSitemapPath() !== null ? rtrim($this->getSitemapPath(), '/') : ''; - $path = $sitemapPath . '/' . $this->_getCurrentSitemapFilename($this->_sitemapIncrement); - $destination = $sitemapPath . '/' . $this->getSitemapFilename(); - - $this->_directory->renameFile($path, $destination); + $path = $this->getFilePath($this->_getCurrentSitemapFilename($this->_sitemapIncrement)); + $destination = $this->getFilePath($this->getSitemapFilename()); + $this->tmpDirectory->renameFile($path, $destination, $this->_directory); } else { // Otherwise create index file with list of generated sitemaps $this->_createSitemapIndex(); @@ -507,10 +502,15 @@ protected function _createSitemapIndex() { $this->_createSitemap($this->getSitemapFilename(), self::TYPE_INDEX); for ($i = 1; $i <= $this->_sitemapIncrement; $i++) { - $xml = $this->_getSitemapIndexRow($this->_getCurrentSitemapFilename($i), $this->_getCurrentDateTime()); + $fileName = $this->_getCurrentSitemapFilename($i); + $path = $this->getFilePath($fileName); + $this->tmpDirectory->renameFile($path, $path, $this->_directory); + $xml = $this->_getSitemapIndexRow($fileName, $this->_getCurrentDateTime()); $this->_writeSitemapRow($xml); } $this->_finalizeSitemap(self::TYPE_INDEX); + $path = $this->getFilePath($this->getSitemapFilename()); + $this->tmpDirectory->renameFile($path, $path, $this->_directory); } /** @@ -638,9 +638,8 @@ protected function _createSitemap($fileName = null, $type = self::TYPE_URL) $this->_sitemapIncrement++; $fileName = $this->_getCurrentSitemapFilename($this->_sitemapIncrement); } - - $path = ($this->getSitemapPath() !== null ? rtrim($this->getSitemapPath(), '/') : '') . '/' . $fileName; - $this->_stream = $this->_directory->openFile($path); + $path = $this->getFilePath($fileName); + $this->_stream = $this->tmpDirectory->openFile($path); $fileHeader = sprintf($this->_tags[$type][self::OPEN_TAG_KEY], $type); $this->_stream->write($fileHeader); @@ -688,6 +687,20 @@ protected function _getCurrentSitemapFilename($index) . '-' . $this->getStoreId() . '-' . $index . '.xml'; } + /** + * Get path to sitemap file + * + * @param string $fileName + * @return string + */ + private function getFilePath(string $fileName): string + { + $path = $this->getSitemapPath() !== null ? rtrim($this->getSitemapPath(), '/') : ''; + $path .= '/' . $fileName; + + return $path; + } + /** * Get base dir * @@ -815,6 +828,7 @@ public function getSitemapUrl($sitemapPath, $sitemapFileName) * @return bool * @deprecated 100.1.5 Because the robots.txt file is not generated anymore, * this method is not needed and will be removed in major release. + * @see no alternatives */ protected function _isEnabledSubmissionRobots() { @@ -829,6 +843,7 @@ protected function _isEnabledSubmissionRobots() * @return void * @deprecated 100.1.5 Because the robots.txt file is not generated anymore, * this method is not needed and will be removed in major release. + * @see no alternatives */ protected function _addSitemapToRobotsTxt($sitemapFileName) { diff --git a/app/code/Magento/Sitemap/Test/Unit/Model/SitemapTest.php b/app/code/Magento/Sitemap/Test/Unit/Model/SitemapTest.php index 0ccd32729c1a2..6c40d257876df 100644 --- a/app/code/Magento/Sitemap/Test/Unit/Model/SitemapTest.php +++ b/app/code/Magento/Sitemap/Test/Unit/Model/SitemapTest.php @@ -1,12 +1,13 @@ sitemapCategoryMock = $this->getMockBuilder(Category::class) - ->disableOriginalConstructor() - ->getMock(); - $this->sitemapProductMock = $this->getMockBuilder(Product::class) - ->disableOriginalConstructor() - ->getMock(); - $this->sitemapCmsPageMock = $this->getMockBuilder(Page::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->helperMockSitemap = $this->getMockBuilder(Data::class) - ->disableOriginalConstructor() - ->getMock(); - $resourceMethods = [ '_construct', 'beginTransaction', @@ -133,35 +107,37 @@ protected function setUp(): void 'commit', '__wakeup', ]; - $this->resourceMock = $this->getMockBuilder(SitemapResource::class) ->onlyMethods($resourceMethods) ->disableOriginalConstructor() ->getMock(); - $this->resourceMock->method('addCommitCallback') ->willReturnSelf(); $this->fileMock = $this->createMock(Write::class); - $this->directoryMock = $this->createMock(DirectoryWrite::class); - $this->directoryMock->method('openFile') ->willReturn($this->fileMock); - - $this->filesystemMock = $this->getMockBuilder(Filesystem::class) - ->onlyMethods(['getDirectoryWrite']) - ->disableOriginalConstructor() - ->getMock(); - + $this->tmpFileMock = $this->createMock(Write::class); + $this->tmpDirectoryMock = $this->createMock(DirectoryWrite::class); + $this->tmpDirectoryMock->method('openFile') + ->willReturn($this->tmpFileMock); + $this->mediaDirectoryMock = $this->createMock(DirectoryWrite::class); + $this->filesystemMock = $this->createMock(Filesystem::class); $this->filesystemMock->method('getDirectoryWrite') - ->willReturn($this->directoryMock); + ->willReturnMap( + [ + [DirectoryList::PUB, $this->directoryMock], + [DirectoryList::SYS_TMP, $this->tmpDirectoryMock], + [DirectoryList::MEDIA, $this->mediaDirectoryMock], + ] + ); + + $this->configReaderMock = $this->createMock(SitemapConfigReaderInterface::class); + $this->itemProviderMock = $this->createMock(ItemProviderInterface::class); - $this->configReaderMock = $this->getMockForAbstractClass(SitemapConfigReaderInterface::class); - $this->itemProviderMock = $this->getMockForAbstractClass(ItemProviderInterface::class); - $this->request = $this->createMock(Http::class); $this->store = $this->createPartialMock(Store::class, ['isFrontUrlSecure', 'getBaseUrl']); - $this->storeManagerMock = $this->getMockForAbstractClass(StoreManagerInterface::class); + $this->storeManagerMock = $this->createMock(StoreManagerInterface::class); $this->storeManagerMock->method('getStore') ->willReturn($this->store); } @@ -423,35 +399,28 @@ protected function prepareSitemapModelMock( } $actualData[$currentFile] .= $str; }; - // Check that all expected lines were written - $this->fileMock->expects( - $this->exactly($expectedWrites) - )->method( - 'write' - )->willReturnCallback( - $streamWriteCallback - ); + $this->tmpFileMock->expects($this->exactly($expectedWrites)) + ->method('write') + ->willReturnCallback($streamWriteCallback); $checkFileCallback = function ($file) use (&$currentFile) { $currentFile = $file; - };// Check that all expected file descriptors were created - $this->directoryMock->expects($this->exactly(count($expectedFile)))->method('openFile') + }; + // Check that all expected file descriptors were created + $this->tmpDirectoryMock->expects($this->exactly(count($expectedFile))) + ->method('openFile') ->willReturnCallback($checkFileCallback); // Check that all file descriptors were closed - $this->fileMock->expects($this->exactly(count($expectedFile))) + $this->tmpFileMock->expects($this->exactly(count($expectedFile))) ->method('close'); if (count($expectedFile) == 1) { - $this->directoryMock->expects($this->once()) + $this->tmpDirectoryMock->expects($this->once()) ->method('renameFile') - ->willReturnCallback( - function ($from, $to) { - Assert::assertEquals('/sitemap-1-1.xml', $from); - Assert::assertEquals('/sitemap.xml', $to); - } - ); + ->with('/sitemap-1-1.xml', '/sitemap.xml', $this->directoryMock) + ->willReturn(true); } // Check robots txt @@ -591,17 +560,11 @@ protected function getModelMock($mockBeforeSave = false) */ private function getModelConstructorArgs() { - $categoryFactory = $this->getMockBuilder(CategoryFactory::class) - ->disableOriginalConstructor() - ->getMock(); - - $productFactory = $this->getMockBuilder(ProductFactory::class) - ->disableOriginalConstructor() - ->getMock(); - - $cmsFactory = $this->getMockBuilder(PageFactory::class) - ->disableOriginalConstructor() - ->getMock(); + $categoryFactory = $this->createMock(CategoryFactory::class); + $productFactory = $this->createMock(ProductFactory::class); + $cmsFactory = $this->createMock(PageFactory::class); + $helperMockSitemap = $this->createMock(Data::class); + $request = $this->createMock(Http::class); $objectManager = new ObjectManager($this); $escaper = $objectManager->getObject(Escaper::class); @@ -614,12 +577,12 @@ private function getModelConstructorArgs() 'productFactory' => $productFactory, 'cmsFactory' => $cmsFactory, 'storeManager' => $this->storeManagerMock, - 'sitemapData' => $this->helperMockSitemap, + 'sitemapData' => $helperMockSitemap, 'filesystem' => $this->filesystemMock, 'itemProvider' => $this->itemProviderMock, 'configReader' => $this->configReaderMock, 'escaper' => $escaper, - 'request' => $this->request, + 'request' => $request, ] ); $constructArguments['resource'] = null;