Skip to content

Commit

Permalink
Mark some classes are readonly (#2269)
Browse files Browse the repository at this point in the history
  • Loading branch information
ildyria committed Mar 1, 2024
1 parent 37c4180 commit 41bea7a
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 93 deletions.
2 changes: 1 addition & 1 deletion app/Actions/Import/Exec.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ public function do(
foreach ($dirs as $dir) {
$this->assertImportNotCancelled();
/** @var Album|null */
$album = $this->importMode->shallSkipDuplicates() ?
$album = $this->importMode->shallSkipDuplicates ?
Album::query()
->select(['albums.*'])
->join('base_albums', 'base_albums.id', '=', 'albums.id')
Expand Down
16 changes: 8 additions & 8 deletions app/Actions/Photo/Archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ protected function file(Photo $photo, DownloadVariantType $downloadVariant): Str

$responseGenerator = function () use ($archiveFileInfo) {
$outputStream = fopen('php://output', 'wb');
stream_copy_to_stream($archiveFileInfo->getFile()->read(), $outputStream);
$archiveFileInfo->getFile()->close();
stream_copy_to_stream($archiveFileInfo->file->read(), $outputStream);
$archiveFileInfo->file->close();
fclose($outputStream);
};

Expand All @@ -100,11 +100,11 @@ protected function file(Photo $photo, DownloadVariantType $downloadVariant): Str
$disposition = HeaderUtils::makeDisposition(
HeaderUtils::DISPOSITION_ATTACHMENT,
$archiveFileInfo->getFilename(),
mb_check_encoding($archiveFileInfo->getFilename(), 'ASCII') ? '' : 'Photo' . $archiveFileInfo->getFile()->getExtension()
mb_check_encoding($archiveFileInfo->getFilename(), 'ASCII') ? '' : 'Photo' . $archiveFileInfo->file->getExtension()
);
$response->headers->set('Content-Type', $photo->type);
$response->headers->set('Content-Disposition', $disposition);
$response->headers->set('Content-Length', strval($archiveFileInfo->getFile()->getFilesize()));
$response->headers->set('Content-Length', strval($archiveFileInfo->file->getFilesize()));
// Note: Using insecure hashing algorithm is fine here.
// The ETag header must only be different for different size variants
// Pre-image resistance and collision robustness is not required.
Expand All @@ -114,10 +114,10 @@ protected function file(Photo $photo, DownloadVariantType $downloadVariant): Str
// we must avoid illegal characters like `/` and md5 returns a
// hexadecimal string.
$response->headers->set('ETag', md5(
$archiveFileInfo->getFile()->getBasename() .
$archiveFileInfo->file->getBasename() .
$downloadVariant->value .
$photo->updated_at->toAtomString() .
$archiveFileInfo->getFile()->getFilesize())
$archiveFileInfo->file->getFilesize())
);
$response->headers->set('Last-Modified', $photo->updated_at->format(\DateTimeInterface::RFC7231));

Expand Down Expand Up @@ -232,10 +232,10 @@ protected function zip(Collection $photos, DownloadVariantType $downloadVariant)
);
} while (array_key_exists($filename, $uniqueFilenames));
}
$zip->addFileFromStream(fileName: $filename, stream: $archiveFileInfo->getFile()->read(),
$zip->addFileFromStream(fileName: $filename, stream: $archiveFileInfo->file->read(),
compressionMethod: $this->deflateLevel === -1 ? ZipMethod::STORE : ZipMethod::DEFLATE,
deflateLevel: $this->deflateLevel);
$archiveFileInfo->getFile()->close();
$archiveFileInfo->file->close();
// Reset the execution timeout for every iteration.
try {
set_time_limit((int) ini_get('max_execution_time'));
Expand Down
2 changes: 1 addition & 1 deletion app/Actions/Photo/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
* Hence, this method collects all files which need to be removed.
* The caller can then decide to delete them asynchronously.
*/
class Delete
readonly class Delete
{
protected FileDeleter $fileDeleter;

Expand Down
56 changes: 10 additions & 46 deletions app/Actions/Photo/Extensions/ArchiveFileInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,53 +31,27 @@
* Note that the full path does not necessarily contains the base filename,
* because the source file might be named completely differently.
*/
class ArchiveFileInfo
final readonly class ArchiveFileInfo
{
protected string $baseFilename;
protected string $baseFilenameAddon;
protected BaseMediaFile $file;

/**
* ArchiveFileInfo constructor.
*
* @param string $baseFilename the base filename (without directory
* and extension)
* @param string $baseFilenameAddon the "addon" to the base filename
* @param BaseMediaFile $file the source file
*/
public function __construct(string $baseFilename, string $baseFilenameAddon, BaseMediaFile $file)
{
$this->baseFilename = $baseFilename;
$this->baseFilenameAddon = $baseFilenameAddon;
$this->file = $file;
}

/**
* Returns the base filename.
*
* The base file name should be used to create a "meaningful" filename
* which is offered to the client for download or put into the archive.
*
* @return string the base filename
*/
public function getBaseFilename(): string
{
return $this->baseFilename;
}

/**
* Returns the addon to the base filename.
*
* The base file name should be used to create a "meaningful" filename
* which is offered to the client for download or put into the archive.
* The addon enables to create different filenames for different variants
* of the same photo.
*
* @return string the addon to the base filename
* @param string $baseFilename the base filename (without directory
* and extension)
* @param string $baseFilenameAddon the "addon" to the base filename
* @param BaseMediaFile $file the source file
*/
public function getBaseFileNameAddon(): string
public function __construct(
private string $baseFilename,
private string $baseFilenameAddon,
public BaseMediaFile $file)
{
return $this->baseFilenameAddon;
}

/**
Expand All @@ -90,16 +64,6 @@ public function getBaseFileNameAddon(): string
*/
public function getFilename(string $extraAddon = ''): string
{
return $this->getBaseFilename() . $this->getBaseFileNameAddon() . $extraAddon . $this->file->getExtension();
}

/**
* Returns the source file.
*
* @return BaseMediaFile the source file
*/
public function getFile(): BaseMediaFile
{
return $this->file;
return $this->baseFilename . $this->baseFilenameAddon . $extraAddon . $this->file->getExtension();
}
}
4 changes: 2 additions & 2 deletions app/Actions/Photo/Strategies/AddDuplicateStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function do(): Photo

// At least update the existing photo with additional metadata if
// available
if ($this->parameters->importMode->shallResyncMetadata()) {
if ($this->parameters->importMode->shallResyncMetadata) {
$this->hydrateMetadata();
if ($this->photo->isDirty()) {
Log::notice(__METHOD__ . ':' . __LINE__ . ' Updating metadata of existing photo.');
Expand All @@ -36,7 +36,7 @@ public function do(): Photo
}
}

if ($this->parameters->importMode->shallSkipDuplicates()) {
if ($this->parameters->importMode->shallSkipDuplicates) {
if ($hasBeenReSynced) {
throw new PhotoResyncedException();
} else {
Expand Down
4 changes: 2 additions & 2 deletions app/Actions/Photo/Strategies/AddStandaloneStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public function do(): Photo
private function putSourceIntoFinalDestination(FlysystemFile $targetFile): StreamStats
{
try {
if ($this->parameters->importMode->shallImportViaSymlink()) {
if ($this->parameters->importMode->shallImportViaSymlink) {
if (!$targetFile->isLocalFile()) {
throw new ConfigurationException('Symlinking is only supported on local filesystems');
}
Expand Down Expand Up @@ -274,7 +274,7 @@ private function putSourceIntoFinalDestination(FlysystemFile $targetFile): Strea
$this->sourceFile->close();
$targetFile->close();
}
if ($this->parameters->importMode->shallDeleteImported()) {
if ($this->parameters->importMode->shallDeleteImported) {
// This may throw an exception, if the original has been
// readable, but is not writable
// In this case, the media file will have been copied, but
Expand Down
4 changes: 2 additions & 2 deletions app/Actions/Photo/Strategies/AddVideoPartnerStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ protected function putSourceIntoFinalDestination(FlysystemFile $videoTargetFile)
// AddStandaloneStrategy::putSourceIntoFinalDestination()
// except that we can skip the part about normalization of
// orientation, because we don't support that for videos.
if ($this->parameters->importMode->shallImportViaSymlink()) {
if ($this->parameters->importMode->shallImportViaSymlink) {
if (!$videoTargetFile->isLocalFile()) {
throw new ConfigurationException('Symlinking is only supported on local filesystems');
}
Expand All @@ -116,7 +116,7 @@ protected function putSourceIntoFinalDestination(FlysystemFile $videoTargetFile)
$streamStat = $videoTargetFile->write($this->videoSourceFile->read(), true);
$this->videoSourceFile->close();
$videoTargetFile->close();
if ($this->parameters->importMode->shallDeleteImported()) {
if ($this->parameters->importMode->shallDeleteImported) {
// This may throw an exception, if the original has been
// readable, but is not writable
// In this case, the media file will have been copied, but
Expand Down
48 changes: 17 additions & 31 deletions app/Actions/Photo/Strategies/ImportMode.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,27 @@

namespace App\Actions\Photo\Strategies;

class ImportMode
/**
* Define import mode.
*/
final readonly class ImportMode
{
public bool $shallDeleteImported;
public bool $shallSkipDuplicates;
public bool $shallImportViaSymlink;
public bool $shallResyncMetadata;

public function __construct(
protected readonly bool $deleteImported = false,
protected readonly bool $skipDuplicates = false,
protected bool $importViaSymlink = false,
protected bool $resyncMetadata = false,
bool $deleteImported = false,
bool $skipDuplicates = false,
bool $importViaSymlink = false,
bool $resyncMetadata = false,
) {
$this->shallDeleteImported = $deleteImported;
$this->shallSkipDuplicates = $skipDuplicates;
// avoid incompatible settings (delete originals takes precedence over symbolic links)
if ($deleteImported) {
$this->importViaSymlink = false;
}
$this->shallImportViaSymlink = $deleteImported ? false : $importViaSymlink;
// (re-syncing metadata makes no sense when importing duplicates)
if (!$skipDuplicates) {
$this->resyncMetadata = false;
}
}

public function shallDeleteImported(): bool
{
return $this->deleteImported;
}

public function shallSkipDuplicates(): bool
{
return $this->skipDuplicates;
}

public function shallImportViaSymlink(): bool
{
return $this->importViaSymlink;
}

public function shallResyncMetadata(): bool
{
return $this->resyncMetadata;
$this->shallResyncMetadata = !$skipDuplicates ? false : $resyncMetadata;
}
}

0 comments on commit 41bea7a

Please sign in to comment.