From d735be3200209847448945d6b94d4c127abcc8a3 Mon Sep 17 00:00:00 2001 From: Jared Whiklo Date: Wed, 17 Jan 2024 11:39:39 -0600 Subject: [PATCH] More PHP 8 and phpstan fixes --- composer.json | 2 +- src/AbstractManifest.php | 32 ++--- src/Bag.php | 235 +++++++++++++------------------ src/BagUtils.php | 16 +-- src/Commands/ValidateCommand.php | 4 +- src/Fetch.php | 143 +++++++++---------- src/PayloadManifest.php | 6 +- src/TagManifest.php | 8 +- tests/BagItTestFramework.php | 19 +-- 9 files changed, 212 insertions(+), 253 deletions(-) diff --git a/composer.json b/composer.json index 9be8071..64ea5d6 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,7 @@ }, "scripts": { "phpstan": [ - "php -d memory_limit=-1 ./vendor/bin/phpstan analyze -l 3 src tests" + "php -d memory_limit=-1 ./vendor/bin/phpstan analyze -l 5 src tests" ], "profile": [ "php -d xdebug.mode=profile -d xdebug.output_dir=mytracedir/ -d xdebug.start_with_request=yes -d xdebug.use_compression=true ./vendor/bin/phpunit" diff --git a/src/AbstractManifest.php b/src/AbstractManifest.php index f7f4337..dfa8334 100644 --- a/src/AbstractManifest.php +++ b/src/AbstractManifest.php @@ -19,51 +19,51 @@ abstract class AbstractManifest /** * The bag this manifest is part of. * - * @var \whikloj\BagItTools\Bag + * @var Bag */ - protected $bag; + protected Bag $bag; /** * The hash algorithm for this manifest. * * @var string */ - protected $algorithm; + protected string $algorithm; /** * Associative array where paths are keys and hashes are values. * * @var array */ - protected $hashes = []; + protected array $hashes = []; /** * Array of the same paths as in $hashes but normalized for case and characters to check for duplication. * * @var array */ - protected $normalizedPaths = []; + protected array $normalizedPaths = []; /** * The filename for this manifest. * * @var string */ - protected $filename; + protected string $filename; /** * Errors while validating this manifest. * * @var array */ - protected $manifestErrors = []; + protected array $manifestErrors = []; /** * Warnings generated while validating this manifest. * * @var array */ - protected $manifestWarnings = []; + protected array $manifestWarnings = []; /** * Errors/Warnings generated while loading. @@ -72,14 +72,14 @@ abstract class AbstractManifest * * @var array * Array of arrays with keys 'error' and 'warning' - * @see \whikloj\BagItTools\AbstractManifest::resetLoadIssues() + * @see AbstractManifest::resetLoadIssues */ - protected $loadIssues; + protected array $loadIssues; /** * Manifest constructor. * - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag this manifest is part of. * @param string $algorithm * The BagIt name of the hash algorithm. @@ -87,7 +87,7 @@ abstract class AbstractManifest * The manifest filename. * @param boolean $load * Whether we are loading an existing file - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read manifest file. */ protected function __construct(Bag $bag, string $algorithm, string $filename, bool $load = false) @@ -145,7 +145,7 @@ public function getWarnings(): array /** * Update the hashes for each path. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Error writing the manifest file to disk. */ public function update(): void @@ -201,7 +201,7 @@ public function getHashes(): array /** * Load the paths and hashes from the file on disk, does not validate. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read manifest file. */ protected function loadFile(): void @@ -254,7 +254,7 @@ protected function loadFile(): void /** * Utility to recreate the manifest file using the currently stored hashes. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If we can't write the manifest files. */ protected function writeToDisk(): void @@ -286,7 +286,7 @@ protected function writeToDisk(): void */ private function checkIncomingFilePath(string $filepath, int $lineCount): void { - if (substr($filepath, 0, 2) == "./") { + if (str_starts_with($filepath, "./")) { $this->addLoadWarning("Line $lineCount : Paths SHOULD not be relative"); } if (BagUtils::checkUnencodedFilepath($filepath)) { diff --git a/src/Bag.php b/src/Bag.php index a8a4cce..4f29181 100644 --- a/src/Bag.php +++ b/src/Bag.php @@ -54,26 +54,6 @@ class Bag 'bag-count', ]; - /** - * Reserved element names for Bag-info fields. - */ - private const BAG_INFO_RESERVED_ELEMENTS = [ - 'source-organization', - 'organization-address', - 'contact-name', - 'contact-phone', - 'contact-email', - 'external-description', - 'bagging-date', - 'external-identifier', - 'payload-oxum', - 'bag-size', - 'bag-group-identifier', - 'bag-count', - 'internal-sender-identifier', - 'internal-sender-description', - ]; - /** * Fields you can't set because we generate them on $bag->update(). */ @@ -112,10 +92,6 @@ class Bag 'LPT2', 'LPT3', 'LPT4', 'LPT5', 'LPT6', 'LPT7', 'LPT8', 'LPT9', ]; - private const WINDOWS_PATH_CHARACTERS = [ - '<', '>', ':', '"', '/', '|', '?', '*', - ]; - /** * Extensions which map to a tar file. */ @@ -148,49 +124,49 @@ class Bag * * @var array */ - private $packageExtensions; + private array $packageExtensions; /** * Array of current bag version with keys 'major' and 'minor'. * * @var array */ - private $currentVersion = self::DEFAULT_BAGIT_VERSION; + private array $currentVersion = self::DEFAULT_BAGIT_VERSION; /** * Current bag file encoding. * * @var string */ - private $currentFileEncoding = self::DEFAULT_FILE_ENCODING; + private string $currentFileEncoding = self::DEFAULT_FILE_ENCODING; /** * Array of payload manifests. * * @var array */ - private $payloadManifests; + private array $payloadManifests; /** * Array of tag manifests. * * @var array */ - private $tagManifests; + private array $tagManifests; /** * List of relative file paths for all files. * * @var array */ - private $payloadFiles; + private array $payloadFiles; /** * Reference to a Fetch file or null if not used. * - * @var \whikloj\BagItTools\Fetch + * @var Fetch|null */ - private $fetchFile = null; + private ?Fetch $fetchFile = null; /** * The absolute path to the root of the bag, all other file paths are @@ -199,14 +175,14 @@ class Bag * * @var string */ - private $bagRoot; + private string $bagRoot; /** * Is this an extended bag? * * @var boolean */ - private $isExtended = false; + private bool $isExtended = false; /** * The valid algorithms from the current version of PHP filtered to those @@ -215,35 +191,35 @@ class Bag * * @var array */ - private $validHashAlgorithms; + private array $validHashAlgorithms; /** * Errors when validating a bag. * * @var array */ - private $bagErrors; + private array $bagErrors; /** * Warnings when validating a bag. * * @var array */ - private $bagWarnings; + private array $bagWarnings; /** * Have we changed the bag and not written it to disk? * * @var boolean */ - private $changed = false; + private bool $changed = false; /** * Bag Info data. * * @var array */ - private $bagInfoData = []; + private array $bagInfoData = []; /** * Unique array of all Bag info tags/values. Tags are stored once in lower case with an array of all instances @@ -251,14 +227,14 @@ class Bag * * @var array */ - private $bagInfoTagIndex = []; + private array $bagInfoTagIndex = []; /** - * Did we load this from disk. + * Did we load this from disk? * * @var boolean */ - private $loaded; + private bool $loaded; /** * Bag constructor. @@ -268,9 +244,9 @@ class Bag * @param boolean $new * Are we making a new bag? * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems accessing a file. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Bag directory exists for new bag or various issues for loading an existing bag. */ private function __construct(string $rootPath, bool $new = true) @@ -301,9 +277,9 @@ private function __construct(string $rootPath, bool $new = true) * * @param string $rootPath * Path to the new bag, must not exist - * @return \whikloj\BagItTools\Bag + * @return Bag * The bag. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If we can't create the directory. */ public static function create(string $rootPath): Bag @@ -316,9 +292,9 @@ public static function create(string $rootPath): Bag * * @param string $rootPath * Path to the existing bag. - * @return \whikloj\BagItTools\Bag + * @return Bag * The bag object. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If we can't read files in the bag. */ public static function load(string $rootPath): Bag @@ -335,7 +311,7 @@ public static function load(string $rootPath): Bag * * @return boolean * True if the bag is valid - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Problems writing to disk. */ public function isValid(): bool @@ -351,7 +327,7 @@ public function isValid(): bool $this->mergeErrors($this->fetchFile->getErrors()); } $manifests = array_values($this->payloadManifests); - if (is_array($this->tagManifests) && count($this->tagManifests) > 0) { + if (count($this->tagManifests) > 0) { // merge in the tag manifests so we can do them all at once. $manifests = array_merge($manifests, array_values($this->tagManifests)); } @@ -363,26 +339,10 @@ public function isValid(): bool return (count($this->bagErrors) == 0); } - /** - * Validate the bag as it appears on disk. - * - * @return bool - * True if bag is valid. - * @throws \whikloj\BagItTools\Exceptions\BagItException - * If problems updating the bag. - * @deprecated 4.1.0 Name change of same function to better signify the boolean response - * @see \whikloj\BagItTools\Bag::isValid() - * @codeCoverageIgnore - */ - public function validate(): bool - { - return $this->isValid(); - } - /** * Write the updated BagIt files to disk. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Errors with writing files to disk. */ public function update(): void @@ -407,7 +367,7 @@ public function update(): void /** * This does cleanup functions related to packaging, for example deleting downloaded files referenced in fetch.txt * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting files from filesystem. */ public function finalize(): void @@ -425,7 +385,7 @@ public function finalize(): void * * @param string $filepath * The full path to create the archive at. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Problems creating the archive. */ public function package(string $filepath): void @@ -448,9 +408,9 @@ public function package(string $filepath): void * @param string $dest * Relative path for the destination. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Source file does not exist or the destination is outside the data directory. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues writing to the filesystem. */ public function addFile(string $source, string $dest): void @@ -473,8 +433,8 @@ public function addFile(string $source, string $dest): void throw new BagItException("File $dest already exists in the bag."); } $dirname = dirname($fullDest); - if (substr($this->makeRelative($dirname), 0, 5) == "data/") { - // Create any missing missing directories inside data. + if (str_starts_with($this->makeRelative($dirname), "data/")) { + // Create any missing directories inside data. if (!file_exists($dirname)) { BagUtils::checkedMkdir($dirname, 0777, true); } @@ -488,7 +448,7 @@ public function addFile(string $source, string $dest): void * * @param string $dest * The relative path of the file. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting the file. */ public function removeFile(string $dest): void @@ -513,9 +473,9 @@ public function removeFile(string $dest): void * The contents of the file. * @param string $dest * The name of the file in the bag. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Generated source file does not exist or the destination is outside the data directory. Should not occur. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues creating/deleting temporary file. */ public function createFile(string $string, string $dest): void @@ -705,7 +665,7 @@ public function removeBagInfoTagValue(string $tag, string $value, bool $case_sen * The tag to add. * @param string $value * The value to add. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * When you try to set an auto-generated tag value. */ public function addBagInfoTag(string $tag, string $value): void @@ -725,7 +685,7 @@ public function addBagInfoTag(string $tag, string $value): void * * @param array $tags * Associative array of tag => value - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * When you try to set an auto-generated tag value. */ public function addBagInfoTags(array $tags): void @@ -777,7 +737,7 @@ private function addBagInfoTagsInternal(array $tags): void * * @param string $encoding * The MIME name of the character set to encode with. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If we don't support the requested character set. */ public function setFileEncoding(string $encoding): void @@ -848,7 +808,7 @@ public function algorithmIsSupported(string $algorithm): bool * * @param string $algorithm * Algorithm to add. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Asking for an unsupported algorithm. */ public function addAlgorithm(string $algorithm): void @@ -874,7 +834,7 @@ public function addAlgorithm(string $algorithm): void * * @param string $algorithm * Algorithm to remove - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Trying to remove the last algorithm or asking for an unsupported algorithm. */ public function removeAlgorithm(string $algorithm): void @@ -906,9 +866,9 @@ public function removeAlgorithm(string $algorithm): void * * @param string $algorithm * Algorithm to use. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems removing/reading/ - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Asking for an unsupported algorithm. */ public function setAlgorithm(string $algorithm): void @@ -925,9 +885,9 @@ public function setAlgorithm(string $algorithm): void * * @param array $algorithms * Array of algorithms to use. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems removing/reading/creating the new payload/tag manifest files. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If an unsupported algorithm is provided. */ public function setAlgorithms(array $algorithms): void @@ -945,7 +905,7 @@ public function setAlgorithms(array $algorithms): void * * @param array $algorithms * Array of algorithms using their internal names. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Errors */ private function setAlgorithmsInternal(array $algorithms): void @@ -977,10 +937,10 @@ private function setAlgorithmsInternal(array $algorithms): void * The destination path in the bag. * @param null|int $size * Size of the file to be stored in the fetch file, if desired. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * On errors adding the file. */ - public function addFetchFile(string $url, string $destination, int $size = null) + public function addFetchFile(string $url, string $destination, int $size = null): void { if (!isset($this->fetchFile)) { $this->fetchFile = new Fetch($this, false); @@ -1002,7 +962,7 @@ public function listFetchFiles(): array /** * Wipe the fetch file data * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting files/directories from the filesystem. */ public function clearFetch(): void @@ -1019,7 +979,7 @@ public function clearFetch(): void * * @param string $url * The url to delete. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems deleting the file. */ public function removeFetchFile(string $url): void @@ -1141,7 +1101,7 @@ public function getPayloadManifests(): array */ public function getTagManifests(): ?array { - return ($this->tagManifests ?? null); + return $this->tagManifests ?? null; } /** @@ -1182,7 +1142,7 @@ public function pathInBagData(string $filepath): bool { $external = $this->makeAbsolute($filepath); $relative = $this->makeRelative($external); - return ($relative !== "" && substr($relative, 0, 5) === "data/"); + return $relative !== "" && str_starts_with($relative, "data/"); } @@ -1196,7 +1156,7 @@ public function pathInBagData(string $filepath): bool public function checkForEmptyDir(string $path): void { $parentPath = dirname($path); - if (substr($this->makeRelative($parentPath), 0, 5) == "data/") { + if (str_starts_with($this->makeRelative($parentPath), "data/")) { $files = scandir($parentPath); $payload = array_diff($files, [".", ".."]); if (count($payload) == 0) { @@ -1208,9 +1168,9 @@ public function checkForEmptyDir(string $path): void /** * Upgrade an older bag to comply with the 1.0 specification. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If the bag cannot be upgraded for some reason. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues writing files to the filesystem. */ public function upgrade(): void @@ -1240,7 +1200,7 @@ public function upgrade(): void /** * Load a bag from disk. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If a file cannot be read. */ private function loadBag(): void @@ -1265,9 +1225,9 @@ private function loadBag(): void /** * Create a new bag and output the default parts. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * If the bag root directory already exists. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems writing to the filesystem. */ private function createNewBag(): void @@ -1296,7 +1256,7 @@ private function resetErrorsAndWarnings(): void /** * Update a fetch.txt if it exists. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException */ private function updateFetch(): void { @@ -1314,7 +1274,7 @@ private function updateFetch(): void * @return bool * Does bag-info.txt exists. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read bag-info.txt */ private function loadBagInfo(): bool @@ -1338,7 +1298,7 @@ private function loadBagInfo(): bool } $line = $this->decodeText($line) . PHP_EOL; $lineLength = strlen($line); - if (substr($line, 0, 2) === " " || $line[0] == "\t") { + if (str_starts_with($line, " ") || $line[0] == "\t") { // Continuation of a line if (count($bagData) > 0) { $previousValue = $bagData[count($bagData) - 1]['value']; @@ -1442,7 +1402,7 @@ private function updateBagInfoIndex(): void } /** - * Internal case insensitive search of bag info. + * Internal case-insensitive search of bag info. * * @param string $internal_tag * Trimmed and lowercase tag. @@ -1457,7 +1417,7 @@ private function bagInfoTagExists(string $internal_tag): bool /** * Write the contents of the bag-info array to disk. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues writing the file to disk. */ private function updateBagInfo(): void @@ -1536,7 +1496,7 @@ private function convertToHumanReadable(int $bytes): string /** * Remove the bag-info.txt file and data. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to delete the bag-info.txt file. */ private function removeBagInfo(): void @@ -1633,7 +1593,7 @@ private static function wrapAtLength(string $text, int $length): array * @return boolean * Are there any tag manifest files. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems with glob() pattern or loading manifest. */ private function loadTagManifests(): bool @@ -1673,7 +1633,7 @@ private function ensureTagManifests(): void /** * Run update against the tag manifests. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting the tag manifest files. */ private function updateTagManifests(): void @@ -1683,8 +1643,7 @@ private function updateTagManifests(): void } $this->clearTagManifests(); $this->ensureTagManifests(); - $hashes = (is_array($this->payloadManifests) ? $this->payloadManifests : - [self::DEFAULT_HASH_ALGORITHM => ""]); + $hashes = $this->payloadManifests ?? [self::DEFAULT_HASH_ALGORITHM => ""]; $hashes = array_diff_key($hashes, $this->tagManifests); foreach (array_keys($hashes) as $hash) { $this->tagManifests[$hash] = new TagManifest($this, $hash); @@ -1697,7 +1656,7 @@ private function updateTagManifests(): void /** * Remove all tagmanifest files. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Errors with glob() pattern or deleting files from filesystem. */ private function clearTagManifests(): void @@ -1712,7 +1671,7 @@ private function clearTagManifests(): void * * @param array $exclusions * Hash algorithm names of manifests to preserve. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting files from the filesystem. */ private function removeAllTagManifests(array $exclusions = []): void @@ -1732,7 +1691,7 @@ private function removeAllTagManifests(array $exclusions = []): void * * @param string $internal_name * The hash name to remove. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems deleting the tag manifest file. */ private function removeTagManifest(string $internal_name): void @@ -1748,9 +1707,9 @@ private function removeTagManifest(string $internal_name): void /** * Load all payload manifests found on disk. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems with glob() pattern - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Invalid algorithm detected. */ private function loadPayloadManifests(): void @@ -1789,7 +1748,7 @@ private function loadPayloadManifests(): void /** * Run update against the payload manifests. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting payload manifest files. */ private function updatePayloadManifests(): void @@ -1811,7 +1770,7 @@ private function updatePayloadManifests(): void /** * Remove all manifest files. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Errors with glob() pattern. */ private function clearPayloadManifests(): void @@ -1825,7 +1784,7 @@ private function clearPayloadManifests(): void * * @param array $exclusions * Hash algorithm names of manifests to preserve. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting a file. */ private function removeAllPayloadManifests(array $exclusions = []): void @@ -1842,7 +1801,7 @@ private function removeAllPayloadManifests(array $exclusions = []): void * * @param string $internal_name * The hash name to remove. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues deleting the payload manifest file. */ private function removePayloadManifest(string $internal_name): void @@ -1858,7 +1817,7 @@ private function removePayloadManifest(string $internal_name): void /** * Load the bagit.txt on disk. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Can't read the file on disk. */ private function loadBagIt(): void @@ -1931,7 +1890,7 @@ function (&$item) { /** * Update the bagit.txt on disk. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues putting contents to the file. */ private function updateBagIt(): void @@ -1958,7 +1917,7 @@ private function updateBagIt(): void /** * Load a fetch.txt if it exists. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read fetch.txt for existing bag. */ private function loadFetch(): void @@ -1975,9 +1934,9 @@ private function loadFetch(): void * * @param string $filename * The archive filename. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems creating the archive. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Unable to determine archive format. */ private function makePackage(string $filename): void @@ -1996,7 +1955,7 @@ private function makePackage(string $filename): void * * @param string $filename * The archive filename. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems creating the archive. */ private function makeZip(string $filename): void @@ -2020,16 +1979,13 @@ private function makeZip(string $filename): void * * @param string $filename * The archive filename. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems creating the archive. */ private function makeTar(string $filename): void { $compression = self::extensionTarCompression($filename); $tar = new Archive_Tar($filename, $compression); - if ($tar === false) { - throw new FilesystemException("Error creating Tar file."); - } $parent = $this->getParentDir(); $files = BagUtils::getAllFiles($this->bagRoot); if (!$tar->createModify($files, "", $parent)) { @@ -2055,9 +2011,9 @@ private function getParentDir(): string * The full path to the archive file. * @return string * The full path to extracted bag. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems accessing and/or uncompressing files on filesystem. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Unable to determine correct archive format or file does not exist. */ private static function uncompressBag(string $filepath): string @@ -2083,14 +2039,14 @@ private static function uncompressBag(string $filepath): string * The full path to the zip file. * @return string * The path the archive file was extracted to. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems extracting the zip file. */ private static function unzipBag(string $filename): string { $zip = new ZipArchive(); $res = $zip->open($filename); - if ($res === false) { + if ($res !== true) { throw new FilesystemException("Unable to unzip $filename"); } $directory = self::extractDir(); @@ -2106,7 +2062,7 @@ private static function unzipBag(string $filename): string * The fullpath to the tar file. * @return string * The path the archive file was extracted to. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems extracting the zip file. */ private static function untarBag(string $filename): string @@ -2132,8 +2088,7 @@ private static function untarBag(string $filename): string private static function extensionTarCompression(string $filename): ?string { $filename = strtolower(basename($filename)); - return (substr($filename, -3) == 'bz2' ? 'bz2' : (substr($filename, -2) == 'gz' ? 'gz' : - null)); + return (str_ends_with($filename, '.bz2') ? 'bz2' : (str_ends_with($filename, '.gz') ? 'gz' : null)); } /** @@ -2141,7 +2096,7 @@ private static function extensionTarCompression(string $filename): ?string * * @return string * The path to a new temporary directory. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues creating/deleting files on filesystem. */ private static function extractDir(): string @@ -2186,7 +2141,7 @@ private static function hasExtension(string $filepath, array $extensions): bool $filename = strtolower(basename($filepath)); foreach ($extensions as $extension) { $extension = ".$extension"; - if (substr($filename, -strlen($extension)) === $extension) { + if (str_ends_with($filename, $extension)) { return true; } } @@ -2200,7 +2155,7 @@ private static function hasExtension(string $filepath, array $extensions): bool * The temporary directory. * @return string * The bag directory. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Find more or less than one directory (not including . and ..) */ private static function getDirectory(string $filepath): string @@ -2226,7 +2181,7 @@ private static function getDirectory(string $filepath): string * * @param string $filePattern * The file pattern. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems matching or deleting files. */ private function clearFilesOfPattern(string $filePattern): void @@ -2334,7 +2289,7 @@ private function hasHash(string $internal_name): bool * Output of getHashName * @return boolean * Do we support the algorithm - * @see \whikloj\BagItTools\Bag::getHashName() + * @see Bag::getHashName */ private function hashIsSupported(string $internal_name): bool { @@ -2401,7 +2356,7 @@ private static function shouldNotRepeatBagInfoExists(string $key, array $bagData private static function determineHashFromFilename(string $filepath): ?string { $filename = basename($filepath); - return preg_match('~\-([a-z0-9]+)\.txt$~', $filename, $matches) ? $matches[1] : null; + return preg_match('~-([a-z0-9]+)\.txt$~', $filename, $matches) ? $matches[1] : null; } diff --git a/src/BagUtils.php b/src/BagUtils.php index b964f52..b39a792 100644 --- a/src/BagUtils.php +++ b/src/BagUtils.php @@ -81,7 +81,7 @@ public static function baseInData(string $path): string * @return array * Array of matches. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Error in matching pattern. */ public static function findAllByPattern(string $pattern): array @@ -208,7 +208,7 @@ public static function getAllFiles(string $directory, array $exclusions = []): a * The source path. * @param string $destFile * The destination path. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If the copy() call fails. * @see \copy() */ @@ -228,7 +228,7 @@ public static function checkedCopy(string $sourceFile, string $destFile): void * The permissions on the new directories. * @param bool $recursive * Whether to create intermediate directories automatically. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If the mkdir() call fails. * @see \mkdir() */ @@ -250,7 +250,7 @@ public static function checkedMkdir(string $path, int $mode = 0777, bool $recurs * Flags to pass on to file_put_contents. * @return int * Number of bytes written to the file. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * On any error putting the contents to the file. * @see \file_put_contents() */ @@ -268,7 +268,7 @@ public static function checkedFilePut(string $path, string $contents, int $flags * * @param string $path * The path to remove. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If the call to unlink() fails. * @see \unlink() */ @@ -288,7 +288,7 @@ public static function checkedUnlink(string $path): void * The prefix to the file. * @return string * The path to the temporary filename. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues creating the file. * @see \tempnam() */ @@ -309,7 +309,7 @@ public static function checkedTempnam(string $directory = "", string $prefix = " * The file pointer. * @param string $content * The content to write. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problem writing to file. */ public static function checkedFwrite($fp, string $content): void @@ -320,7 +320,7 @@ public static function checkedFwrite($fp, string $content): void throw new FilesystemException("Error writing to file"); } } catch (TypeError $e) { - throw new FilesystemException("Error writing to file"); + throw new FilesystemException("Error writing to file", $e->getCode(), $e); } } diff --git a/src/Commands/ValidateCommand.php b/src/Commands/ValidateCommand.php index 8685529..b772117 100644 --- a/src/Commands/ValidateCommand.php +++ b/src/Commands/ValidateCommand.php @@ -22,7 +22,7 @@ class ValidateCommand extends Command /** * {@inheritdoc} */ - protected function configure() + protected function configure(): void { $this->setName('validate') ->setDescription('Validate a BagIt bag.') @@ -50,7 +50,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $io->error("Path $path does not exist, cannot validate."); } else { try { - if (isset($realpath) && $realpath !== false) { + if (isset($realpath)) { $path = $realpath; } $bag = Bag::load($path); diff --git a/src/Fetch.php b/src/Fetch.php index 1b6232c..e1cb41f 100644 --- a/src/Fetch.php +++ b/src/Fetch.php @@ -4,6 +4,8 @@ namespace whikloj\BagItTools; +use CurlHandle; +use CurlMultiHandle; use Normalizer; use whikloj\BagItTools\Exceptions\BagItException; use whikloj\BagItTools\Exceptions\FilesystemException; @@ -25,51 +27,51 @@ class Fetch /** * The bag this fetch file is part of * - * @var \whikloj\BagItTools\Bag + * @var Bag */ - private $bag; + private Bag $bag; /** * The current absolute path to the fetch.txt file. * * @var string */ - private $filename; + private string $filename; /** * Information from the fetch.txt, array of arrays with keys 'uri', 'size', and 'destination' * * @var array */ - private $files; + private array $files; /** * Errors * * @var array */ - private $fetchErrors = []; + private array $fetchErrors = []; /** * Urls and Files that validated and should be downloaded. * * @var array */ - private $downloadQueue = []; + private array $downloadQueue = []; /** * Curl version number string. * * @var string */ - private $curlVersion; + private mixed $curlVersion; /** * Standard curl options to use. * * @var array */ - private $curlOptions = [ + private array $curlOptions = [ CURLOPT_CONNECTTIMEOUT => 10, CURLOPT_RETURNTRANSFER => true, ]; @@ -77,11 +79,11 @@ class Fetch /** * Fetch constructor. * - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag this fetch is part of. * @param bool $load * Whether to load a fetch.txt - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read fetch.txt for existing bag. */ public function __construct(Bag $bag, bool $load = false) @@ -109,10 +111,10 @@ public function getData(): array /** * Download the files. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Unable to open file handle to save to. */ - public function downloadAll() + public function downloadAll(): void { if (count($this->getErrors()) > 0) { throw new BagItException("The fetch.txt file has errors, unable to download files."); @@ -137,10 +139,10 @@ public function downloadAll() * @param array $fetchData * Array with mandatory keys 'uri' and 'destination' and optional key 'size'. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * For all validation errors. */ - private function validateData(array $fetchData) + private function validateData(array $fetchData): void { $uri = $fetchData['uri']; $dest = BagUtils::baseInData($fetchData['destination']); @@ -165,10 +167,10 @@ private function validateData(array $fetchData) * The bag destination path for the file. * @param int|null $size * The expected size of the file, or null for unknown. - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Errors with adding this file to your fetch file. */ - public function addFile(string $url, string $destination, int $size = null): void + public function addFile(string $url, string $destination, ?int $size = null): void { $fetchData = [ 'uri' => $url, @@ -187,7 +189,7 @@ public function addFile(string $url, string $destination, int $size = null): voi * @param array $fetchData * Array of data with keys 'uri', 'destination' and optionally 'size'. * - * @throws \whikloj\BagItTools\Exceptions\BagItException + * @throws BagItException * Problems downloading the file. */ public function download(array $fetchData): void @@ -227,7 +229,7 @@ public function download(array $fetchData): void * * @param string $url * The url to remove. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Issues removing the file from the filesystem. */ public function removeFile(string $url): void @@ -251,7 +253,7 @@ public function removeFile(string $url): void /** * Update the fetch.txt on disk with the fetch file records. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If we can't write to disk. */ public function update(): void @@ -263,7 +265,7 @@ public function update(): void * Remove any downloaded files referenced in fetch.txt. This is called before we package up the Bag or finalize the * directory. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems removing file from filesystem. */ public function cleanup(): void @@ -281,7 +283,7 @@ public function cleanup(): void /** * Clean up any downloaded files and then wipe the internal data array. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Problems removing file from filesystem. */ public function clearData(): void @@ -325,7 +327,7 @@ public function reservedPath(string $dest): bool /** * Load an existing fetch.txt * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read the fetch.txt file. */ private function loadFiles(): void @@ -345,7 +347,7 @@ private function loadFiles(): void if (empty($line)) { continue; } - if (preg_match("~^([^\s]+)\s+(\d+|\-)\s+(.*)$~", $line, $matches)) { + if (preg_match("~^(\S+)\s+(\d+|\-)\s+(.*)$~", $line, $matches)) { // We just store what you give us, we'll validate when you load the contents to validate the bag. $uri = $matches[1]; $filesize = $matches[2]; @@ -379,7 +381,7 @@ private function loadFiles(): void * The content from curl. * @param string $destination * The relative path to the final file. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Trouble writing to disk. */ private function saveFileData(string $content, string $destination): void @@ -388,8 +390,8 @@ private function saveFileData(string $content, string $destination): void $fullDest = $this->bag->makeAbsolute($destination); $fullDest = Normalizer::normalize($fullDest); $dirname = dirname($fullDest); - if (substr($this->bag->makeRelative($dirname), 0, 5) == "data/") { - // Create any missing missing directories inside data. + if (str_starts_with($this->bag->makeRelative($dirname), "data/")) { + // Create any missing directories inside data. if (!file_exists($dirname)) { BagUtils::checkedMkdir($dirname, 0777, true); } @@ -401,24 +403,18 @@ private function saveFileData(string $content, string $destination): void /** * Create a cUrl multi handler. * - * @return false|resource - * False on error, otherwise the cUrl resource + * @return CurlMultiHandle + * The cUrl resource */ - private function createMultiCurl() + private function createMultiCurl(): CurlMultiHandle { $mh = curl_multi_init(); if ( version_compare('7.62.0', $this->curlVersion) > 0 && version_compare('7.43.0', $this->curlVersion) <= 0 ) { - // Try enabling HTTP/1.1 pipelining and HTTP/2 multiplexing if our version is less than 7.62 - // CURLPIPE_HTTP1 is deprecated in PHP 7.4 - if (version_compare('7.4', PHP_VERSION) > 0) { - $values = CURLPIPE_HTTP1 | CURLPIPE_MULTIPLEX; - } else { - $values = CURLPIPE_MULTIPLEX; - } - curl_multi_setopt($mh, CURLMOPT_PIPELINING, $values); + // Try enabling HTTP/2 multiplexing if our version is less than 7.62 + curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX); } if (version_compare('7.30.0', $this->curlVersion) <= 0) { // Set a limit to how many connections can be opened. @@ -436,12 +432,15 @@ private function createMultiCurl() * If this is a download() call versus a downloadAll() call. * @param int|null $size * Expected download size or null if unknown - * @return false|resource + * @return CurlHandle|false * False on error, otherwise the cUl resource. */ - private function createCurl(string $url, bool $single = false, ?int $size = null) + private function createCurl(string $url, bool $single = false, ?int $size = null): CurlHandle|bool { $ch = curl_init($url); + if ($ch === false) { + return false; + } $options = $this->curlOptions; if ($single === true) { // If this is set during curl_multi_exec, it swallows error messages. @@ -484,7 +483,7 @@ private static function curlXferInfo(int $expectDl, int $currDl): int /** * Download files using Curl. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to open a file handle to download to. */ private function downloadFiles(): void @@ -493,49 +492,47 @@ private function downloadFiles(): void $mh = $this->createMultiCurl(); $curl_handles = []; $destinations = []; - if ($mh !== false) { - foreach ($this->downloadQueue as $key => $download) { - $fullPath = $this->bag->makeAbsolute($download['destination']); - // Don't download again. - if (!file_exists($fullPath)) { - $destinations[$key] = $fullPath; - $size = is_int($download['size']) ? $download['size'] : null; - $curl_handles[$key] = $this->createCurl($download['uri'], false, $size); - curl_multi_add_handle($mh, $curl_handles[$key]); - } + foreach ($this->downloadQueue as $key => $download) { + $fullPath = $this->bag->makeAbsolute($download['destination']); + // Don't download again. + if (!file_exists($fullPath)) { + $destinations[$key] = $fullPath; + $size = is_int($download['size']) ? $download['size'] : null; + $curl_handles[$key] = $this->createCurl($download['uri'], false, $size); + curl_multi_add_handle($mh, $curl_handles[$key]); } - $running = null; - do { - $status = curl_multi_exec($mh, $running); - while (false !== curl_multi_info_read($mh)) { - // Need to read the information or we lose any callback aborted messages. - } - } while ($running && $status == CURLM_OK); - if ($status != CURLM_OK) { - $this->addError("Problems with multifile download."); + } + $running = null; + do { + $status = curl_multi_exec($mh, $running); + while (false !== curl_multi_info_read($mh)) { + // Need to read the information or we lose any callback aborted messages. } - $handle_count = count($curl_handles); - for ($x = 0; $x < $handle_count; $x += 1) { - $error = curl_error($curl_handles[$x]); - $url = curl_getinfo($curl_handles[$x], CURLINFO_EFFECTIVE_URL); - if (!empty($error)) { - $this->addError("Failed to fetch URL ($url) : $error"); - } else { - $content = curl_multi_getcontent($curl_handles[$x]); - $this->saveFileData($content, $destinations[$x]); - } - curl_multi_remove_handle($mh, $curl_handles[$x]); - curl_close($curl_handles[$x]); + } while ($running && $status == CURLM_OK); + if ($status != CURLM_OK) { + $this->addError("Problems with multifile download."); + } + $handle_count = count($curl_handles); + for ($x = 0; $x < $handle_count; $x += 1) { + $error = curl_error($curl_handles[$x]); + $url = curl_getinfo($curl_handles[$x], CURLINFO_EFFECTIVE_URL); + if (!empty($error)) { + $this->addError("Failed to fetch URL ($url) : $error"); + } else { + $content = curl_multi_getcontent($curl_handles[$x]); + $this->saveFileData($content, $destinations[$x]); } - curl_multi_close($mh); + curl_multi_remove_handle($mh, $curl_handles[$x]); + curl_close($curl_handles[$x]); } + curl_multi_close($mh); } } /** * Utility to recreate the fetch file using the currently stored files. * - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * If we can't write the fetch file. */ private function writeToDisk(): void diff --git a/src/PayloadManifest.php b/src/PayloadManifest.php index 98c91f4..d72f35e 100644 --- a/src/PayloadManifest.php +++ b/src/PayloadManifest.php @@ -4,6 +4,8 @@ namespace whikloj\BagItTools; +use whikloj\BagItTools\Exceptions\FilesystemException; + /** * Payload Manifest extension of AbstractManifest. * @@ -16,13 +18,13 @@ class PayloadManifest extends AbstractManifest /** * PayloadManifest constructor. * - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag this manifest is part of. * @param string $algorithm * The BagIt name of the hash algorithm. * @param bool $load * Whether we are loading an existing file - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read manifest file. */ public function __construct(Bag $bag, string $algorithm, bool $load = false) diff --git a/src/TagManifest.php b/src/TagManifest.php index a3d6593..70d67fc 100644 --- a/src/TagManifest.php +++ b/src/TagManifest.php @@ -4,6 +4,8 @@ namespace whikloj\BagItTools; +use whikloj\BagItTools\Exceptions\FilesystemException; + /** * Tag Manifest extension of AbstractManifest class. * @@ -16,13 +18,13 @@ class TagManifest extends AbstractManifest /** * PayloadManifest constructor. * - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag this manifest is part of. * @param string $algorithm * The BagIt name of the hash algorithm. * @param boolean $load * Whether we are loading an existing file - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to read manifest file. */ public function __construct(Bag $bag, string $algorithm, bool $load = false) @@ -81,6 +83,6 @@ public function validate(): void */ private function isTagManifest(string $filepath): bool { - return (substr(basename($filepath), 0, 11) == "tagmanifest"); + return str_starts_with(basename($filepath), "tagmanifest"); } } diff --git a/tests/BagItTestFramework.php b/tests/BagItTestFramework.php index 9403d8a..46108c3 100644 --- a/tests/BagItTestFramework.php +++ b/tests/BagItTestFramework.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use ReflectionClass; +use ReflectionException; use ReflectionMethod; use whikloj\BagItTools\Bag; use whikloj\BagItTools\BagUtils; @@ -70,7 +71,7 @@ class BagItTestFramework extends TestCase * the test throws an exception to ensure it gets deleted. * @var string */ - protected $tmpdir; + protected string $tmpdir; /** * {@inheritdoc} @@ -87,7 +88,7 @@ public function setUp(): void public function tearDown(): void { parent::tearDown(); - if (isset($this->tmpdir) && file_exists($this->tmpdir)) { + if (file_exists($this->tmpdir)) { self::deleteDirAndContents($this->tmpdir); } } @@ -97,7 +98,7 @@ public function tearDown(): void * * @return string * The filename. - * @throws \whikloj\BagItTools\Exceptions\FilesystemException + * @throws FilesystemException * Unable to generate a new temporary directory. */ protected function getTempName(): string @@ -161,6 +162,8 @@ protected function prepareExtendedTestBag(): string * The source directory. * @return string * The path to the copy of the bag. + * @throws FilesystemException + * Unable to create temporary directory. */ protected function copyTestBag(string $testDir): string { @@ -215,10 +218,10 @@ private static function copyDir(string $src, string $dest): void * @param string $method * Method to get. * - * @return \ReflectionMethod + * @return ReflectionMethod * Reflection of the method. * - * @throws \ReflectionException + * @throws ReflectionException */ protected static function getReflectionMethod(string $class, string $method): ReflectionMethod { @@ -230,7 +233,7 @@ protected static function getReflectionMethod(string $class, string $method): Re /** * Assert the encoding in the bagit.txt is X and the version is 1.0 - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag. * @param string $version_string * The BagIt version. @@ -242,7 +245,7 @@ protected function assertBagItFileVersion(Bag $bag, string $version_string): voi /** * Assert the encoding in the bagit.txt is X and the version is 1.0 - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag. * @param string $encoding * The file encoding. @@ -254,7 +257,7 @@ protected function assertBagItFileEncoding(Bag $bag, string $encoding): void /** * Assert the version and encoding in the actual bagit.txt on disk - * @param \whikloj\BagItTools\Bag $bag + * @param Bag $bag * The bag. * @param string|null $version_string * The version string or null to use the default.