diff --git a/README.md b/README.md index 4874667..6c96014 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # BagItTools [![Minimum PHP Version](https://img.shields.io/badge/php-%3E%3D%208.0-8892BF.svg?style=flat-square)](https://php.net/) -[![Github Actions](https://github.com/whikloj/BagItTools/workflows/Build/badge.svg?branch=main)](https://github.com/whikloj/BagItTools/actions?query=workflow%3A%22Build%22+branch%3Amain) +[![Github Actions](https://github.com/whikloj/BagItTools/actions/workflows/main.yml/badge.svg)](https://github.com/whikloj/BagItTools/actions/workflows/main.yml) [![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg?style=flat-square)](./LICENSE) [![codecov](https://codecov.io/gh/whikloj/BagItTools/branch/main/graph/badge.svg)](https://codecov.io/gh/whikloj/BagItTools) diff --git a/composer.json b/composer.json index b8806c8..20fc500 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,7 @@ }, "scripts": { "phpstan": [ - "php -d memory_limit=-1 ./vendor/bin/phpstan analyze -l 5 src tests" + "php -d memory_limit=-1 ./vendor/bin/phpstan analyze -l 7 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 dfa8334..471e9ff 100644 --- a/src/AbstractManifest.php +++ b/src/AbstractManifest.php @@ -33,14 +33,14 @@ abstract class AbstractManifest /** * Associative array where paths are keys and hashes are values. * - * @var array + * @var array */ protected array $hashes = []; /** * Array of the same paths as in $hashes but normalized for case and characters to check for duplication. * - * @var array + * @var array */ protected array $normalizedPaths = []; @@ -54,14 +54,14 @@ abstract class AbstractManifest /** * Errors while validating this manifest. * - * @var array + * @var array> */ protected array $manifestErrors = []; /** * Warnings generated while validating this manifest. * - * @var array + * @var array> */ protected array $manifestWarnings = []; @@ -70,7 +70,7 @@ abstract class AbstractManifest * Because of the path key in the hash array if there are multiple entries for a path we need to track it during * load but present it at validate(). * - * @var array + * @var array>> * Array of arrays with keys 'error' and 'warning' * @see AbstractManifest::resetLoadIssues */ @@ -125,7 +125,7 @@ public function getFilename(): string /** * Return the array of errors. * - * @return array + * @return array> */ public function getErrors(): array { @@ -135,7 +135,7 @@ public function getErrors(): array /** * Return the array of warnings. * - * @return array + * @return array> */ public function getWarnings(): array { @@ -186,7 +186,7 @@ public function validate(): void /** * Return the payload and hashes as an associative array. * - * @return array + * @return array * Array of paths => hashes */ public function getHashes(): array diff --git a/src/Bag.php b/src/Bag.php index bd35f9d..332baf6 100644 --- a/src/Bag.php +++ b/src/Bag.php @@ -72,7 +72,7 @@ class Bag * * @see https://tools.ietf.org/html/rfc8493#section-2.4 * - * @var array + * @var array */ private const HASH_ALGORITHMS = array( 'md5' => 'md5', @@ -229,7 +229,7 @@ class Bag /** * Have we changed the bag and not written it to disk? * - * @var boolean + * @var bool */ private bool $changed = false; @@ -263,7 +263,7 @@ class Bag /** * Array of BagIt profiles. - * @var array + * @var array */ private array $profiles = []; @@ -586,7 +586,7 @@ public function makeRelative(string $path): string /** * Return raw bag info data. * - * @return array + * @return array> * Bag Info data. */ public function getBagInfoData(): array @@ -613,7 +613,7 @@ public function hasBagInfoTag(string $tag): bool * * @param string $tag * Bag info tag to locate - * @return array + * @return array * Array of values for the tag. */ public function getBagInfoByTag(string $tag): array @@ -732,7 +732,7 @@ public function addBagInfoTag(string $tag, string $value): void /** * Add multiple bag info tags from an array. * - * @param array $tags + * @param array> $tags * Associative array of tag => value * @throws BagItException * When you try to set an auto-generated tag value. @@ -754,7 +754,7 @@ public function addBagInfoTags(array $tags): void /** * Internal function adding the values to the various tag arrays. * - * @param array $tags + * @param array> $tags * Associative array of tag => value */ private function addBagInfoTagsInternal(array $tags): void @@ -814,7 +814,7 @@ public function getFileEncoding(): string /** * Get the currently active payload (and tag) manifests. * - * @return array + * @return array * Internal hash names for current manifests. */ public function getAlgorithms(): array @@ -828,7 +828,7 @@ public function getAlgorithms(): array * @param string $hashAlgorithm * The requested hash algorithms. * - * @return boolean Do we already have this payload manifest. + * @return bool Do we already have this payload manifest. */ public function hasAlgorithm(string $hashAlgorithm): bool { @@ -841,7 +841,7 @@ public function hasAlgorithm(string $hashAlgorithm): bool * * @param string $algorithm * The requested hash algorithm - * @return boolean + * @return bool * Whether it is supported by our PHP. */ public function algorithmIsSupported(string $algorithm): bool @@ -930,7 +930,7 @@ public function setAlgorithm(string $algorithm): void /** * Replaces any existing hash algorithms with the ones requested. * - * @param array $algorithms + * @param array $algorithms * Array of algorithms to use. * @throws FilesystemException * Problems removing/reading/creating the new payload/tag manifest files. @@ -950,7 +950,7 @@ public function setAlgorithms(array $algorithms): void /** * Internal utility to remove all algorithms not specified and add any missing. * - * @param array $algorithms + * @param array $algorithms * Array of algorithms using their internal names. * @throws FilesystemException * Errors @@ -987,7 +987,7 @@ private function setAlgorithmsInternal(array $algorithms): void * @throws BagItException * On errors adding the file. */ - public function addFetchFile(string $url, string $destination, int $size = null): void + public function addFetchFile(string $url, string $destination, ?int $size = null): void { if (!$this->hasFetchFile()) { $this->fetchFile = new Fetch($this, false); @@ -998,9 +998,9 @@ public function addFetchFile(string $url, string $destination, int $size = null) } /** - * Return the fetch file data, an array of arrays with keys 'url', 'destination' and (optionally) 'size'. + * Return the fetch file data, an array of DownloadFile objects. * - * @return array + * @return array */ public function listFetchFiles(): array { @@ -1049,7 +1049,7 @@ public function hasFetchFile(): bool /** * Get the current version array or default if not specified. * - * @return array + * @return array * Current version. */ public function getVersion(): array @@ -1094,7 +1094,7 @@ public function getDataDirectory(): string /** * Check the bag's extended status. * - * @return boolean + * @return bool * Does the bag use extended features? */ public function isExtended(): bool @@ -1105,7 +1105,7 @@ public function isExtended(): bool /** * Turn extended bag features on or off. * - * @param boolean $extBag + * @param bool $extBag * Whether the bag should be extended or not. */ public function setExtended(bool $extBag): void @@ -1119,7 +1119,7 @@ public function setExtended(bool $extBag): void /** * Get errors on the bag. * - * @return array + * @return array> * The errors. */ public function getErrors(): array @@ -1130,7 +1130,7 @@ public function getErrors(): array /** * Get any warnings related to the bag. * - * @return array + * @return array> * The warnings. */ public function getWarnings(): array @@ -1141,7 +1141,7 @@ public function getWarnings(): array /** * Get the payload manifests as an associative array with hash algorithm as key. * - * @return array + * @return array * hash algorithm => Payload manifests */ public function getPayloadManifests(): array @@ -1152,7 +1152,7 @@ public function getPayloadManifests(): array /** * Get the tag manifests as an associative array with hash algorithm as key. * - * @return array|null + * @return array|null * hash algorithm => Tag manifests or null if not an extended bag. */ public function getTagManifests(): ?array @@ -1303,7 +1303,7 @@ public function getSerializationMimeType(): ?string } /** - * @return array The profiles that have been added to the bag. + * @return array The profiles that have been added to the bag. */ public function getBagProfiles(): array { @@ -1707,7 +1707,7 @@ private function removeBagInfo(): void /** * Calculate the total file size and amount of files of all payload files. * - * @return array|null + * @return array|null * The total file size and amount of all files or * null if we couldn't read all the file sizes. */ @@ -1737,7 +1737,7 @@ private function calculateTotalFileSizeAndAmountOfFiles(): ?array * * @param string $text * The whole tag and value as one. - * @return array + * @return array * The text as an array. */ private static function wrapBagInfoText(string $text): array @@ -1773,7 +1773,7 @@ function ($o) { * The text to wrap. * @param int $length * The length to wrap at. - * @return array + * @return array * Rows of text. */ private static function wrapAtLength(string $text, int $length): array @@ -1786,7 +1786,7 @@ private static function wrapAtLength(string $text, int $length): array /** * Load all tag manifests (if any). * - * @return boolean + * @return bool * Are there any tag manifest files. * * @throws FilesystemException @@ -1865,7 +1865,7 @@ private function clearTagManifests(): void /** * Remove tag manifests. * - * @param array $exclusions + * @param array $exclusions * Hash algorithm names of manifests to preserve. * @throws FilesystemException * Issues deleting files from the filesystem. @@ -1978,7 +1978,7 @@ private function clearPayloadManifests(): void /** * Remove payload manifests. * - * @param array $exclusions + * @param array $exclusions * Hash algorithm names of manifests to preserve. * @throws FilesystemException * Issues deleting a file. @@ -2271,7 +2271,12 @@ private static function untarBag(string $filename): string $tar = new Archive_Tar($filename, $compression); $res = $tar->extract($directory); if ($res === false) { - throw new FilesystemException("Unable to untar $filename : " . $tar->error_object->getMessage()); + $error = $tar->error_object; + $mesg = "Unable to untar $filename"; + if (is_object($error) && get_class($error) === 'PEAR_Error') { + $mesg .= " : " . $error->getMessage(); + } + throw new FilesystemException($mesg); } return $directory; } @@ -2316,7 +2321,7 @@ private static function extractDir(): string * * @param string|null $file_extension * The file extensions. - * @param array $extensions + * @param array $extensions * The list of extensions to check. * @return bool * The list of extensions or an empty array. @@ -2372,7 +2377,11 @@ private static function getExtension(string $filepath): ?string */ private static function getDirectory(string $filepath): string { - $files = array_diff(scandir($filepath), [".", ".."]); + $dir_files = scandir($filepath); + if ($dir_files === false) { + throw new BagItException("Unable to read directory $filepath"); + } + $files = array_diff($dir_files, [".", ".."]); $dirs = []; if (count($files) > 0) { foreach ($files as $file) { @@ -2513,8 +2522,8 @@ private function hashIsSupported(string $internal_name): bool * * @param string $search The key to look for. * @param int|string $key The associative or numeric key to look in. - * @param array $map The associative array to search. - * @return boolean True if the key exists regardless of case. + * @param array> $map The associative array to search. + * @return bool True if the key exists regardless of case. */ private static function arrayKeyExistsNoCase(string $search, int|string $key, array $map): bool { @@ -2532,9 +2541,9 @@ function (&$item) { * Check that the key is not non-repeatable and already in the bagInfo. * * @param string $key The key being added. - * @param array $bagData The current bag data. + * @param array> $bagData The current bag data. * - * @return boolean + * @return bool * True if the key is non-repeatable and already in the */ private static function mustNotRepeatBagInfoExists(string $key, array $bagData): bool @@ -2547,9 +2556,9 @@ private static function mustNotRepeatBagInfoExists(string $key, array $bagData): * Check that the key is not non-repeatable and already in the bagInfo. * * @param string $key The key being added. - * @param array $bagData The current bag data. + * @param array> $bagData The current bag data. * - * @return boolean + * @return bool * True if the key is non-repeatable and already in the */ private static function shouldNotRepeatBagInfoExists(string $key, array $bagData): bool @@ -2602,7 +2611,7 @@ private function compareVersion(string $version): int /** * Utility to merge manifest and fetch errors into the bag errors. * - * @param array $newErrors + * @param array> $newErrors * The new errors to be added. */ private function mergeErrors(array $newErrors): void @@ -2613,7 +2622,7 @@ private function mergeErrors(array $newErrors): void /** * Utility to merge manifest and fetch warnings into the bag warnings. * - * @param array $newWarnings + * @param array> $newWarnings * The new warnings to be added. */ private function mergeWarnings(array $newWarnings): void diff --git a/src/BagUtils.php b/src/BagUtils.php index 1eb09da..8291c89 100644 --- a/src/BagUtils.php +++ b/src/BagUtils.php @@ -79,7 +79,7 @@ public static function baseInData(string $path): string * @param string $pattern * The pattern to search for. * - * @return array + * @return array * Array of matches. * * @throws FilesystemException @@ -116,6 +116,7 @@ public static function getValidCharset(string $charset): ?string * @param bool $add_absolute * Whether to prepend the current working directory if the path is relative. * @return string + * @throws FilesystemException If unable to get the current working directory. */ public static function getAbsolute(string $path, bool $add_absolute = false): string { @@ -134,6 +135,9 @@ public static function getAbsolute(string $path, bool $add_absolute = false): st if (!($startWithLetterDir || $startWithSeparator) && $add_absolute) { // This was relative to start with, prepend the current working directory. $current_dir = getcwd(); + if ($current_dir === false) { + throw new FilesystemException("Unable to get current working directory"); + } return BagUtils::getAbsolute(rtrim($current_dir, '/') . '/' . ltrim($path, '/')); } @@ -183,10 +187,12 @@ function ($i) { * * @param string $directory * The starting full path. - * @param array $exclusions + * @param array $exclusions * Array with directory names to skip. - * @return array + * @return array * List of files with absolute path. + * @throws FilesystemException + * If unable to scan a directory. */ public static function getAllFiles(string $directory, array $exclusions = []): array { @@ -194,7 +200,11 @@ public static function getAllFiles(string $directory, array $exclusions = []): a $found_files = []; while ($currentPath = array_shift($paths)) { - $files = array_diff(scandir($currentPath), [".", ".."]); + $dir_files = scandir($currentPath); + if ($dir_files === false) { + throw new FilesystemException("Unable to scan directory $currentPath"); + } + $files = array_diff($dir_files, [".", ".."]); foreach ($files as $file) { $fullPath = $currentPath . '/' . $file; if (is_dir($fullPath) && !in_array($file, $exclusions)) { @@ -408,12 +418,16 @@ public static function checkUnencodedFilepath(string $filepath): bool * * @param string $data * The file data as a single string. - * @return array + * @return array * Array split on \r\n, \r, and \n */ public static function splitFileDataOnLineEndings(string $data): array { - return preg_split("/(\r\n|\r|\n)/", $data); + $data = preg_split("/(\r\n|\r|\n)/", $data); + if ($data === false) { + return []; + } + return $data; } /** @@ -457,7 +471,11 @@ public static function deleteEmptyDirTree(string $path, string $rootDir): void } if (file_exists($path) && is_dir($path)) { $parent = dirname($path); - $files = array_diff(scandir($path), [".", ".."]); + $dir_files = scandir($path); + if ($dir_files === false) { + throw new FilesystemException("Unable to scan directory $parent"); + } + $files = array_diff($dir_files, [".", ".."]); if (count($files) === 0) { self::checkedRmDir($path); } diff --git a/src/Commands/ValidateCommand.php b/src/Commands/ValidateCommand.php index b772117..bdf68fc 100644 --- a/src/Commands/ValidateCommand.php +++ b/src/Commands/ValidateCommand.php @@ -43,14 +43,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int $path = BagUtils::standardizePathSeparators($input->getArgument('bag-path')); if (($path[0] ?? "") !== '/' && !preg_match("/^[a-z]:/i", $path)) { - $path = getcwd() . '/' . $path; + $root_path = getcwd(); + if ($root_path === false) { + $io->error("Failed to get current working directory."); + return(1); + } + $path = "$root_path/$path"; $realpath = realpath($path); } - if ((isset($realpath) && $realpath === false) || !file_exists($path)) { + if ((isset($realpath) && is_bool($realpath)) || !file_exists($path)) { $io->error("Path $path does not exist, cannot validate."); } else { try { - if (isset($realpath)) { + if (isset($realpath) && is_string($realpath)) { $path = $realpath; } $bag = Bag::load($path); diff --git a/src/CurlInstance.php b/src/CurlInstance.php index ccb60ce..a073d47 100644 --- a/src/CurlInstance.php +++ b/src/CurlInstance.php @@ -4,6 +4,7 @@ use CurlHandle; use CurlMultiHandle; +use whikloj\BagItTools\Exceptions\BagItException; trait CurlInstance { @@ -17,13 +18,18 @@ trait CurlInstance * @param int|null $size * Expected download size or null if unknown * @return CurlHandle - * False on error, otherwise the cUl resource. + * The cUrl resource. + * @throws BagItException + * On cUrl error. */ public static function createCurl(string $url, bool $single = false, ?int $size = null): CurlHandle { $ch = curl_init($url); - $curlVersion = curl_version()['version']; - $options = self::setupCurl($curlVersion); + if ($ch === false) { + throw new BagItException("Failed to initialize cUrl with URL ($url)."); + } + $curlVersion = curl_version(); + $options = is_array($curlVersion) ? self::setupCurl($curlVersion['version']) : []; if ($single === true) { // If this is set during curl_multi_exec, it swallows error messages. $options[CURLOPT_FAILONERROR] = true; @@ -66,19 +72,21 @@ private static function curlXferInfo(int $expectDl, int $currDl): int * Create a cUrl multi handler. * * @return CurlMultiHandle - * False on error, otherwise the cUrl resource + * The cUrl resource */ public static function createMultiCurl(): CurlMultiHandle { - $curlVersion = curl_version()['version']; + $curlVersion = curl_version(); + $version = is_array($curlVersion) ? $curlVersion['version'] : false; $mh = curl_multi_init(); if ( - version_compare('7.62.0', $curlVersion) > 0 && - version_compare('7.43.0', $curlVersion) <= 0 + $version && + version_compare('7.62.0', $version) > 0 && + version_compare('7.43.0', $version) <= 0 ) { curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX); } - if (version_compare('7.30.0', $curlVersion) <= 0) { + if ($version && version_compare('7.30.0', $version) <= 0) { // Set a limit to how many connections can be opened. curl_multi_setopt($mh, CURLMOPT_MAX_TOTAL_CONNECTIONS, 10); } @@ -87,7 +95,7 @@ public static function createMultiCurl(): CurlMultiHandle /** * Set general CURLOPTS based on the Curl version. - * @return array The options to set. + * @return array The options to set. */ private static function setupCurl(string $curlVersion): array { diff --git a/src/DownloadFile.php b/src/DownloadFile.php new file mode 100644 index 0000000..426e68e --- /dev/null +++ b/src/DownloadFile.php @@ -0,0 +1,138 @@ +uri = $uri; + $this->destination = $destination; + $this->size = $size; + } + + /** + * Property getter for array_column usage. + * @param string $prop The property to get. + * @return string|int|null The value of the property. + */ + public function __get(string $prop) + { + return $this->$prop; + } + + /** + * Property isset check for array_column usage. + * @param string $prop The property to check. + * @return bool True if the property is set. + */ + public function __isset(string $prop): bool + { + return isset($this->$prop); + } + + /** + * @return string The destination file path. + */ + public function getDestination(): string + { + return $this->destination; + } + + /** + * @return string The URL to download. + */ + public function getUrl(): string + { + return $this->uri; + } + + /** + * @return int|null The expected download size or null if unknown. + */ + public function getSize(): ?int + { + return $this->size; + } + + /** + * Get the size as a string. + * @return string The size as a string. + */ + public function getSizeString(): string + { + return $this->size === null ? '-' : (string) $this->size; + } + + /** + * Validate the download URL. + * @throws BagItException If the URL is invalid. + */ + public function validateDownload(): void + { + if (!$this->validateUrl($this->getUrl())) { + // skip invalid URLs or non-http URLs + throw new BagItException("URL {$this->getUrl()} does not seem to have a scheme or host"); + } + if (!$this->internalValidateUrl($this->getUrl())) { + throw new BagItException("This library only supports http/https URLs"); + } + } + + /** + * Validate URLs can be processed by this library. + * + * @param string $url + * The URL. + * @return bool + * True if we can process it. + */ + private function validateUrl(string $url): bool + { + $parts = parse_url($url); + if (!isset($parts['scheme']) || !isset($parts['host'])) { + return false; + } + return true; + } + + /** + * BagItTools specific (non-spec) requirements for URLs. + * + * @param string $url + * The URL. + * @return bool + * True if we can process it. + */ + private function internalValidateUrl(string $url): bool + { + $parts = parse_url($url); + if ( + is_array($parts) && + array_key_exists('scheme', $parts) && + $parts['scheme'] !== 'http' && + $parts['scheme'] !== 'https' + ) { + return false; + } + return true; + } +} diff --git a/src/Fetch.php b/src/Fetch.php index 14fa659..08c9c16 100644 --- a/src/Fetch.php +++ b/src/Fetch.php @@ -39,23 +39,23 @@ class Fetch private string $filename; /** - * Information from the fetch.txt, array of arrays with keys 'uri', 'size', and 'destination' + * Information from the fetch.txt, array of DownloadFile objects. * - * @var array + * @var array */ private array $files; /** * Errors * - * @var array + * @var array> */ private array $fetchErrors = []; /** - * Urls and Files that validated and should be downloaded. + * DownloadFile objects that validated and should be downloaded. * - * @var array + * @var array */ private array $downloadQueue = []; @@ -80,9 +80,9 @@ public function __construct(Bag $bag, bool $load = false) } /** - * Return the array of file data. + * Return the array of file data objects. * - * @return array + * @return array */ public function getData(): array { @@ -117,23 +117,16 @@ public function downloadAll(): void /** * Validate fetch data. * - * @param array $fetchData - * Array with mandatory keys 'uri' and 'destination' and optional key 'size'. + * @param DownloadFile $fetchData + * Download data. * * @throws BagItException * For all validation errors. */ - private function validateData(array $fetchData): void + private function validateData(DownloadFile $fetchData): void { - $uri = $fetchData['uri']; - $dest = BagUtils::baseInData($fetchData['destination']); - if (!$this->validateUrl($uri)) { - // skip invalid URLs or non-http URLs - throw new BagItException("URL $uri does not seem to have a scheme or host"); - } - if (!$this->internalValidateUrl($uri)) { - throw new BagItException("This library only supports http/https URLs"); - } + $fetchData->validateDownload(); + $dest = BagUtils::baseInData($fetchData->getDestination()); if (!$this->bag->pathInBagData($dest)) { throw new BagItException("Path $dest resolves outside the bag."); } @@ -153,13 +146,7 @@ private function validateData(array $fetchData): void */ public function addFile(string $url, string $destination, ?int $size = null): void { - $fetchData = [ - 'uri' => $url, - 'destination' => $destination, - ]; - if (is_int($size)) { - $fetchData['size'] = $size; - } + $fetchData = new DownloadFile($url, $destination, $size); // Download the file now to help with manifest handling, deleted when you package() or finalize(). $this->download($fetchData); } @@ -167,20 +154,20 @@ public function addFile(string $url, string $destination, ?int $size = null): vo /** * Download a single file as it is added to the fetch file so we can generate checksums. * - * @param array $fetchData + * @param DownloadFile $fetchData * Array of data with keys 'uri', 'destination' and optionally 'size'. * * @throws BagItException * Problems downloading the file. */ - public function download(array $fetchData): void + public function download(DownloadFile $fetchData): void { $this->validateData($fetchData); - $uri = $fetchData['uri']; + $uri = $fetchData->getUrl(); if ($this->urlExistsInFile($uri)) { throw new BagItException("This URL ($uri) is already in fetch.txt"); } - $dest = BagUtils::baseInData($fetchData['destination']); + $dest = BagUtils::baseInData($fetchData->getDestination()); if ($this->destinationExistsInFile($dest)) { throw new BagItException("This destination ($dest) is already in the fetch.txt"); } @@ -189,7 +176,7 @@ public function download(array $fetchData): void if (file_exists($fullDest)) { throw new BagItException("File already exists at the destination path $dest"); } - $size = $fetchData['size'] ?? null; + $size = $fetchData->getSize(); $ch = $this->createCurl($uri, true, $size); $output = curl_exec($ch); $error = curl_error($ch); @@ -197,12 +184,8 @@ public function download(array $fetchData): void if (!empty($error) || $output === false) { throw new BagItException("Error with download of $uri : $error"); } - $this->saveFileData($output, $dest); - $this->files[] = [ - 'uri' => $fetchData['uri'], - 'size' => (!empty($fetchData['size']) ? $fetchData['size'] : '-'), - 'destination' => $dest, - ]; + $this->saveFileData((string) $output, $dest); + $this->files[] = $fetchData; } /** @@ -218,10 +201,10 @@ public function removeFile(string $url): void if ($this->urlExistsInFile($url)) { $newFiles = []; foreach ($this->files as $file) { - if (strtolower($url) !== strtolower($file['uri'])) { + if (strtolower($url) !== strtolower($file->getUrl())) { $newFiles[] = $file; } else { - $fullFile = $this->bag->makeAbsolute($file['destination']); + $fullFile = $this->bag->makeAbsolute($file->getDestination()); if (file_exists($fullFile)) { BagUtils::checkedUnlink($fullFile); } @@ -252,7 +235,7 @@ public function update(): void public function cleanup(): void { foreach ($this->files as $file) { - $fullPath = $this->bag->makeAbsolute($file['destination']); + $fullPath = $this->bag->makeAbsolute($file->getDestination()); if (file_exists($fullPath)) { // Remove the file because we are being packaged or finalized. BagUtils::checkedUnlink($fullPath); @@ -279,7 +262,7 @@ public function clearData(): void /** * Return the errors. * - * @return array + * @return array> * Array of errors. */ public function getErrors(): array @@ -331,10 +314,8 @@ private function loadFiles(): void 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]; - if ($filesize != "-") { - $filesize = (int)$filesize; - } + $filesize = trim($matches[2]); + $filesize = ($filesize === "-") ? null : (int)$filesize; $destination = BagUtils::baseInData($matches[3]); if (BagUtils::checkUnencodedFilepath($destination)) { $this->addError( @@ -343,11 +324,11 @@ private function loadFiles(): void ); } $destination = BagUtils::decodeFilepath($destination); - $this->files[] = [ - 'uri' => $uri, - 'size' => $filesize, - 'destination' => $destination, - ]; + $this->files[] = new DownloadFile( + $uri, + $destination, + $filesize + ); } else { $this->addError("Line $lineCount: This line is not valid."); } @@ -394,12 +375,11 @@ private function downloadFiles(): void $curl_handles = []; $destinations = []; foreach ($this->downloadQueue as $key => $download) { - $fullPath = $this->bag->makeAbsolute($download['destination']); + $fullPath = $this->bag->makeAbsolute($download->getDestination()); // 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_handles[$key] = $this->createCurl($download->getUrl(), false, $download->getSize()); curl_multi_add_handle($mh, $curl_handles[$key]); } } @@ -447,8 +427,8 @@ private function writeToDisk(): void throw new FilesystemException("Unable to write $this->filename"); } foreach ($this->files as $fileData) { - $destination = BagUtils::encodeFilepath($fileData['destination']); - $line = "{$fileData['uri']} {$fileData['size']} $destination" . PHP_EOL; + $destination = BagUtils::encodeFilepath($fileData->getDestination()); + $line = "{$fileData->getUrl()} {$fileData->getSizeString()} $destination" . PHP_EOL; $line = $this->bag->encodeText($line); BagUtils::checkedFwrite($fp, $line); } @@ -456,40 +436,6 @@ private function writeToDisk(): void } } - /** - * Validate URLs can be processed by this library. - * - * @param string $url - * The URL. - * @return bool - * True if we can process it. - */ - private function validateUrl(string $url): bool - { - $parts = parse_url($url); - if (!isset($parts['scheme']) || !isset($parts['host'])) { - return false; - } - return true; - } - - /** - * BagItTools specific (non-spec) requirements for URLs. - * - * @param string $url - * The URL. - * @return bool - * True if we can process it. - */ - private function internalValidateUrl(string $url): bool - { - $parts = parse_url($url); - if ($parts['scheme'] !== 'http' && $parts['scheme'] !== 'https') { - return false; - } - return true; - } - /** * Check if the url is already in the file. * @@ -517,7 +463,7 @@ private function destinationExistsInFile(string $dest): bool } /** - * Check multi-dimensional array for a specific value in a specific field. + * Check array of DownloadFiles for a specific value in a specific field. * * @param string $arg * The value to look for. diff --git a/src/Profiles/BagItProfile.php b/src/Profiles/BagItProfile.php index 1403df7..9ba8c33 100644 --- a/src/Profiles/BagItProfile.php +++ b/src/Profiles/BagItProfile.php @@ -5,9 +5,13 @@ namespace whikloj\BagItTools\Profiles; use Exception; +use PharIo\Manifest\Manifest; +use whikloj\BagItTools\AbstractManifest; use whikloj\BagItTools\Bag; use whikloj\BagItTools\BagUtils; +use whikloj\BagItTools\Exceptions\FilesystemException; use whikloj\BagItTools\Exceptions\ProfileException; +use whikloj\BagItTools\PayloadManifest; /** * Class for holding a BagItProfile. @@ -72,25 +76,25 @@ class BagItProfile protected ?string $contactEmail = null; /** - * @var array + * @var array * The list of profile specific tags for this profile. Each tag is a key to an array with keys of "required", * "values", "repeatable" and "description". Does not include the required "BagIt-Profile-Identifier" tag. */ protected array $profileBagInfoTags = []; /** - * @var array A list of "required" BagInfo tags. + * @var array A list of "required" BagInfo tags. */ protected array $requiredBagInfoTags = []; /** - * @var array + * @var array * The list of required manifest algorithms. e.g. ["sha1", "md5"]. */ protected array $manifestsRequired = []; /** - * @var array + * @var array * The list of allowed manifest algorithms. e.g. ["sha1", "md5"]. If manifestsRequired is not empty then this list * must include all the required algorithms. */ @@ -122,40 +126,40 @@ class BagItProfile protected string $serialization = "optional"; /** - * @var array + * @var array * A list of MIME types acceptable as serialized formats. If serialization is required then this list must contain * one or more values. If serialization is forbidden, this is ignored. */ protected array $acceptSerialization = []; /** - * @var array + * @var array * A list of BagIt version numbers that will be accepted, e.g. "1.0". At least one version is required. */ protected array $acceptBagItVersion = []; /** - * @var array + * @var array * Each tag manifest type in LIST is required. e.g. ["sha1", "md5"]. */ protected array $tagManifestsRequired = []; /** - * @var array + * @var array * The list of allowed tag manifest algorithms. e.g. ["sha1", "md5"]. If tagManifestsRequired is not empty then * this list must include all the required algorithms. */ protected array $tagManifestsAllowed = []; /** - * @var array + * @var array * A list of a tag files that MUST be included in a conformant Bag. Entries are full path names relative to the * Bag base directory. */ protected array $tagFilesRequired = []; /** - * @var array + * @var array * A list of tag files that MAY be included in a conformant Bag. Entries are either full path names relative to the * bag base directory or path name patterns in which asterisks can represent zero or more characters (c.f. glob(7)). * At least all the tag files listed in Tag-Files-Required MUST be in included in Tag-Files-Allowed. @@ -163,14 +167,14 @@ class BagItProfile protected array $tagFilesAllowed = []; /** - * @var array + * @var array * A list of a payload files and/or directories that MUST be included in a conformant Bag. Entries are full path * names relative to the Bag base directory, e.g. data/LICENSE.txt or data/src/. */ protected array $payloadFilesRequired = []; /** - * @var array + * @var array * A list of payload files or directories that MAY be included in a conformant Bag. Each entry MUST be either a * full path name relative to the bag base directory, or a path name pattern in which asterisks can represent zero * or more characters (c.f. glob(7)). Paths requiring permitted directories MUST end with /* (not /). @@ -182,7 +186,7 @@ class BagItProfile protected array $payloadFilesAllowed = []; /** - * @var array + * @var array * A list of warnings that were generated during the validation of the profile. */ protected array $profileWarnings = []; @@ -332,7 +336,7 @@ private function setContactEmail(?string $contactEmail): BagItProfile } /** - * @return array The list of acceptable tags for this profile. + * @return array The list of acceptable tags for this profile. */ public function getBagInfoTags(): array { @@ -340,7 +344,7 @@ public function getBagInfoTags(): array } /** - * @param array $bagInfoTags Parsed profile Bag-Info sections + * @param array> $bagInfoTags Parsed profile Bag-Info sections * @return BagItProfile The profile object. */ private function setBagInfoTags(array $bagInfoTags): BagItProfile @@ -362,7 +366,7 @@ private function setBagInfoTags(array $bagInfoTags): BagItProfile } /** - * @return array The list of required manifest algorithms. e.g. ["sha1", "md5"]. + * @return array The list of required manifest algorithms. e.g. ["sha1", "md5"]. */ public function getManifestsRequired(): array { @@ -370,7 +374,7 @@ public function getManifestsRequired(): array } /** - * @param array $manifestsRequired The list of required manifest algorithms. e.g. ["sha1", "md5"]. + * @param array $manifestsRequired The list of required manifest algorithms. e.g. ["sha1", "md5"]. * @return BagItProfile The profile object. */ private function setManifestsRequired(array $manifestsRequired): BagItProfile @@ -380,7 +384,7 @@ private function setManifestsRequired(array $manifestsRequired): BagItProfile } /** - * @return array The list of allowed manifest algorithms. e.g. ["sha1", "md5"]. + * @return array The list of allowed manifest algorithms. e.g. ["sha1", "md5"]. */ public function getManifestsAllowed(): array { @@ -388,7 +392,7 @@ public function getManifestsAllowed(): array } /** - * @param array $manifestsAllowed The list of allowed manifest algorithms. e.g. ["sha1", "md5"]. + * @param array $manifestsAllowed The list of allowed manifest algorithms. e.g. ["sha1", "md5"]. * @return BagItProfile The profile object. * @throws ProfileException If manifestsAllowed does not include all entries from manifestsRequired. */ @@ -478,7 +482,7 @@ private function setSerialization(string $serialization): BagItProfile } /** - * @return array The list of MIME types acceptable as serialized formats. + * @return array The list of MIME types acceptable as serialized formats. */ public function getAcceptSerialization(): array { @@ -486,7 +490,7 @@ public function getAcceptSerialization(): array } /** - * @param array|null $acceptSerialization The list of MIME types acceptable as serialized formats. + * @param array|null $acceptSerialization The list of MIME types acceptable as serialized formats. * @return BagItProfile The profile object. */ private function setAcceptSerialization(?array $acceptSerialization): BagItProfile @@ -496,7 +500,7 @@ private function setAcceptSerialization(?array $acceptSerialization): BagItProfi } /** - * @return array The list of BagIt version numbers that will be accepted, e.g. "1.0". + * @return array The list of BagIt version numbers that will be accepted, e.g. "1.0". */ public function getAcceptBagItVersion(): array { @@ -504,7 +508,7 @@ public function getAcceptBagItVersion(): array } /** - * @param array $acceptBagItVersion The list of BagIt version numbers that will be accepted, e.g. "1.0". + * @param array $acceptBagItVersion The list of BagIt version numbers that will be accepted, e.g. "1.0". * @return BagItProfile The profile object. */ private function setAcceptBagItVersion(array $acceptBagItVersion): BagItProfile @@ -514,7 +518,7 @@ private function setAcceptBagItVersion(array $acceptBagItVersion): BagItProfile } /** - * @return array The list of required tag manifest algorithms. e.g. ["sha1", "md5"]. + * @return array The list of required tag manifest algorithms. e.g. ["sha1", "md5"]. */ public function getTagManifestsRequired(): array { @@ -522,7 +526,7 @@ public function getTagManifestsRequired(): array } /** - * @param array $tagManifestsRequired The list of required tag manifest algorithms. e.g. ["sha1", "md5"]. + * @param array $tagManifestsRequired The list of required tag manifest algorithms. e.g. ["sha1", "md5"]. * @return BagItProfile The profile object. */ private function setTagManifestsRequired(array $tagManifestsRequired): BagItProfile @@ -532,7 +536,7 @@ private function setTagManifestsRequired(array $tagManifestsRequired): BagItProf } /** - * @return array The list of allowed tag manifest algorithms. e.g. ["sha1", "md5"]. + * @return array The list of allowed tag manifest algorithms. e.g. ["sha1", "md5"]. */ public function getTagManifestsAllowed(): array { @@ -540,7 +544,7 @@ public function getTagManifestsAllowed(): array } /** - * @param array $tagManifestAllowed The list of allowed tag manifest algorithms. e.g. ["sha1", "md5"]. + * @param array $tagManifestAllowed The list of allowed tag manifest algorithms. e.g. ["sha1", "md5"]. * @return BagItProfile The profile object. */ private function setTagManifestsAllowed(array $tagManifestAllowed): BagItProfile @@ -550,7 +554,7 @@ private function setTagManifestsAllowed(array $tagManifestAllowed): BagItProfile } /** - * @return array The list of tag files that MUST be included in a conformant Bag. + * @return array The list of tag files that MUST be included in a conformant Bag. */ public function getTagFilesRequired(): array { @@ -558,7 +562,7 @@ public function getTagFilesRequired(): array } /** - * @param array $tagFilesRequired The list of tag files that MUST be included in a conformant Bag. + * @param array $tagFilesRequired The list of tag files that MUST be included in a conformant Bag. * @return BagItProfile The profile object. */ private function setTagFilesRequired(array $tagFilesRequired): BagItProfile @@ -568,7 +572,7 @@ private function setTagFilesRequired(array $tagFilesRequired): BagItProfile } /** - * @return array The list of tag files that MAY be included in a conformant Bag. + * @return array The list of tag files that MAY be included in a conformant Bag. */ public function getTagFilesAllowed(): array { @@ -576,7 +580,7 @@ public function getTagFilesAllowed(): array } /** - * @param array $tagFilesAllowed The list of tag files/paths that MAY be included in a conformant Bag. + * @param array $tagFilesAllowed The list of tag files/paths that MAY be included in a conformant Bag. * @return BagItProfile The profile object. */ private function setTagFilesAllowed(array $tagFilesAllowed): BagItProfile @@ -587,8 +591,8 @@ private function setTagFilesAllowed(array $tagFilesAllowed): BagItProfile /** * Assert that the array of paths are covered by the array of allowed paths and glob style patterns. - * @param array $paths The list of paths. - * @param array $allowed The list of allowed paths, and glob style patterns. + * @param array $paths The list of paths. + * @param array $allowed The list of allowed paths, and glob style patterns. * @return bool True if all paths are covered by allowed paths/patterns. */ private function isRequiredPathsCoveredByAllowed(array $paths, array $allowed): bool @@ -605,9 +609,9 @@ private function isRequiredPathsCoveredByAllowed(array $paths, array $allowed): /** * Get the list of paths that are not covered by the allowed paths and glob style patterns. - * @param array $paths The list of paths. - * @param array $allowed The list of allowed paths and glob style patterns. - * @return array The list of paths not covered by allowed paths/patterns. + * @param array $paths The list of paths. + * @param array $allowed The list of allowed paths and glob style patterns. + * @return array The list of paths not covered by allowed paths/patterns. */ private function getPathsNotCoveredByAllowed(array $paths, array $allowed): array { @@ -643,7 +647,7 @@ private function convertGlobToRegex(string $glob): string } /** - * @return array The list of payload files that MUST be included in a conformant Bag. + * @return array The list of payload files that MUST be included in a conformant Bag. */ public function getPayloadFilesRequired(): array { @@ -651,7 +655,7 @@ public function getPayloadFilesRequired(): array } /** - * @param array $payloadFilesRequired The list of payload files that MUST be included in a conformant Bag. + * @param array $payloadFilesRequired The list of payload files that MUST be included in a conformant Bag. * @return BagItProfile The profile object. */ private function setPayloadFilesRequired(array $payloadFilesRequired): BagItProfile @@ -661,7 +665,7 @@ private function setPayloadFilesRequired(array $payloadFilesRequired): BagItProf } /** - * @return array The list of payload files that MAY be included in a conformant Bag. + * @return array The list of payload files that MAY be included in a conformant Bag. */ public function getPayloadFilesAllowed(): array { @@ -669,7 +673,8 @@ public function getPayloadFilesAllowed(): array } /** - * @param array $payloadFilesAllowed The list of payload files/paths that MAY be included in a conformant Bag. + * @param array $payloadFilesAllowed The list of payload files/paths that MAY be included in a + * conformant Bag. * @return BagItProfile The profile object. */ private function setPayloadFilesAllowed(array $payloadFilesAllowed): BagItProfile @@ -844,6 +849,7 @@ public function isValid(): bool * @param Bag $bag The bag to validate. * @return bool True if the bag is valid. * @throws ProfileException If the bag is not valid. + * @throws FilesystemException If cannot complete the validation. */ public function validateBag(Bag $bag): bool { @@ -864,9 +870,9 @@ public function validateBag(Bag $bag): bool $errors[] = "Profile does not allow tag ($requiredTag) to repeat, there are " . count($bag->getBagInfoByTag($requiredTag)) . " values in the bag"; } - if ($infoTag->getValues() !== [] && $bag->hasBagInfoTag($requiredTag)) { + if (!empty($infoTag->getValues()) && $bag->hasBagInfoTag($requiredTag)) { $diff = array_diff($bag->getBagInfoByTag($requiredTag), $infoTag->getValues()); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile requires tag ($requiredTag) to have value(s) (" . implode(", ", $infoTag->getValues()) . ") but the bag has value(s) (" . implode(", ", $diff) . ")"; @@ -881,6 +887,9 @@ public function validateBag(Bag $bag): bool } if ($this->isDataEmpty()) { $manifests = current($bag->getPayloadManifests()); + if ($manifests === false) { + throw new FilesystemException("Unable to get payload manifests"); + } $hashes = $manifests->getHashes(); if (count($hashes) > 1) { $errors[] = "Profile requires /data directory to be empty or contain a single 0 byte file but it" . @@ -888,9 +897,13 @@ public function validateBag(Bag $bag): bool } elseif (count($hashes) == 1) { $file = array_key_first($hashes); $absolute = $bag->makeAbsolute($file); - if (stat($absolute)['size'] > 0) { + $file_stats = stat($absolute); + if ($file_stats === false) { + throw new FilesystemException("Unable to stat file $absolute"); + } + if ($file_stats['size'] > 0) { $errors[] = "Profile requires /data directory to be empty or contain a single 0 byte file but it" . - " contains a single file of size " . stat($absolute)['size']; + " contains a single file of size " . $file_stats['size']; } } } @@ -924,76 +937,64 @@ public function validateBag(Bag $bag): bool if ($this->getManifestsRequired() !== []) { $manifests = array_keys($bag->getPayloadManifests()); $diff = array_diff($this->getManifestsRequired(), $manifests); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile requires payload manifest(s) which are missing from the bag (" . implode(", ", $diff) . ")"; } } - if ($this->getManifestsAllowed() !== []) { + if (!empty($this->getManifestsAllowed())) { $manifests = array_keys($bag->getPayloadManifests()); $diff = array_diff($manifests, $this->getManifestsAllowed()); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile allows payload manifest(s) (" . implode(", ", $this->getManifestsAllowed()) . "), but the bag has manifest(s) (" . implode(", ", $diff) . ") which are not allowed"; } } - if ($this->getTagManifestsRequired() !== []) { + if (!empty($this->getTagManifestsRequired())) { $manifests = array_keys($bag->getTagManifests()); $diff = array_diff($this->getTagManifestsRequired(), $manifests); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile requires tag manifest(s) which are missing from the bag (" . implode(", ", $diff) . ")"; } } - if ($this->getTagManifestsAllowed() !== []) { + if (!empty($this->getTagManifestsAllowed())) { $manifests = array_keys($bag->getTagManifests()); $diff = array_diff($manifests, $this->getTagManifestsAllowed()); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile allows tag manifest(s) (" . implode(", ", $this->getTagManifestsAllowed()) . "), but the bag has manifest(s) (" . implode(", ", $diff) . ") which are not allowed"; } } - if ($this->getTagFilesRequired() !== []) { - // Grab the first tag manifest, they should all be the same - $manifests = $bag->getTagManifests(); - if (count($manifests) === 0) { - $errors[] = "Profile requires tag files but the bag has no tag manifests"; - } else { - $manifest = reset($manifests); - $tag_files = array_keys($manifest->getHashes()); - $diff = array_diff($this->getTagFilesRequired(), $tag_files); - if ($diff !== []) { - $errors[] = "Profile requires tag files(s) which are missing from the bag (" . - implode(", ", $diff) . ")"; - } + if (!empty($this->getTagFilesRequired())) { + $tag_files = self::getManifestFiles($bag->getTagManifests(), "tag"); + $diff = array_diff($this->getTagFilesRequired(), $tag_files); + if ($diff != []) { + $errors[] = "Profile requires tag files(s) which are missing from the bag (" . + implode(", ", $diff) . ")"; } } - if ($this->getTagFilesAllowed() !== []) { - // Grab the first tag manifest, they should all be the same - $manifests = $bag->getTagManifests()[0]; - $tag_files = array_keys($manifests->getHashes()); + if (!empty($this->getTagFilesAllowed())) { + $tag_files = self::getManifestFiles($bag->getTagManifests(), "tag"); $diff = $this->getPathsNotCoveredByAllowed($tag_files, $this->getTagFilesAllowed()); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile allows tag files(s) (" . implode(", ", $this->getTagFilesAllowed()) . "), but the bag has manifest(s) (" . implode(", ", $diff) . ") which are not allowed"; } } - if ($this->getPayloadFilesRequired() !== []) { - // Grab the first tag manifest, they should all be the same - $manifests = $bag->getPayloadManifests()[0]; - $payload_files = array_keys($manifests->getHashes()); + if (!empty($this->getPayloadFilesRequired())) { + $payload_files = self::getManifestFiles($bag->getPayloadManifests(), "payload"); $diff = array_diff($this->getPayloadFilesRequired(), $payload_files); - if ($diff !== []) { + if (!empty($diff)) { $errors[] = "Profile requires payload file(s) which are missing from the bag (" . implode(", ", $diff) . ")"; } } - if ($this->getPayloadFilesAllowed() !== []) { + if (!empty($this->getPayloadFilesAllowed())) { // Grab the first tag manifest, they should all be the same - $manifests = $bag->getPayloadManifests()[0]; - $tag_files = array_keys($manifests->getHashes()); - $diff = $this->getPathsNotCoveredByAllowed($tag_files, $this->getPayloadFilesAllowed()); - if ($diff !== []) { + $payload_files = self::getManifestFiles($bag->getPayloadManifests(), "payload"); + $diff = $this->getPathsNotCoveredByAllowed($payload_files, $this->getPayloadFilesAllowed()); + if (!empty($diff)) { $errors[] = "Profile allows payload files(s) (" . implode(", ", $this->getPayloadFilesAllowed()) . "), but the bag has file(s) (" . implode(", ", $diff) . ") which are not allowed"; } @@ -1004,9 +1005,31 @@ public function validateBag(Bag $bag): bool return true; } + /** + * Function to pull the first manifest and return the list of files. + * @param array $manifests The list of manifests. + * @param string $manifest_type The type of manifest. + * @return array The list of file paths. + * @throws ProfileException If the bag has no manifests. + */ + private static function getManifestFiles(array $manifests, string $manifest_type): array + { + // Grab the first tag manifest, they should all be the same + $manifests = reset($manifests); + if ($manifests === false) { + throw new ProfileException( + "Profile requires $manifest_type files but the bag has no $manifest_type manifests" + ); + } + $files = array_keys($manifests->getHashes()); + return array_filter($files, function ($file) { + return is_string($file); + }); + } + /** * Get the list of warnings generated during the validation of the profile. - * @return array The list of warnings. + * @return array The list of warnings. */ public function getWarnings(): array { diff --git a/src/Profiles/ProfileFactory.php b/src/Profiles/ProfileFactory.php index 67dc0ec..cee9b4a 100644 --- a/src/Profiles/ProfileFactory.php +++ b/src/Profiles/ProfileFactory.php @@ -41,7 +41,8 @@ public static function generateProfileFromUri(string $url): BagItProfile $curl = self::createCurl($url, true); curl_setopt($curl, CURLOPT_HTTPHEADER, ['Accept: application/json']); $response = curl_exec($curl); - if ($response === false) { + if (is_bool($response)) { + // We set CURLOPT_RETURNTRANSFER to true so boolean is a request error. throw new ProfileException("Error downloading profile"); } $http_code = curl_getinfo($curl, CURLINFO_HTTP_CODE); diff --git a/src/Profiles/ProfileTags.php b/src/Profiles/ProfileTags.php index 1c66c68..342e90d 100644 --- a/src/Profiles/ProfileTags.php +++ b/src/Profiles/ProfileTags.php @@ -29,7 +29,7 @@ class ProfileTags private bool $required = false; /** - * @var array + * @var array */ private array $values = []; @@ -52,7 +52,7 @@ class ProfileTags * ProfileTags constructor. * @param string $tag * @param bool $required - * @param array $values + * @param array $values * @param bool $repeatable * @param string $description */ @@ -82,7 +82,7 @@ public function isRequired(): bool } /** - * @return array + * @return array */ public function getValues(): array { @@ -107,7 +107,7 @@ public function getDescription(): string /** * Return any tags defined in the BagItProfile but not in the specification. - * @return array Array of tagName => tagValue + * @return array Array of tagName => tagValue */ public function getOtherTagOptions(): array { @@ -116,7 +116,7 @@ public function getOtherTagOptions(): array /** * Set the other tag options. - * @param array $tagOptions Array of optionName => optionValue + * @param array $tagOptions Array of optionName => optionValue */ protected function setOtherTagOptions(array $tagOptions): void { @@ -126,7 +126,7 @@ protected function setOtherTagOptions(array $tagOptions): void /** * Create a ProfileTags object from a JSON array. * @param string $tag Tag name - * @param array $tagOpts Tag options + * @param array|mixed> $tagOpts Tag options * @return ProfileTags The created object. */ public static function fromJson(string $tag, array $tagOpts): ProfileTags diff --git a/tests/BagItTestFramework.php b/tests/BagItTestFramework.php index 46108c3..006fc79 100644 --- a/tests/BagItTestFramework.php +++ b/tests/BagItTestFramework.php @@ -4,6 +4,7 @@ namespace whikloj\BagItTools\Test; +use Exception; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionException; @@ -117,11 +118,17 @@ protected function getTempName(): string * * @param string $path * The directory to delete. + * @throws Exception + * Unable to scan directory contents. */ protected static function deleteDirAndContents(string $path): void { if (is_dir($path)) { - $files = array_diff(scandir($path), [".", ".."]); + $dir_files = scandir($path); + if ($dir_files === false) { + throw new Exception("Unable to read directory contents"); + } + $files = array_diff($dir_files, [".", ".."]); foreach ($files as $file) { $currentFile = $path . '/' . $file; if (is_dir($currentFile)) { @@ -176,8 +183,8 @@ protected function copyTestBag(string $testDir): string /** * Compare two arrays have all the same elements, does not compare order. * - * @param array $expected The expected array. - * @param array $testing The array to test. + * @param array> $expected The expected array. + * @param array> $testing The array to test. */ protected function assertArrayEquals(array $expected, array $testing): void { @@ -194,10 +201,15 @@ protected function assertArrayEquals(array $expected, array $testing): void * * @param string $src The original directory. * @param string $dest The destination directory. + * @throws Exception Unable to scan directory contents. */ private static function copyDir(string $src, string $dest): void { - $files = array_diff(scandir($src), [".", ".."]); + $dir_files = scandir($src); + if ($dir_files === false) { + throw new Exception("Unable to read directory contents"); + } + $files = array_diff($dir_files, [".", ".."]); foreach ($files as $item) { if (is_dir("$src/$item")) { if (!is_dir("$dest/$item")) { @@ -213,7 +225,7 @@ private static function copyDir(string $src, string $dest): void /** * Get a private or protected method to test it directly. * - * @param string $class + * @param class-string $class * Class to refect. * @param string $method * Method to get. @@ -295,6 +307,9 @@ protected function assertBagItVersionEncoding( protected function assertStringContainsStringWithoutNewlines(string $expected, string $original): void { $split_original = preg_split("/(\r\n|\r|\n)/", $original); + if ($split_original === false) { + $this->fail("Unable to split string into lines"); + } array_walk($split_original, function (&$o) { $o = trim($o); }); @@ -302,4 +317,36 @@ protected function assertStringContainsStringWithoutNewlines(string $expected, s $final = trim($new_original); $this->assertStringContainsString($expected, $final); } + + /** + * Get the current working directory. + * + * @return string + * The current working directory. + * @throws Exception + * Unable to get the current working directory. + */ + protected static function getCwd(): string + { + $cwd = getcwd(); + if ($cwd === false) { + throw new Exception("Unable to get current working directory"); + } + return $cwd; + } + + /** + * @param string $path The path to the file. + * @param string $mode The mode to open the file in. + * @return resource The file pointer. + * @throws Exception Unable to open the file. + */ + protected static function openFile(string $path, string $mode) + { + $fp = fopen($path, $mode); + if ($fp === false) { + throw new Exception("Unable to open file " . basename($path)); + } + return $fp; + } } diff --git a/tests/BagItWebserverFramework.php b/tests/BagItWebserverFramework.php index 4532047..aa62286 100644 --- a/tests/BagItWebserverFramework.php +++ b/tests/BagItWebserverFramework.php @@ -62,9 +62,15 @@ public static function setUpBeforeClass(): void // Add custom headers if defined. $response_headers = [ 'Cache-Control' => 'no-cache', - 'Content-Length' => isset($file['content']) ? strlen($file['content']) : - stat($file['filename'])['size'] ] + ($file['headers'] ?? []); + if (isset($file['content'])) { + $response_headers['Content-Length'] = strlen($file['content']); + } else { + $stats = stat($file['filename']); + if ($stats !== false) { + $response_headers['Content-Length'] = $stats['size']; + } + } // Use custom status code if defined. $status_code = $file['status_code'] ?? 200; self::$remote_urls[$counter] = self::$webserver->setResponseOfPath( diff --git a/tests/BagTest.php b/tests/BagTest.php index 46a66e1..894ede0 100644 --- a/tests/BagTest.php +++ b/tests/BagTest.php @@ -57,7 +57,7 @@ public function testCreateBagRelative(): void mkdir($this->tmpdir); $newDir = $this->tmpdir . "/some-new-dir"; $this->assertDirectoryDoesNotExist($newDir); - $curr = getcwd(); + $curr = self::getCwd(); chdir($this->tmpdir); $bag = Bag::create("some-new-dir"); $this->assertTrue($bag->isValid()); @@ -75,7 +75,7 @@ public function testCreateBagRelative2(): void mkdir($this->tmpdir); $newDir = $this->tmpdir . "/some-new-dir"; $this->assertDirectoryDoesNotExist($newDir); - $curr = getcwd(); + $curr = self::getCwd(); chdir($this->tmpdir); $bag = Bag::create("./some-new-dir"); $this->assertTrue($bag->isValid()); @@ -883,7 +883,7 @@ public function testUpdateV07(): void $bag = Bag::load($this->tmpdir); $this->assertEquals('0.97', $bag->getVersionString()); $this->assertTrue($bag->isValid()); - $fp = fopen($bag->getBagRoot() . '/bag-info.txt', 'r'); + $fp = self::openFile($bag->getBagRoot() . '/bag-info.txt', 'r'); while (!feof($fp)) { $line = (string) fgets($fp); $line = trim($line); @@ -897,7 +897,7 @@ public function testUpdateV07(): void $bag->upgrade(); $this->assertEquals('1.0', $bag->getVersionString()); $this->assertTrue($bag->isValid()); - $fp = fopen($bag->getBagRoot() . '/bag-info.txt', 'r'); + $fp = self::openFile($bag->getBagRoot() . '/bag-info.txt', 'r'); while (!feof($fp)) { $line = (string) fgets($fp); $line = trim($line); @@ -982,7 +982,7 @@ public function testEmptyBagShouldValidate(): void public function testBagItTooManyLines(): void { $this->tmpdir = $this->prepareBasicTestBag(); - $fp = fopen($this->tmpdir . '/bagit.txt', 'a'); + $fp = self::openFile($this->tmpdir . '/bagit.txt', 'a'); fwrite($fp, "This is more stuff\n"); fclose($fp); $bag = Bag::load($this->tmpdir); @@ -997,7 +997,7 @@ public function testBagItTooManyLines(): void public function testBagItVersionLineInvalid(): void { $this->tmpdir = $this->prepareBasicTestBag(); - $fp = fopen($this->tmpdir . '/bagit.txt', 'w'); + $fp = self::openFile($this->tmpdir . '/bagit.txt', 'w'); fwrite($fp, "BagIt-Version: M.N\nTag-File-Character-Encoding: UTF-8\n"); fclose($fp); $bag = Bag::load($this->tmpdir); @@ -1012,7 +1012,7 @@ public function testBagItVersionLineInvalid(): void public function testBagItEncodingLineError(): void { $this->tmpdir = $this->prepareBasicTestBag(); - $fp = fopen($this->tmpdir . '/bagit.txt', 'w'); + $fp = self::openFile($this->tmpdir . '/bagit.txt', 'w'); fwrite($fp, "BagIt-Version: 1.0\nTag-File-Encoding: UTF-8\n"); fclose($fp); $bag = Bag::load($this->tmpdir); @@ -1027,7 +1027,7 @@ public function testBagItEncodingLineError(): void public function testFailOnEncodedBagIt(): void { $this->tmpdir = $this->prepareBasicTestBag(); - $fp = fopen($this->tmpdir . '/bagit.txt', 'w'); + $fp = self::openFile($this->tmpdir . '/bagit.txt', 'w'); $encoded_string = mb_convert_encoding("BagIt-Version: 1.0\nTag-File-Encoding: UTF-8\n", "byte4le"); fwrite($fp, $encoded_string); fclose($fp); @@ -1065,7 +1065,7 @@ public function testRelativePathsExists(): void $this->expectException(BagItException::class); $this->expectExceptionMessage("New bag directory $fullpath exists"); - $curr = getcwd(); + $curr = self::getCwd(); chdir($this->tmpdir); try { Bag::create("existing_bag"); @@ -1084,7 +1084,7 @@ public function testRelativePathDoesntExist(): void { // Make the directory mkdir($this->tmpdir); - $curr = getcwd(); + $curr = self::getCwd(); $full_path = $this->tmpdir . "/existing_bag"; chdir($this->tmpdir); $bag = Bag::create("existing_bag"); diff --git a/tests/BagUtilsTest.php b/tests/BagUtilsTest.php index 4d17621..13c02b9 100644 --- a/tests/BagUtilsTest.php +++ b/tests/BagUtilsTest.php @@ -91,7 +91,7 @@ public function testGetAbsolute(): void public function testGetAbsoluteRelative(): void { mkdir($this->tmpdir); - $current = getcwd(); + $current = self::getCwd(); chdir($this->tmpdir); $bag_name = "new_bag_directory"; $full_path = $this->tmpdir . '/' . $bag_name; diff --git a/tests/CurlInstanceTest.php b/tests/CurlInstanceTest.php index 981222f..9bdcfbd 100644 --- a/tests/CurlInstanceTest.php +++ b/tests/CurlInstanceTest.php @@ -6,7 +6,7 @@ class CurlInstanceTest extends BagItTestFramework { - public function testCreateCurl() + public function testCreateCurl(): void { $mock = new class { @@ -17,7 +17,7 @@ public function testCreateCurl() $this->assertInstanceOf(\CurlHandle::class, $handle); } - public function testCurlMulti() + public function testCurlMulti(): void { $mock = new class { diff --git a/tests/ExtendedBagTest.php b/tests/ExtendedBagTest.php index c4abc9b..cefb1ee 100644 --- a/tests/ExtendedBagTest.php +++ b/tests/ExtendedBagTest.php @@ -51,6 +51,9 @@ public function testNoTagManifest(): void $this->assertFalse($bag->isExtended()); $payloads = array_keys($bag->getPayloadManifests()); $hash = reset($payloads); + if ($hash === false) { + $this->fail("No payload manifest found."); + } $manifests = $bag->getTagManifests(); $this->assertCount(0, $manifests); @@ -823,9 +826,13 @@ private function switchLineEndingsTo(string $newEnding): void ]; foreach ($files as $file) { $path = $this->tmpdir . '/' . $file; + $contents = file_get_contents($path); + if ($contents === false) { + $this->fail("Failed to read file: $path"); + } file_put_contents( $path, - str_replace("\n", $newEnding, file_get_contents($path)) + str_replace("\n", $newEnding, $contents) ); } } diff --git a/tests/FetchTest.php b/tests/FetchTest.php index aeaafce..36fe7f7 100644 --- a/tests/FetchTest.php +++ b/tests/FetchTest.php @@ -8,6 +8,7 @@ use donatj\MockWebServer\Response; use whikloj\BagItTools\Bag; use whikloj\BagItTools\BagUtils; +use whikloj\BagItTools\DownloadFile; use whikloj\BagItTools\Exceptions\BagItException; use whikloj\BagItTools\Fetch; @@ -86,6 +87,33 @@ private function setupBag(string $fetchFile): Fetch return new Fetch($bag, true); } + /** + * Compare two arrays of Download files. + * @param array $expected The expected file. + * @param array $file The file to compare. + * @return void + */ + private function assertDownloadFilesEquals(array $expected, array $file): void + { + $this->assertEquals(count($expected), count($file)); + for ($foo = 0; $foo < count($expected); $foo += 1) { + $this->assertDownloadFileEquals($expected[$foo], $file[$foo]); + } + } + + /** + * Compare two DownloadFile objects. + * @param DownloadFile $expected The expected file. + * @param DownloadFile $file The file to compare. + * @return void + */ + private function assertDownloadFileEquals(DownloadFile $expected, DownloadFile $file): void + { + $this->assertEquals($expected->getUrl(), $file->getUrl()); + $this->assertEquals($expected->getDestination(), $file->getDestination()); + $this->assertEquals($expected->getSize(), $file->getSize()); + } + /** * Test destinations that resolve outside the data directory. * @group Fetch @@ -135,7 +163,8 @@ public function testFetchFileCorrectlyEncodedCharacters(): void * @covers ::downloadAll * @covers ::downloadFiles * @covers ::validateData - * @covers ::internalValidateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateDownload + * @covers \whikloj\BagItTools\DownloadFile::validateUrl */ public function testNotHttpUrl(): void { @@ -283,23 +312,20 @@ public function testRemoveFetchFile(): void $file_one_dest = 'data/dir1/dir2/first_text.txt'; $file_two_dest = 'data/dir1/dir2/second_text.txt'; $expected_with_both = [ - [ - 'uri' => self::$remote_urls[0], - 'size' => '-', - 'destination' => $file_one_dest, - ], - [ - 'uri' => self::$remote_urls[1], - 'size' => '-', - 'destination' => $file_two_dest, - ], + new DownloadFile( + self::$remote_urls[0], + $file_one_dest, + ), + new DownloadFile( + self::$remote_urls[1], + $file_two_dest, + ), ]; $expected_with_one = [ - [ - 'uri' => self::$remote_urls[1], - 'size' => '-', - 'destination' => $file_two_dest, - ], + new DownloadFile( + self::$remote_urls[1], + $file_two_dest, + ), ]; $bag = Bag::create($this->tmpdir); $this->assertFileDoesNotExist($bag->makeAbsolute($file_one_dest)); @@ -308,13 +334,13 @@ public function testRemoveFetchFile(): void $this->assertFileExists($bag->makeAbsolute($file_one_dest)); $bag->addFetchFile(self::$remote_urls[1], $file_two_dest); $this->assertFileExists($bag->makeAbsolute($file_one_dest)); - $this->assertEquals($expected_with_both, $bag->listFetchFiles()); + $this->assertDownloadFilesEquals($expected_with_both, $bag->listFetchFiles()); // Url doesn't exist, nothing happens $bag->removeFetchFile('http://example.org/not/real'); // Now really remove it. $bag->removeFetchFile(self::$remote_urls[0]); $this->assertFileDoesNotExist($bag->makeAbsolute($file_one_dest)); - $this->assertEquals($expected_with_one, $bag->listFetchFiles()); + $this->assertDownloadFilesEquals($expected_with_one, $bag->listFetchFiles()); } /** @@ -335,16 +361,14 @@ public function testClearFetch(): void $file_one_dest = 'data/dir1/dir2/first_text.txt'; $file_two_dest = 'data/dir1/dir2/second_text.txt'; $expected_with_both = [ - [ - 'uri' => self::$remote_urls[0], - 'size' => '-', - 'destination' => $file_one_dest, - ], - [ - 'uri' => self::$remote_urls[1], - 'size' => '-', - 'destination' => $file_two_dest, - ], + new DownloadFile( + self::$remote_urls[0], + $file_one_dest, + ), + new DownloadFile( + self::$remote_urls[1], + $file_two_dest, + ), ]; $bag = Bag::create($this->tmpdir); $this->assertFileDoesNotExist($bag->makeAbsolute($file_one_dest)); @@ -353,7 +377,7 @@ public function testClearFetch(): void $this->assertFileExists($bag->makeAbsolute($file_one_dest)); $bag->addFetchFile(self::$remote_urls[1], $file_two_dest); $this->assertFileExists($bag->makeAbsolute($file_one_dest)); - $this->assertEquals($expected_with_both, $bag->listFetchFiles()); + $this->assertDownloadFilesEquals($expected_with_both, $bag->listFetchFiles()); $bag->clearFetch(); $this->assertFileDoesNotExist($bag->makeAbsolute($file_one_dest)); $this->assertFileDoesNotExist($bag->makeAbsolute($file_two_dest)); @@ -371,16 +395,15 @@ public function testClearFetchFile(): void { $file_one_dest = 'data/dir1/dir2/first_text.txt'; $expected_with_both = [ - [ - 'uri' => self::$remote_urls[0], - 'size' => '-', - 'destination' => $file_one_dest, - ], + new DownloadFile( + self::$remote_urls[0], + $file_one_dest, + ), ]; $bag = Bag::create($this->tmpdir); $bag->addFetchFile(self::$remote_urls[0], $file_one_dest); $this->assertFileExists($bag->makeAbsolute($file_one_dest)); - $this->assertEquals($expected_with_both, $bag->listFetchFiles()); + $this->assertDownloadFilesEquals($expected_with_both, $bag->listFetchFiles()); $bag->update(); $this->assertFileExists($bag->makeAbsolute("fetch.txt")); $bag->clearFetch(); @@ -543,7 +566,7 @@ public function testMultiDownloadPartialFailure(): void /** * Test validate a URI without a scheme * @group Fetch - * @covers ::validateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateUrl * @covers ::validateData * @throws \ReflectionException */ @@ -552,26 +575,27 @@ public function testUriNoScheme(): void $reflection = $this->getReflectionMethod('whikloj\BagItTools\Fetch', 'validateData'); $bag = Bag::create($this->tmpdir); $fetch = new Fetch($bag); - $good_data = [ - 'uri' => 'http://example.org', - 'destination' => 'somewhere', - ]; + $good_data = new DownloadFile( + 'http://example.org', + 'somewhere' + ); $reflection->invokeArgs($fetch, [$good_data]); $this->expectException(BagItException::class); $this->expectExceptionMessage("URL somewhere.com does not seem to have a scheme or host"); - $bad_data = [ - 'uri' => 'somewhere.com', - 'destination' => 'somewhere', - ]; + $bad_data = new DownloadFile( + 'somewhere.com', + 'somewhere', + ); $reflection->invokeArgs($fetch, [$bad_data]); } /** * Test validate a URI without a host * @group Fetch - * @covers ::validateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateDownload * @covers ::validateData */ public function testUriNoHost(): void @@ -583,10 +607,10 @@ public function testUriNoHost(): void $this->expectException(BagItException::class); $this->expectExceptionMessage("URL http:// does not seem to have a scheme or host"); - $data = [ - 'uri' => 'http://', - 'destination' => 'somewhere', - ]; + $data = new DownloadFile( + 'http://', + 'somewhere', + ); $reflection->invokeArgs($fetch, [$data]); } @@ -594,26 +618,27 @@ public function testUriNoHost(): void * Test validate a URI with a scheme we don't support. * @group Fetch * @covers ::validateData - * @covers ::internalValidateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateUrl + * @covers \whikloj\BagItTools\DownloadFile::validateDownload */ public function testUriInvalidScheme(): void { $reflection = $this->getReflectionMethod('whikloj\BagItTools\Fetch', 'validateData'); $bag = Bag::create($this->tmpdir); $fetch = new Fetch($bag); - $good_data = [ - 'uri' => 'http://somewhere.com', - 'destination' => 'somewhere', - ]; + $good_data = new DownloadFile( + 'http://somewhere.com', + 'somewhere' + ); $reflection->invokeArgs($fetch, [$good_data]); $this->expectException(BagItException::class); $this->expectExceptionMessage("This library only supports http/https URLs"); - $bad_data = [ - 'uri' => 'ftp://somewhere.com', - 'destination' => 'somewhere', - ]; + $bad_data = new DownloadFile( + 'ftp://somewhere.com', + 'somewhere' + ); $reflection->invokeArgs($fetch, [$bad_data]); } @@ -666,7 +691,11 @@ public function testCreateFetchWithEncodedCharacters(): void } $bag->update(); // Read the fetch.txt file from disk. - $fetch = explode("\n", file_get_contents($bag->getBagRoot() . "/fetch.txt")); + $contents = file_get_contents($bag->getBagRoot() . "/fetch.txt"); + if ($contents === false) { + $this->fail("Failed to read fetch.txt file."); + } + $fetch = explode("\n", $contents); $fetch = array_filter($fetch); array_walk($fetch, function (&$o) { $o = trim(explode(" ", $o)[2]); diff --git a/tests/ManifestTest.php b/tests/ManifestTest.php index 1662259..d3d55f2 100644 --- a/tests/ManifestTest.php +++ b/tests/ManifestTest.php @@ -4,6 +4,7 @@ namespace whikloj\BagItTools\Test; +use Exception; use whikloj\BagItTools\Bag; use whikloj\BagItTools\Exceptions\BagItException; @@ -65,21 +66,21 @@ public function testCheckManifests(): void $this->assertFileExists($file); } - $fp = fopen($test_files['payload'], 'rb'); + $fp = self::openFile($test_files['payload'], 'rb'); $line = self::getLine($fp, $bag->getFileEncoding()); $expected_filepath = 'data/some/directory/file.txt'; $constraint1 = self::stringEndsWith($expected_filepath); $this->assertTrue($constraint1->evaluate($line, '', true)); fclose($fp); - $fp = fopen($test_files['tag'], 'rb'); + $fp = self::openFile($test_files['tag'], 'rb'); $constraints = self::logicalOr( self::stringEndsWith('bagit.txt'), self::stringEndsWith('bag-info.txt'), self::stringEndsWith('manifest-sha256.txt') ); while (feof($fp)) { - $line = $this->getLine($fp, $bag->getFileEncoding()); + $line = self::getLine($fp, $bag->getFileEncoding()); $this->assertTrue($constraints->evaluate($line, '', true)); } fclose($fp); @@ -190,7 +191,11 @@ public function testWriteManifestWithSpecialCharacters(): void $bag->addFile(self::TEST_TEXT['filename'], "already-encoded-%25-double-it.txt"); $bag->update(); // Read the lines from the manifest file. - $paths = explode("\n", file_get_contents($bag->getBagRoot() . "/manifest-sha512.txt")); + $contents = file_get_contents($bag->getBagRoot() . "/manifest-sha512.txt"); + if ($contents === false) { + $this->fail("Failed to read manifest file."); + } + $paths = explode("\n", $contents); $paths = array_filter($paths); array_walk($paths, function (&$o) { $o = trim(explode(" ", $o)[1]); @@ -261,10 +266,15 @@ private function prepareManifest(string $manifest_filename): void * The file encoding * @return string * The line from the file decoded to UTF-8. + * @throws Exception + * Unable to read line from file. */ private static function getLine($fp, string $file_encoding): string { $line = fgets($fp); + if ($line === false) { + throw new Exception("Failed to read line from file."); + } $line = mb_convert_encoding($line, 'UTF-8', $file_encoding); return trim($line); } diff --git a/tests/Profiles/BagItProfileFooTest.php b/tests/Profiles/BagItProfileFooTest.php index 8079482..5250cb9 100644 --- a/tests/Profiles/BagItProfileFooTest.php +++ b/tests/Profiles/BagItProfileFooTest.php @@ -2,8 +2,6 @@ namespace whikloj\BagItTools\Test\Profiles; -use whikloj\BagItTools\Test\Profiles\ProfileTestFramework; - /** * Test BagItProfile against the specifications foo * @package Profiles @@ -11,16 +9,25 @@ */ class BagItProfileFooTest extends ProfileTestFramework { + /** + * {@inheritDoc} + */ protected function getProfileFilename(): string { return self::TEST_RESOURCES . '/profiles/bagProfileFoo.json'; } + /** + * {@inheritDoc} + */ protected function getProfileUri(): string { return self::$remote_urls[0]; } + /** + * {@inheritDoc} + */ protected function getProfileValues(): array { return [ diff --git a/tests/Profiles/BagProfileTest.php b/tests/Profiles/BagProfileTest.php index 0ccf2fc..014f3f1 100644 --- a/tests/Profiles/BagProfileTest.php +++ b/tests/Profiles/BagProfileTest.php @@ -2,6 +2,7 @@ namespace whikloj\BagItTools\Test\Profiles; +use Exception; use whikloj\BagItTools\Bag; use whikloj\BagItTools\Exceptions\ProfileException; use whikloj\BagItTools\Profiles\BagItProfile; @@ -14,7 +15,7 @@ */ class BagProfileTest extends BagItTestFramework { - private static $profiles = self::TEST_RESOURCES . '/profiles'; + private static string $profiles = self::TEST_RESOURCES . '/profiles'; /** * Try to validate a bag. @@ -23,7 +24,11 @@ class BagProfileTest extends BagItTestFramework */ public function testValidateBag1(): void { - $profile = BagItProfile::fromJson(file_get_contents(self::$profiles . "/bagProfileFoo.json")); + $json = file_get_contents(self::$profiles . "/bagProfileFoo.json"); + if ($json === false) { + throw new Exception("Failed to read profile file"); + } + $profile = BagItProfile::fromJson($json); $this->assertTrue($profile->isValid()); $this->tmpdir = $this->prepareExtendedTestBag(); $bag = Bag::load($this->tmpdir); @@ -487,6 +492,9 @@ public function testAddProfileToBag(): void public function testAddSameProfileTwice(): void { $profileJson = file_get_contents(self::$profiles . "/bagProfileFoo.json"); + if ($profileJson === false) { + throw new Exception("Failed to read profile file"); + } $bag = Bag::create($this->tmpdir); $this->assertCount(0, $bag->getBagProfiles()); $bag->addBagProfileByJson($profileJson); @@ -510,6 +518,9 @@ public function testAddDifferentProfiles(): void $profile1 = file_get_contents(self::$profiles . "/bagProfileFoo.json"); $profile2 = file_get_contents(self::$profiles . "/bagProfileBar.json"); $profile3 = file_get_contents(self::$profiles . "/btrProfile.json"); + if ($profile1 === false || $profile2 === false || $profile3 === false) { + throw new Exception("Failed to read profile file"); + } $bag = Bag::create($this->tmpdir); $this->assertCount(0, $bag->getBagProfiles()); $bag->addBagProfileByJson($profile1); @@ -552,6 +563,9 @@ public function testClearAllProfiles(): void $profile1 = file_get_contents(self::$profiles . "/bagProfileFoo.json"); $profile2 = file_get_contents(self::$profiles . "/bagProfileBar.json"); $profile3 = file_get_contents(self::$profiles . "/btrProfile.json"); + if ($profile1 === false || $profile2 === false || $profile3 === false) { + throw new Exception("Failed to read profile file"); + } $bag = Bag::create($this->tmpdir); $this->assertCount(0, $bag->getBagProfiles()); $bag->addBagProfileByJson($profile1); @@ -569,6 +583,9 @@ public function testClearAllProfiles(): void public function testGetBagProfile(): void { $profile = file_get_contents(self::$profiles . "/bagProfileBar.json"); + if ($profile === false) { + throw new Exception("Failed to read profile file"); + } $bag = Bag::create($this->tmpdir); $bag->addBagProfileByJson($profile); $testProfiles = $bag->getBagProfiles(); diff --git a/tests/Profiles/ProfileTestFramework.php b/tests/Profiles/ProfileTestFramework.php index 363b57e..789787f 100644 --- a/tests/Profiles/ProfileTestFramework.php +++ b/tests/Profiles/ProfileTestFramework.php @@ -52,7 +52,7 @@ public static function setUpBeforeClass(): void protected BagItProfile $jsonProfile; /** - * @var array An array of the test values to match against the profile. + * @var array An array of the test values to match against the profile. */ protected array $profileValues = []; @@ -61,6 +61,9 @@ public function setUp(): void parent::setUp(); $json = file_get_contents($this->getProfileFilename()); + if ($json === false) { + throw new \Exception("Failed to read profile file."); + } $this->jsonProfile = BagItProfile::fromJson($json); $this->assertTrue($this->jsonProfile->isValid()); @@ -81,7 +84,7 @@ abstract protected function getProfileFilename(): string; abstract protected function getProfileUri(): string; /** - * @return array The expected values of the profile. + * @return array The expected values of the profile. */ abstract protected function getProfileValues(): array; @@ -474,11 +477,9 @@ public function testPayloadFilesAllowed(): void ); } - - /** * Assert the bag-info tags are as expected. - * @param array $expected The expected tags. + * @param array|mixed> $expected The expected tags. * @param BagItProfile $profile The profile to check. */ protected function assertProfileBagInfoTags(array $expected, BagItProfile $profile): void