Skip to content

Commit

Permalink
ACP2E-3100: Image File does not exist in New Relic Error Log
Browse files Browse the repository at this point in the history
  • Loading branch information
abukatar committed Jun 17, 2024
1 parent cf2ea5d commit 26071c7
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 20 deletions.
71 changes: 63 additions & 8 deletions app/code/Magento/Catalog/Model/View/Asset/Placeholder.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@
namespace Magento\Catalog\Model\View\Asset;

use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\Filesystem;
use Magento\Framework\Filesystem\DriverPool;
use Magento\Framework\View\Asset\ContextInterface;
use Magento\Framework\View\Asset\File\NotFoundException;
use Magento\Framework\View\Asset\LocalInterface;
use Magento\Framework\View\Asset\Repository;
use Magento\Catalog\Model\Product\Media\ConfigInterface;

/**
* A locally available image placeholder file asset that can be referred with a file type
Expand Down Expand Up @@ -53,28 +58,49 @@ class Placeholder implements LocalInterface
*/
private $scopeConfig;

/**
* @var \Magento\Framework\Filesystem\Directory\WriteInterface
*/
private $directoryMedia;

/**
* @var ConfigInterface
*/
private $mediaConfig;

/**
* Placeholder constructor.
*
* @param ContextInterface $context
* @param ScopeConfigInterface $scopeConfig
* @param Repository $assetRepo
* @param string $type
* @param Filesystem|null $filesystem
* @param ConfigInterface|null $mediaConfig
*
*/
public function __construct(
ContextInterface $context,
ScopeConfigInterface $scopeConfig,
Repository $assetRepo,
$type
$type,
?Filesystem $filesystem = null,
?ConfigInterface $mediaConfig = null
) {
$this->context = $context;
$this->scopeConfig = $scopeConfig;
$this->assetRepo = $assetRepo;
$this->type = $type;
$filesystem = $filesystem ?? ObjectManager::getInstance()->get(Filesystem::class);
$this->mediaConfig = $mediaConfig ?? ObjectManager::getInstance()->get(ConfigInterface::class);
$this->directoryMedia = $filesystem->getDirectoryWrite(
DirectoryList::MEDIA,
DriverPool::FILE
);
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function getUrl()
{
Expand All @@ -101,7 +127,7 @@ public function getContentType()
public function getPath()
{
if ($this->getFilePath() !== null) {
$result = $this->getContext()->getPath()
$result = $this->getLocalMediaPath()
. DIRECTORY_SEPARATOR . $this->getModule()
. DIRECTORY_SEPARATOR . $this->getFilePath();
} else {
Expand All @@ -119,7 +145,35 @@ public function getPath()
}

/**
* {@inheritdoc}
* Get path for local media
*
* @return string
*/
private function getLocalMediaPath()
{
return $this->directoryMedia->getAbsolutePath($this->mediaConfig->getBaseMediaPath());
}

/**
* Get relative placeholder path
*
* @return string|null
*/
public function getRelativePath()
{
$result = null;
//will use system placeholder unless another specified in the config
if ($this->getFilePath() !== null) {
$result = DIRECTORY_SEPARATOR . DirectoryList::MEDIA
. DIRECTORY_SEPARATOR . $this->directoryMedia->getRelativePath($this->mediaConfig->getBaseMediaPath())
. DIRECTORY_SEPARATOR . $this->getModule()
. DIRECTORY_SEPARATOR . $this->getFilePath();
}
return $result;
}

/**
* @inheritdoc
*/
public function getSourceFile()
{
Expand All @@ -137,15 +191,15 @@ public function getSourceContentType()
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function getContent()
{
return null;
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function getFilePath()
{
Expand All @@ -163,7 +217,8 @@ public function getFilePath()
}

/**
* {@inheritdoc}
* @inheritdoc
*
* @return ContextInterface
*/
public function getContext()
Expand All @@ -172,7 +227,7 @@ public function getContext()
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function getModule()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,40 @@
use Magento\Store\Model\ScopeInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Magento\Framework\Filesystem;
use Magento\Catalog\Model\Product\Media\ConfigInterface;

class PlaceholderTest extends TestCase
{
/**
* @var Placeholder
*/
protected $model;
private $model;

/**
* @var ScopeConfigInterface|MockObject
*/
protected $scopeConfig;
private $scopeConfig;

/**
* @var Repository|MockObject
*/
protected $repository;
private $repository;

/**
* @var ContextInterface|MockObject
*/
protected $imageContext;
private $imageContext;

/**
* @var Filesystem|MockObject
*/
private $filesystem;

/**
* @var ConfigInterface|MockObject
*/
private $mediaConfig;

protected function setUp(): void
{
Expand All @@ -47,11 +59,18 @@ protected function setUp(): void
$this->repository = $this->getMockBuilder(Repository::class)
->disableOriginalConstructor()
->getMock();
$this->filesystem = $this->createMock(Filesystem::class);
$this->filesystem->expects($this->any())
->method('getDirectoryWrite')
->willReturn($this->createMock(\Magento\Framework\Filesystem\Directory\WriteInterface::class));
$this->mediaConfig = $this->createMock(ConfigInterface::class);
$this->model = new Placeholder(
$this->imageContext,
$this->scopeConfig,
$this->repository,
'thumbnail'
'thumbnail',
$this->filesystem,
$this->mediaConfig
);
}

Expand Down Expand Up @@ -87,7 +106,9 @@ public function testGetPathAndGetSourceFile($imageType, $placeholderPath)
$this->imageContext,
$this->scopeConfig,
$this->repository,
$imageType
$imageType,
$this->filesystem,
$this->mediaConfig
);
$absolutePath = '/var/www/html/magento2ce/pub/media/catalog/product';

Expand All @@ -108,8 +129,7 @@ public function testGetPathAndGetSourceFile($imageType, $placeholderPath)
$this->repository->expects($this->any())->method('createAsset')->willReturn($assetMock);
} else {
$this->imageContext->expects($this->any())->method('getPath')->willReturn($absolutePath);
$expectedResult = $absolutePath
. DIRECTORY_SEPARATOR . $imageModel->getModule()
$expectedResult = DIRECTORY_SEPARATOR . $imageModel->getModule()
. DIRECTORY_SEPARATOR . $placeholderPath;
}

Expand All @@ -128,7 +148,9 @@ public function testGetUrl($imageType, $placeholderPath)
$this->imageContext,
$this->scopeConfig,
$this->repository,
$imageType
$imageType,
$this->filesystem,
$this->mediaConfig
);

$this->scopeConfig->expects($this->any())
Expand Down
13 changes: 13 additions & 0 deletions app/code/Magento/MediaStorage/App/Media.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,18 @@ private function createLocalCopy(): void
}
}

/**
* Create local vopy for the placeholder
*
* @param string $relativeFileName
* @return void
*/
private function createPlaceholderLocalCopy(string $relativeFileName): void
{
$synchronizer = $this->syncFactory->create(['directory' => $this->directoryMedia]);
$synchronizer->synchronize($relativeFileName);
}

/**
* Check if media directory changed
*
Expand All @@ -248,6 +260,7 @@ private function checkMediaDirectoryChanged(): bool
private function setPlaceholderImage(): void
{
$placeholder = $this->placeholderFactory->create(['type' => 'image']);
$this->createPlaceholderLocalCopy($placeholder->getRelativePath());
$this->response->setFilePath($placeholder->getPath());
}

Expand Down
7 changes: 4 additions & 3 deletions app/code/Magento/MediaStorage/Test/Unit/App/MediaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,7 @@ public function testProcessRequestReturnsNotFoundIfFileIsNotSynchronized(): void
{
$this->mediaModel = $this->createMediaModel();

$this->sync->expects(self::once())
->method('synchronize')
$this->sync->method('synchronize')
->with(self::RELATIVE_FILE_PATH);
$this->directoryMediaMock->expects(self::once())
->method('getAbsolutePath')
Expand Down Expand Up @@ -270,9 +269,11 @@ protected function createMediaModel(bool $isAllowed = true): Media

$driverFile = $this->createMock(Filesystem\Driver\File::class);
$driverFile->method('getRealPath')->willReturn('');
$placeholder = $this->createMock(Placeholder::class);
$placeholder->method('getRelativePath')->willReturn(self::RELATIVE_FILE_PATH);
$placeholderFactory = $this->createMock(PlaceholderFactory::class);
$placeholderFactory->method('create')
->willReturn($this->createMock(Placeholder::class));
->willReturn($placeholder);

return new Media(
$this->configFactoryMock,
Expand Down

0 comments on commit 26071c7

Please sign in to comment.