Skip to content

Adjust commands to the common style in the MediaGallery and change exception handle logic #25614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

namespace Magento\MediaGallery\Model\Asset\Command;

use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Exception\CouldNotDeleteException;
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\Exception\CouldNotDeleteException;
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
use Psr\Log\LoggerInterface;

/**
Expand Down Expand Up @@ -62,11 +62,11 @@ public function execute(string $mediaAssetPath): void
$tableName = $this->resourceConnection->getTableName(self::TABLE_MEDIA_GALLERY_ASSET);
$connection->delete($tableName, [self::MEDIA_GALLERY_ASSET_PATH . ' = ?' => $mediaAssetPath]);
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __(
'Could not delete media asset with path %path: %error',
['path' => $mediaAssetPath, 'error' => $exception->getMessage()]
);
$this->logger->critical($message);
throw new CouldNotDeleteException($message, $exception);
}
}
Expand Down
6 changes: 3 additions & 3 deletions app/code/Magento/MediaGallery/Model/Asset/Command/GetById.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,23 @@ public function execute(int $mediaAssetId): AssetInterface
->where('amg.id = ?', $mediaAssetId);
$mediaAssetData = $connection->query($select)->fetch();
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __(
'En error occurred during get media asset data by id %id: %error',
['id' => $mediaAssetId, 'error' => $exception->getMessage()]
);
$this->logger->critical($message);
throw new IntegrationException($message, $exception);
}

if (empty($mediaAssetData)) {
$message = __('There is no such media asset with id "%1"', $mediaAssetId);
$message = __('There is no such media asset with id %id', ['id' => $mediaAssetId]);
throw new NoSuchEntityException($message);
}

try {
return $this->assetFactory->create(['data' => $mediaAssetData]);
} catch (\Exception $exception) {
$this->logger->critical($exception->getMessage());
$this->logger->critical($exception);
$message = __(
'En error occurred during initialize media asset with id %id: %error',
['id' => $mediaAssetId, 'error' => $exception->getMessage()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ public function execute(string $mediaFilePath): AssetInterface

return $mediaAssets;
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __('An error occurred during get media asset list: %1', $exception->getMessage());
$this->logger->critical($message);
throw new IntegrationException($message, $exception);
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/MediaGallery/Model/Asset/Command/Save.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ public function execute(AssetInterface $mediaAsset): int
$connection->insertOnDuplicate($tableName, $this->extractor->extract($mediaAsset, AssetInterface::class));
return (int) $connection->lastInsertId($tableName);
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __('An error occurred during media asset save: %1', $exception->getMessage());
$this->logger->critical($message);
throw new CouldNotSaveException($message, $exception);
}
}
Expand Down
8 changes: 7 additions & 1 deletion app/code/Magento/MediaGallery/Model/DataExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
class DataExtractor implements DataExtractorInterface
{
/**
* @inheritdoc
* Extract data from an object using available getters (does not process extension attributes)
*
* @param object $object
* @param string|null $interface
*
* @return array
* @throws \ReflectionException
*/
public function extract($object, string $interface = null): array
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

namespace Magento\MediaGallery\Model\Keyword\Command;

use Magento\Framework\Exception\IntegrationException;
use Magento\MediaGalleryApi\Api\Data\KeywordInterface;
use Magento\MediaGalleryApi\Api\Data\KeywordInterfaceFactory;
use Magento\MediaGalleryApi\Model\Keyword\Command\GetAssetKeywordsInterface;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Exception\NotFoundException;
use Psr\Log\LoggerInterface;

/**
* ClassGetAssetKeywords
Expand All @@ -31,27 +32,35 @@ class GetAssetKeywords implements GetAssetKeywordsInterface
*/
private $assetKeywordFactory;

/**
* @var LoggerInterface
*/
private $logger;

/**
* GetAssetKeywords constructor.
*
* @param ResourceConnection $resourceConnection
* @param KeywordInterfaceFactory $assetKeywordFactory
* @param LoggerInterface $logger
*/
public function __construct(
ResourceConnection $resourceConnection,
KeywordInterfaceFactory $assetKeywordFactory
KeywordInterfaceFactory $assetKeywordFactory,
LoggerInterface $logger
) {
$this->resourceConnection = $resourceConnection;
$this->assetKeywordFactory = $assetKeywordFactory;
$this->logger = $logger;
}

/**
* Get asset related keywords.
*
* @param int $assetId
*
* @return KeywordInterface[]
* @throws NotFoundException
* @return KeywordInterface[]|[]
* @throws IntegrationException
*/
public function execute(int $assetId): array
{
Expand All @@ -71,8 +80,9 @@ public function execute(int $assetId): array

return $keywords;
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __('An error occurred during get asset keywords: %1', $exception->getMessage());
throw new NotFoundException($message, $exception);
throw new IntegrationException($message, $exception);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\DB\Adapter\Pdo\Mysql;
use Magento\Framework\Exception\CouldNotSaveException;
use Psr\Log\LoggerInterface;

/**
* Class SaveAssetKeywords
Expand All @@ -33,22 +34,34 @@ class SaveAssetKeywords implements SaveAssetKeywordsInterface
*/
private $saveAssetLinks;

/**
* @var LoggerInterface
*/
private $logger;

/**
* SaveAssetKeywords constructor.
*
* @param ResourceConnection $resourceConnection
* @param SaveAssetLinks $saveAssetLinks
* @param LoggerInterface $logger
*/
public function __construct(
ResourceConnection $resourceConnection,
SaveAssetLinks $saveAssetLinks
SaveAssetLinks $saveAssetLinks,
LoggerInterface $logger
) {
$this->resourceConnection = $resourceConnection;
$this->saveAssetLinks = $saveAssetLinks;
$this->logger = $logger;
}

/**
* @inheritdoc
* Save asset keywords.
*
* @param KeywordInterface[] $keywords
* @param int $assetId
* @throws CouldNotSaveException
*/
public function execute(array $keywords, int $assetId): void
{
Expand All @@ -72,6 +85,7 @@ public function execute(array $keywords, int $assetId): void
$this->saveAssetLinks->execute($assetId, $this->getKeywordIds($data));
}
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __('An error occurred during save asset keyword: %1', $exception->getMessage());
throw new CouldNotSaveException($message, $exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\DB\Adapter\Pdo\Mysql;
use Magento\Framework\Exception\CouldNotSaveException;
use Psr\Log\LoggerInterface;

/**
* Class SaveAssetLinks
Expand All @@ -29,14 +30,22 @@ class SaveAssetLinks
private $resourceConnection;

/**
* SaveAssetKeywords constructor.
* @var LoggerInterface
*/
private $logger;

/**
* SaveAssetLinks constructor.
*
* @param ResourceConnection $resourceConnection
* @param LoggerInterface $logger
*/
public function __construct(
ResourceConnection $resourceConnection
ResourceConnection $resourceConnection,
LoggerInterface $logger
) {
$this->resourceConnection = $resourceConnection;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -66,6 +75,7 @@ public function execute(int $assetId, array $keywordIds): void
);
}
} catch (\Exception $exception) {
$this->logger->critical($exception);
$message = __('An error occurred during save asset keyword links: %1', $exception->getMessage());
throw new CouldNotSaveException($message, $exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ public function afterRemoveImage(
try {
$this->deleteMediaAssetByPath->execute($file);
} catch (\Exception $exception) {
$message = __('An error occurred during media asset delete at media processor: %1', $exception->getMessage());
$this->logger->critical($message->render());
$this->logger->critical($exception);
}

return $result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@

namespace Magento\MediaGallery\Test\Unit\Model\Keyword\Command;

use Magento\Framework\Exception\IntegrationException;
use Magento\MediaGallery\Model\Keyword\Command\GetAssetKeywords;
use Magento\MediaGalleryApi\Api\Data\KeywordInterface;
use Magento\MediaGalleryApi\Api\Data\KeywordInterfaceFactory;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\DB\Select;
use Magento\Framework\Exception\NotFoundException;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

/**
* GetAssetKeywordsTest
*/
class GetAssetKeywordsTest extends TestCase
{
/**
Expand All @@ -34,14 +38,21 @@ class GetAssetKeywordsTest extends TestCase
*/
private $assetKeywordFactoryStub;

/**
* @var LoggerInterface|MockObject
*/
private $loggerMock;

protected function setUp(): void
{
$this->resourceConnectionStub = $this->createMock(ResourceConnection::class);
$this->assetKeywordFactoryStub = $this->createMock(KeywordInterfaceFactory::class);
$this->loggerMock = $this->createMock(LoggerInterface::class);

$this->sut = new GetAssetKeywords(
$this->resourceConnectionStub,
$this->assetKeywordFactoryStub
$this->assetKeywordFactoryStub,
$this->loggerMock
);
}

Expand All @@ -51,7 +62,6 @@ protected function setUp(): void
* @dataProvider casesProvider()
* @param array $databaseQueryResult
* @param int $expectedNumberOfFoundKeywords
* @throws NotFoundException
*/
public function testFind(array $databaseQueryResult, int $expectedNumberOfFoundKeywords): void
{
Expand Down Expand Up @@ -80,9 +90,9 @@ public function casesProvider(): array
}

/**
* Negative test
* Test case when an error occured during get data request.
*
* @throws NotFoundException
* @throws IntegrationException
*/
public function testNotFoundBecauseOfError(): void
{
Expand All @@ -92,7 +102,10 @@ public function testNotFoundBecauseOfError(): void
->method('getConnection')
->willThrowException((new \Exception()));

$this->expectException(NotFoundException::class);
$this->expectException(IntegrationException::class);
$this->loggerMock->expects($this->once())
->method('critical')
->willReturnSelf();

$this->sut->execute($randomAssetId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@

namespace Magento\MediaGallery\Test\Unit\Model\Keyword\Command;

use Magento\MediaGallery\Model\Keyword\Command\SaveAssetKeywords;
use Magento\MediaGallery\Model\Keyword\Command\SaveAssetLinks;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DataObject;
use Magento\Framework\DB\Adapter\Pdo\Mysql;
use Magento\Framework\DB\Select;
use Magento\Framework\Exception\CouldNotSaveException;
use Magento\MediaGallery\Model\Keyword\Command\SaveAssetKeywords;
use Magento\MediaGallery\Model\Keyword\Command\SaveAssetLinks;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

/**
* SaveAssetKeywordsTest.
Expand Down Expand Up @@ -47,6 +48,11 @@ class SaveAssetKeywordsTest extends TestCase
*/
private $selectMock;

/**
* @var LoggerInterface|MockObject
*/
private $loggerMock;

/**
* SetUp
*/
Expand All @@ -58,10 +64,12 @@ public function setUp(): void
->disableOriginalConstructor()
->getMock();
$this->selectMock = $this->createMock(Select::class);
$this->loggerMock = $this->createMock(LoggerInterface::class);

$this->sut = new SaveAssetKeywords(
$this->resourceConnectionMock,
$this->saveAssetLinksMock
$this->saveAssetLinksMock,
$this->loggerMock
);
}

Expand Down Expand Up @@ -106,6 +114,9 @@ public function testAssetNotSavingCausedByError(): void
->method('getConnection')
->willThrowException((new \Exception()));
$this->expectException(CouldNotSaveException::class);
$this->loggerMock->expects($this->once())
->method('critical')
->willReturnSelf();

$this->sut->execute([$keyword], 1);
}
Expand Down
Loading