Skip to content

Commit 6fd46b4

Browse files
whiklojwhikloj
committed
More code cleanup
1 parent a4cdff2 commit 6fd46b4

15 files changed

+115
-112
lines changed

Diff for: ‎src/AbstractManifest.php

+17-25
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace whikloj\BagItTools;
44

5+
use Normalizer;
56
use whikloj\BagItTools\Exceptions\FilesystemException;
67

78
/**
@@ -48,13 +49,6 @@ abstract class AbstractManifest
4849
*/
4950
protected $filename;
5051

51-
/**
52-
* Array of files on disk to validate against.
53-
*
54-
* @var array
55-
*/
56-
protected $filesOnDisk = [];
57-
5852
/**
5953
* Errors while validating this manifest.
6054
*
@@ -91,6 +85,8 @@ abstract class AbstractManifest
9185
* The manifest filename.
9286
* @param boolean $load
9387
* Whether we are loading an existing file
88+
* @throws \whikloj\BagItTools\Exceptions\FilesystemException
89+
* Unable to read manifest file.
9490
*/
9591
protected function __construct(Bag $bag, string $algorithm, string $filename, bool $load = false)
9692
{
@@ -178,8 +174,8 @@ public function validate(): void
178174
$calculatedHash = strtolower($this->calculateHash($fullPath));
179175
$hash = strtolower($hash);
180176
if ($hash !== $calculatedHash) {
181-
$this->addError("{$path} calculated hash ({$calculatedHash}) does not match manifest " .
182-
"({$hash})");
177+
$this->addError("$path calculated hash ($calculatedHash) does not match manifest " .
178+
"($hash)");
183179
}
184180
}
185181
}
@@ -211,9 +207,9 @@ public function getHashes(): array
211207
protected function validatePath(string $path, string $filepath): void
212208
{
213209
if (!file_exists($filepath)) {
214-
$this->addError("{$path} does not exist.");
210+
$this->addError("$path does not exist.");
215211
} elseif ($this->bag->makeRelative($filepath) === "") {
216-
$this->addError("{$path} resolves to a path outside of the data/ directory.");
212+
$this->addError("$path resolves to a path outside of the data/ directory.");
217213
}
218214
}
219215

@@ -231,7 +227,7 @@ protected function loadFile(): void
231227
if (file_exists($fullPath)) {
232228
$file_contents = file_get_contents($fullPath);
233229
if ($file_contents === false) {
234-
throw new FilesystemException("Unable to read file {$fullPath}");
230+
throw new FilesystemException("Unable to read file $fullPath");
235231
}
236232
$lineCount = 0;
237233
$lines = BagUtils::splitFileDataOnLineEndings($file_contents);
@@ -250,17 +246,17 @@ protected function loadFile(): void
250246
// Normalized path in lowercase (for matching)
251247
$lowerNormalized = $this->normalizePath($path);
252248
if (array_key_exists($path, $this->hashes)) {
253-
$this->addLoadError("Line {$lineCount} : Path {$originalPath} appears more than once in " .
249+
$this->addLoadError("Line $lineCount: Path $originalPath appears more than once in " .
254250
"manifest.");
255251
} elseif ($this->matchNormalizedList($lowerNormalized)) {
256-
$this->addLoadWarning("Line {$lineCount} : Path {$originalPath} matches another file when " .
252+
$this->addLoadWarning("Line $lineCount: Path $originalPath matches another file when " .
257253
"normalized for case and characters.");
258254
} else {
259255
$this->hashes[$path] = $hash;
260256
$this->addToNormalizedList($lowerNormalized);
261257
}
262258
} else {
263-
$this->addLoadError("Line $lineCount : Line is not of the form 'checksum path'");
259+
$this->addLoadError("Line $lineCount: Line is not of the form 'checksum path'");
264260
}
265261
}
266262
}
@@ -280,11 +276,11 @@ protected function writeToDisk(): void
280276
}
281277
$fp = fopen(addslashes($fullPath), "w");
282278
if ($fp === false) {
283-
throw new FilesystemException("Unable to write {$fullPath}");
279+
throw new FilesystemException("Unable to write $fullPath");
284280
}
285281
foreach ($this->hashes as $path => $hash) {
286282
$path = BagUtils::encodeFilepath($path);
287-
$line = "{$hash} {$path}" . PHP_EOL;
283+
$line = "$hash $path" . PHP_EOL;
288284
$line = $this->bag->encodeText($line);
289285
BagUtils::checkedFwrite($fp, $line);
290286
}
@@ -399,19 +395,15 @@ private function matchNormalizedList(string $path): bool
399395
*
400396
* @param string $path
401397
* The path.
402-
* @param bool $toLower
403-
* Whether to also lowercase the string.
404398
* @return string
405399
* The normalized path.
406400
*/
407-
private function normalizePath(string $path, bool $toLower = true): string
401+
private function normalizePath(string $path): string
408402
{
409403
$path = urldecode($path);
410-
if ($toLower) {
411-
$path = strtolower($path);
412-
}
413-
if (!\Normalizer::isNormalized($path)) {
414-
$path = \Normalizer::normalize($path);
404+
$path = strtolower($path);
405+
if (!Normalizer::isNormalized($path)) {
406+
$path = Normalizer::normalize($path);
415407
}
416408
return $path;
417409
}

Diff for: ‎src/Bag.php

+36-33
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
namespace whikloj\BagItTools;
44

5+
use Archive_Tar;
6+
use Normalizer;
57
use whikloj\BagItTools\Exceptions\BagItException;
68
use whikloj\BagItTools\Exceptions\FilesystemException;
9+
use ZipArchive;
710

811
/**
912
* Bag class as normal interface for all actions and holder of supporting constructs.
@@ -253,7 +256,7 @@ class Bag
253256
*
254257
* @var boolean
255258
*/
256-
private $loaded = false;
259+
private $loaded;
257260

258261
/**
259262
* Bag constructor.
@@ -367,7 +370,7 @@ public function validate(): bool
367370
public function update(): void
368371
{
369372
if (!file_exists($this->makeAbsolute("data"))) {
370-
BagUtils::checkedMkdir($this->makeAbsolute("data"), 0777);
373+
BagUtils::checkedMkdir($this->makeAbsolute("data"));
371374
}
372375
$this->updateBagIt();
373376
$this->updatePayloadManifests();
@@ -438,16 +441,16 @@ public function addFile(string $source, string $dest): void
438441
if (file_exists($source)) {
439442
$dest = BagUtils::baseInData($dest);
440443
if (!$this->pathInBagData($dest)) {
441-
throw new BagItException("Path {$dest} resolves outside the bag.");
444+
throw new BagItException("Path $dest resolves outside the bag.");
442445
} elseif ($this->reservedFilename($dest)) {
443446
throw new BagItException("The filename requested is reserved on Windows OSes.");
444447
} elseif (isset($this->fetchFile) && $this->fetchFile->reservedPath($dest)) {
445-
throw new BagItException("The path ({$dest}) is used in the fetch.txt file.");
448+
throw new BagItException("The path ($dest) is used in the fetch.txt file.");
446449
} else {
447450
$fullDest = $this->makeAbsolute($dest);
448-
$fullDest = \Normalizer::normalize($fullDest);
451+
$fullDest = Normalizer::normalize($fullDest);
449452
if (file_exists($fullDest)) {
450-
throw new BagItException("File {$dest} already exists in the bag.");
453+
throw new BagItException("File $dest already exists in the bag.");
451454
}
452455
$dirname = dirname($fullDest);
453456
if (substr($this->makeRelative($dirname), 0, 5) == "data/") {
@@ -460,7 +463,7 @@ public function addFile(string $source, string $dest): void
460463
$this->changed = true;
461464
}
462465
} else {
463-
throw new BagItException("{$source} does not exist");
466+
throw new BagItException("$source does not exist");
464467
}
465468
}
466469

@@ -669,7 +672,7 @@ public function addBagInfoTag(string $tag, string $value): void
669672
}
670673
$internal_tag = self::trimLower($tag);
671674
if (in_array($internal_tag, self::BAG_INFO_GENERATED_ELEMENTS)) {
672-
throw new BagItException("Field {$tag} is auto-generated and cannot be manually set.");
675+
throw new BagItException("Field $tag is auto-generated and cannot be manually set.");
673676
}
674677
if (!$this->bagInfoTagExists($internal_tag)) {
675678
$this->bagInfoTagIndex[$internal_tag] = [];
@@ -695,7 +698,7 @@ public function setFileEncoding(string $encoding): void
695698
$encoding = self::trimLower($encoding);
696699
$charset = BagUtils::getValidCharset($encoding);
697700
if (is_null($charset)) {
698-
throw new BagItException("Character set {$encoding} is not supported.");
701+
throw new BagItException("Character set $encoding is not supported.");
699702
} else {
700703
if ($encoding == self::trimLower(self::DEFAULT_FILE_ENCODING)) {
701704
// go back to default.
@@ -786,7 +789,7 @@ public function addAlgorithm(string $algorithm): void
786789
}
787790
$this->changed = true;
788791
} else {
789-
throw new BagItException("Algorithm {$algorithm} is not supported.");
792+
throw new BagItException("Algorithm $algorithm is not supported.");
790793
}
791794
}
792795

@@ -819,7 +822,7 @@ public function removeAlgorithm(string $algorithm): void
819822
}
820823
$this->changed = true;
821824
} else {
822-
throw new BagItException("Algorithm {$algorithm} is not supported.");
825+
throw new BagItException("Algorithm $algorithm is not supported.");
823826
}
824827
}
825828

@@ -848,7 +851,7 @@ public function setAlgorithm(string $algorithm): void
848851
}
849852
$this->changed = true;
850853
} else {
851-
throw new BagItException("Algorithm {$algorithm} is not supported.");
854+
throw new BagItException("Algorithm $algorithm is not supported.");
852855
}
853856
}
854857

@@ -1139,7 +1142,7 @@ private function loadBag(): void
11391142
{
11401143
$root = $this->getBagRoot();
11411144
if (!file_exists($root)) {
1142-
throw new BagItException("Path {$root} does not exist, could not load Bag.");
1145+
throw new BagItException("Path $root does not exist, could not load Bag.");
11431146
}
11441147
$this->resetErrorsAndWarnings();
11451148
// Reset these or we end up with double manifests in a validate() situation.
@@ -1166,7 +1169,7 @@ private function createNewBag(): void
11661169
{
11671170
$this->resetErrorsAndWarnings();
11681171
if (file_exists($this->bagRoot)) {
1169-
throw new BagItException("New bag directory {$this->bagRoot} exists");
1172+
throw new BagItException("New bag directory $this->bagRoot exists");
11701173
}
11711174
BagUtils::checkedMkdir($this->bagRoot . DIRECTORY_SEPARATOR . "data", 0777, true);
11721175
$this->updateBagIt();
@@ -1215,7 +1218,7 @@ private function loadBagInfo(): bool
12151218
if (file_exists($fullPath)) {
12161219
$file_contents = file_get_contents($fullPath);
12171220
if ($file_contents === false) {
1218-
throw new FilesystemException("Unable to access {$info_file}");
1221+
throw new FilesystemException("Unable to access $info_file");
12191222
}
12201223
$bagData = [];
12211224
$lineCount = 0;
@@ -1248,18 +1251,18 @@ private function loadBagInfo(): bool
12481251
if ($this->mustNotRepeatBagInfoExists($current_tag)) {
12491252
$this->addBagError(
12501253
$info_file,
1251-
"Tag {$current_tag} MUST not be repeated. (Line {$lineCount})"
1254+
"Line $lineCount: Tag $current_tag MUST not be repeated."
12521255
);
12531256
} elseif ($this->shouldNotRepeatBagInfoExists($current_tag)) {
12541257
$this->addBagWarning(
12551258
$info_file,
1256-
"Tag {$current_tag} SHOULD NOT be repeated. (Line {$lineCount})"
1259+
"Line $lineCount: Tag $current_tag SHOULD NOT be repeated."
12571260
);
12581261
}
12591262
if (($this->compareVersion('1.0') <= 0) && (!empty($matches[1]) || !empty($matches[3]))) {
12601263
$this->addBagError(
12611264
$info_file,
1262-
"Labels cannot begin or end with a whitespace. (Line {$lineCount})"
1265+
"Line $lineCount: Labels cannot begin or end with a whitespace."
12631266
);
12641267
}
12651268
if ($lineLength >= Bag::BAGINFO_AUTOWRAP_GUESS_LENGTH) {
@@ -1365,7 +1368,7 @@ private function updateBagInfo(): void
13651368
// We don't guarantee newlines remain once you edit a bag.
13661369
$value = str_replace("\r\n", " ", $value);
13671370
$value = str_replace("\n", " ", $value);
1368-
$data = self::wrapBagInfoText("{$tag}: {$value}");
1371+
$data = self::wrapBagInfoText("$tag: $value");
13691372
foreach ($data as $line) {
13701373
$line = $this->encodeText($line);
13711374
BagUtils::checkedFwrite($fp, $line . PHP_EOL);
@@ -1513,7 +1516,7 @@ function ($o) {
15131516
private static function wrapAtLength(string $text, int $length): array
15141517
{
15151518
$text = str_replace("\n", "", $text);
1516-
$wrapped = wordwrap($text, $length, "\n");
1519+
$wrapped = wordwrap($text, $length);
15171520
return explode("\n", $wrapped);
15181521
}
15191522

@@ -1537,7 +1540,7 @@ private function loadTagManifests(): bool
15371540
if (isset($tagManifests[$hash])) {
15381541
$this->addBagError(
15391542
$this->makeRelative($file),
1540-
"More than one tag manifest for hash ({$hash}) found."
1543+
"More than one tag manifest for hash ($hash) found."
15411544
);
15421545
} else {
15431546
$tagManifests[$hash] = new TagManifest($this, $hash, true);
@@ -1653,7 +1656,7 @@ private function loadPayloadManifests(): void
16531656
$hash = self::determineHashFromFilename($manifest);
16541657
$relative_filename = $this->makeRelative($manifest);
16551658
if (!is_null($hash) && !in_array($hash, array_keys(self::HASH_ALGORITHMS))) {
1656-
throw new BagItException("We do not support the algorithm {$hash}");
1659+
throw new BagItException("We do not support the algorithm $hash");
16571660
} elseif (is_null($hash)) {
16581661
$this->addBagError(
16591662
$relative_filename,
@@ -1662,7 +1665,7 @@ private function loadPayloadManifests(): void
16621665
} elseif (isset($this->payloadManifests[$hash])) {
16631666
$this->addBagError(
16641667
$relative_filename,
1665-
"More than one payload manifest for hash ({$hash}) found."
1668+
"More than one payload manifest for hash ($hash) found."
16661669
);
16671670
} else {
16681671
$temp = new PayloadManifest($this, $hash, true);
@@ -1761,7 +1764,7 @@ private function loadBagIt(): void
17611764
} else {
17621765
$contents = file_get_contents($fullPath);
17631766
if ($contents === false) {
1764-
throw new FilesystemException("Unable to read {$fullPath}");
1767+
throw new FilesystemException("Unable to read $fullPath");
17651768
}
17661769
$lines = BagUtils::splitFileDataOnLineEndings($contents);
17671770
// remove blank lines.
@@ -1891,14 +1894,14 @@ private function makePackage(string $filename): void
18911894
*/
18921895
private function makeZip(string $filename): void
18931896
{
1894-
$zip = new \ZipArchive();
1895-
$res = $zip->open($filename, \ZipArchive::CREATE);
1897+
$zip = new ZipArchive();
1898+
$res = $zip->open($filename, ZipArchive::CREATE);
18961899
if ($res === true) {
18971900
$files = BagUtils::getAllFiles($this->bagRoot);
18981901
$parentPrefix = basename($this->bagRoot);
18991902
foreach ($files as $file) {
19001903
$relative = $this->makeRelative($file);
1901-
$zip->addFile($file, "{$parentPrefix}/{$relative}");
1904+
$zip->addFile($file, "$parentPrefix/$relative");
19021905
}
19031906
$zip->close();
19041907
} else {
@@ -1918,14 +1921,14 @@ private function makeTar(string $filename): void
19181921
{
19191922
$extension = self::getExtensions($filename);
19201923
$compression = self::extensionTarCompression($extension);
1921-
$tar = new \Archive_Tar($filename, $compression);
1924+
$tar = new Archive_Tar($filename, $compression);
19221925
if ($tar === false) {
19231926
throw new FilesystemException("Error creating Tar file.");
19241927
}
19251928
$parent = $this->getParentDir();
19261929
$files = BagUtils::getAllFiles($this->bagRoot);
19271930
if (!$tar->createModify($files, "", $parent)) {
1928-
throw new FilesystemException("Error adding files to {$filename}.");
1931+
throw new FilesystemException("Error adding files to $filename.");
19291932
}
19301933
}
19311934

@@ -1955,7 +1958,7 @@ private function getParentDir(): string
19551958
private static function uncompressBag(string $filepath): string
19561959
{
19571960
if (!file_exists($filepath)) {
1958-
throw new BagItException("File {$filepath} does not exist.");
1961+
throw new BagItException("File $filepath does not exist.");
19591962
}
19601963
$extension = self::getExtensions($filepath);
19611964
if (in_array($extension, self::ZIP_EXTENSIONS)) {
@@ -1981,7 +1984,7 @@ private static function uncompressBag(string $filepath): string
19811984
*/
19821985
private static function unzipBag(string $filename): string
19831986
{
1984-
$zip = new \ZipArchive();
1987+
$zip = new ZipArchive();
19851988
$res = $zip->open($filename);
19861989
if ($res === false) {
19871990
throw new FilesystemException("Unable to unzip $filename");
@@ -2008,10 +2011,10 @@ private static function untarBag(string $filename, string $extension): string
20082011
{
20092012
$compression = self::extensionTarCompression($extension);
20102013
$directory = self::extractDir();
2011-
$tar = new \Archive_Tar($filename, $compression);
2014+
$tar = new Archive_Tar($filename, $compression);
20122015
$res = $tar->extract($directory);
20132016
if ($res === false) {
2014-
throw new FilesystemException("Usable to untar {$filename}");
2017+
throw new FilesystemException("Unable to untar $filename");
20152018
}
20162019
return $directory;
20172020
}

0 commit comments

Comments
 (0)