Skip to content
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

#1727: Introduce internal class wrapping SplFileInfo #29461

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Magento\MediaGalleryApi\Api\Data\AssetInterface;
use Magento\MediaGalleryApi\Api\Data\AssetInterfaceFactory;
use Magento\MediaGalleryMetadataApi\Api\ExtractMetadataInterface;
use Magento\MediaGallerySynchronization\Model\Filesystem\SplFileInfoFactory;
use Magento\MediaGallerySynchronization\Model\Filesystem\GetFileInfo;
use Magento\MediaGallerySynchronization\Model\GetContentHash;

/**
Expand Down Expand Up @@ -60,9 +60,9 @@ class CreateAssetFromFile
private $extractMetadata;

/**
* @var SplFileInfoFactory
* @var GetFileInfo
*/
private $splFileInfoFactory;
private $getFileInfo;

/**
* @param Filesystem $filesystem
Expand All @@ -71,7 +71,7 @@ class CreateAssetFromFile
* @param AssetInterfaceFactory $assetFactory
* @param GetContentHash $getContentHash
* @param ExtractMetadataInterface $extractMetadata
* @param SplFileInfoFactory $splFileInfoFactory
* @param GetFileInfo $getFileInfo
*/
public function __construct(
Filesystem $filesystem,
Expand All @@ -80,15 +80,15 @@ public function __construct(
AssetInterfaceFactory $assetFactory,
GetContentHash $getContentHash,
ExtractMetadataInterface $extractMetadata,
SplFileInfoFactory $splFileInfoFactory
GetFileInfo $getFileInfo
) {
$this->filesystem = $filesystem;
$this->driver = $driver;
$this->date = $date;
$this->assetFactory = $assetFactory;
$this->getContentHash = $getContentHash;
$this->extractMetadata = $extractMetadata;
$this->splFileInfoFactory = $splFileInfoFactory;
$this->getFileInfo = $getFileInfo;
}

/**
Expand All @@ -101,7 +101,7 @@ public function __construct(
public function execute(string $path): AssetInterface
{
$absolutePath = $this->getMediaDirectory()->getAbsolutePath($path);
$file = $this->splFileInfoFactory->create($absolutePath);
$file = $this->getFileInfo->execute($absolutePath);
[$width, $height] = getimagesize($absolutePath);

$metadata = $this->extractMetadata->execute($absolutePath);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\MediaGallerySynchronization\Model\Filesystem;

/**
* Internal class wrapping \SplFileInfo
*
* @SuppressWarnings(PHPMD.TooManyFields)
*/
class FileInfo extends \SplFileInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to use ccomposition instead of inheritance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class does not need to extend \SplFileInfo please remove the inheritance

{
/**
* @var string
*/
private $path;

/**
* @var string
*/
private $filename;

/**
* @var string
*/
private $extension;

/**
* @var $basename
*/
private $basename;

/**
* @var string
*/
private $pathname;

/**
* @var int
*/
private $inode;

/**
* @var int
*/
private $size;

/**
* @var int
*/
private $owner;

/**
* @var int
*/
private $group;

/**
* @var int
*/
private $aTime;

/**
* @var int
*/
private $mTime;

/**
* @var int
*/
private $cTime;

/**
* @var string
*/
private $type;

/**
* @var false|string
*/
private $realPath;

/**
* FileInfo constructor.
* @param string $file_name
* @param string $path
* @param string $filename
* @param string $extension
* @param string $basename
* @param string $pathname
* @param int $inode
* @param int $size
* @param int $owner
* @param int $group
* @param int $aTime
* @param int $mTime
* @param int $cTime
* @param string $type
* @param false|string $realPath
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
string $file_name,
string $path,
string $filename,
string $extension,
string $basename,
string $pathname,
int $inode,
int $size,
int $owner,
int $group,
int $aTime,
int $mTime,
int $cTime,
string $type,
$realPath
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strict type is missing

) {
parent::__construct($file_name);
$this->path = $path;
$this->filename = $filename;
$this->extension = $extension;
$this->basename = $basename;
$this->pathname = $pathname;
$this->inode = $inode;
$this->size = $size;
$this->owner = $owner;
$this->group = $group;
$this->aTime = $aTime;
$this->mTime = $mTime;
$this->cTime = $cTime;
$this->type = $type;
$this->realPath = $realPath;
}

/**
* @inheritDoc
*/
public function getPath(): string
{
return $this->path;
}

/**
* @inheritDoc
*/
public function getFilename(): string
{
return $this->filename;
}

/**
* @inheritDoc
*/
public function getExtension(): string
{
return $this->extension;
}

/**
* @inheritDoc
*/
public function getBasename($suffix = null): string
{
return $this->basename;
}

/**
* @inheritDoc
*/
public function getPathname(): string
{
return $this->pathname;
}

/**
* @inheritDoc
*/
public function getInode(): int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Inode method is used, please keep only properties/methods that are required

{
return $this->inode;
}

/**
* @inheritDoc
*/
public function getSize(): int
{
return $this->size;
}

/**
* @inheritDoc
*/
public function getOwner(): int
{
return $this->owner;
}

/**
* @inheritDoc
*/
public function getGroup(): int
{
return $this->group;
}

/**
* @inheritDoc
*/
public function getATime(): int
{
return $this->aTime;
}

/**
* @inheritDoc
*/
public function getMTime(): int
{
return $this->mTime;
}

/**
* @inheritDoc
*/
public function getCTime(): int
{
return $this->cTime;
}

/**
* @inheritDoc
*/
public function getType(): string
{
return $this->type;
}

/**
* @inheritDoc
*/
public function getRealPath()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strict type is missing

{
return $this->realPath;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\MediaGallerySynchronization\Model\Filesystem;

use Magento\MediaGallerySynchronization\Model\Filesystem\FileInfoFactory;

/**
* Get file information
*/
class GetFileInfo
sivaschenko marked this conversation as resolved.
Show resolved Hide resolved
{
/**
* @var FileInfoFactory
*/
private $fileInfoFactory;

/**
* GetFileInfo constructor.
* @param FileInfoFactory $fileInfoFactory
*/
public function __construct(
FileInfoFactory $fileInfoFactory
) {
$this->fileInfoFactory = $fileInfoFactory;
}

/**
* Get file information based on path provided.
*
* @param string $path
* @return FileInfo
*/
public function execute(string $path): FileInfo
{
$splFileInfo = new \SplFileInfo($path);

return $this->fileInfoFactory->create([
'file_name' => $path,
'path' => $splFileInfo->getPath(),
'filename' => $splFileInfo->getFilename(),
'extension' => $splFileInfo->getExtension(),
'basename' => $splFileInfo->getBasename('.' . $splFileInfo->getExtension()),
'pathname' => $splFileInfo->getPathname(),
'perms' => $splFileInfo->getPerms(),
'inode' => $splFileInfo->getInode(),
'size' => $splFileInfo->getSize(),
'owner' => $splFileInfo->getOwner(),
'group' => $splFileInfo->getGroup(),
'aTime' => $splFileInfo->getATime(),
'mTime' => $splFileInfo->getMTime(),
'cTime' => $splFileInfo->getCTime(),
'type' => $splFileInfo->getType(),
'realPath' => $splFileInfo->getRealPath()
]);
}
}
Loading