Minor cleanup (#51)

* Reduce duplication and remove unnecessary checking * Replace DIRECTORY_SEPARATOR with / * use a local variable * Update test matrix for Windows * More README * Separate Fetch test to not run on Windows * Use standardize path separators * Less verbose phpunit * Add gitattributes for php files * remove comment * Debugging * Test changes * Fix assert * Update tests to work under Windows (#49) * Disabled git from converting newlines * Updated FetchTest to work under Windows * Run FetchTest * Disabled FetchTest and added missing extension to the build (#50) Co-authored-by: Jared Whiklo <[email protected]> * Re-enable full test matrix * Missed variable * Fix newline removal * Remove debugging statements Co-authored-by: Jonas Raoni Soares da Silva <[email protected]>
whikloj · Aug 2, 2022 · c6d0be5 · c6d0be5
1 parent f2496ce
commit c6d0be5
Show file tree

Hide file tree

Showing 17 changed files with 338 additions and 346 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1,2 @@
+* -text
+*.php eol=lf
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -15,18 +15,18 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php-versions: ["7.4", "8.0"]
+        php-versions: ["7.4", "8.0", "8.1"]
+        host-os: ["ubuntu-latest", "windows-latest"]
         experimental: [false]
-        host-os: ["ubuntu-latest"]
         include:
           - php-versions: "7.3"
             experimental: false
             host-os: "ubuntu-18.04"
-          - php-versions: "8.1"
-            experimental: true
-            host-os: "ubuntu-latest"
+          - php-versions: "7.3"
+            experimental: false
+            host-os: "windows-latest"
 
-    name: PHP ${{ matrix.php-versions }}
+    name: PHP ${{ matrix.php-versions }} - OS ${{ matrix.host-os }}
     steps:
       - name: Checkout
         uses: actions/checkout@v2
@@ -36,23 +36,34 @@ jobs:
         with:
           php-version: ${{ matrix.php-versions }}
           coverage: none
+          extensions: sockets, intl, bz2
 
       - name: Get composer cache directory
         id: composercache
-        run: echo "::set-output name=dir::$(composer config cache-files-dir)"
+        run: |
+          echo "::set-output name=dir::$(composer config cache-files-dir)"
 
       - name: Cache dependencies
         uses: actions/cache@v2
         with:
           path: ${{ steps.composercache.outputs.dir }}
           key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
-          restore-keys: ${{ runner.os }}-composer-
+          restore-keys: |
+            ${{ runner.os }}-composer-
 
       - name: Install dependencies
         run: composer install --prefer-dist --no-progress
 
-      - name: Run test suite
-        run: composer test
+      - name: Check codestyle
+        run: composer check
+
+      - name: Run test suite (Full)
+        run: composer phpunit
+        if: ${{ matrix.host-os != 'windows-latest' }}
+
+      - name: Run test suite (Windows)
+        run: phpdbg -qrr ./vendor/bin/phpunit --testsuite BagIt-Windows
+        if: ${{ matrix.host-os == 'windows-latest' }}
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v1

diff --git a/README.md b/README.md
@@ -105,6 +105,13 @@ $bag->addFile('../README.md', 'data/documentation/myreadme.md');
 // Add another algorithm
 $bag->addAlgorithm('sha1');
 
+// Get the algorithms
+$algos = $bag->getAlgorithms();
+var_dump($algos); // array(
+                  //   'sha512',
+                  //   'sha1',
+                  // )
+
 // Add a fetch url
 $bag->addFetchFile('http://www.google.ca', 'data/mywebsite.html');
 
@@ -125,10 +132,35 @@ if ($bag->hasBagInfoTag('contact-name')) {
 
     // Remove a specific tag value using array index from the above listing.
     $bag->removeBagInfoTagIndex('contact-name', 1); 
-
+
+    // Get tags
+    $tags = $bag->getBagInfoByTag('contact-name');
+
+    var_dump($tags); // array(
+                     //    'Jared Whiklo',
+                     // )
+
+    $bag->addBagInfoTag('Contact-NAME', 'Bob Saget');
     // Get tags
     $tags = $bag->getBagInfoByTag('contact-name');
 
+    var_dump($tags); // array(
+                     //    'Jared Whiklo',
+                     //    'Bob Saget',
+                     // )
+    // Without the case sensitive flag as true, you must be exact.
+    $bag->removeBagInfoTagValue('contact-name', 'bob saget');
+    $tags = $bag->getBagInfoByTag('contact-name');
+
+    var_dump($tags); // array(
+                     //    'Jared Whiklo',
+                     //    'Bob Saget',
+                     // )
+
+    // With the case sensitive flag set to false, you can be less careful
+    $bag->removeBagInfoTagValue('contact-name', 'bob saget', false);
+    $tags = $bag->getBagInfoByTag('contact-name');
+
     var_dump($tags); // array(
                      //    'Jared Whiklo',
                      // )

diff --git a/composer.json b/composer.json
@@ -23,7 +23,6 @@
   },
   "require-dev": {
     "phpunit/phpunit": "^9.0",
-    "phpdocumentor/phpdocumentor": "^3.0",
     "sebastian/phpcpd": "^6.0",
     "squizlabs/php_codesniffer": "^3.5",
     "donatj/mock-webserver": "^2.1",
@@ -51,7 +50,7 @@
       "./vendor/bin/phpcpd --suffix='.php' src"
     ],
     "phpunit": [
-      "phpdbg -qrr ./vendor/bin/phpunit"
+      "phpdbg -qrr ./vendor/bin/phpunit --testsuite BagIt"
     ],
     "test": [
       "@check",

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -18,9 +18,16 @@
       <clover outputFile="clover.xml"/>
     </report>
   </coverage>
-  <testsuite name="Bagit">
-    <directory suffix=".php">./tests</directory>
-    <exclude>./tests/BagItTestFramework.php</exclude>
-  </testsuite>
+  <testsuites>
+    <testsuite name="BagIt">
+      <directory suffix=".php">./tests</directory>
+      <exclude>./tests/BagItTestFramework.php</exclude>
+    </testsuite>
+    <testsuite name="BagIt-Windows">
+      <directory suffix=".php">./tests</directory>
+      <exclude>./tests/BagItTestFramework.php</exclude>
+      <exclude>./tests/FetchTest.php</exclude>
+    </testsuite>
+  </testsuites>
   <logging/>
 </phpunit>
diff --git a/src/Bag.php b/src/Bag.php
@@ -286,8 +286,8 @@ private function __construct(string $rootPath, bool $new = true)
             $this->validHashAlgorithms,
             array($this, 'normalizeHashAlgorithmName')
         );
-        $this->bagRoot = $this->internalPath($rootPath);
-        $this->bagRoot = $this->internalPath(BagUtils::getAbsolute($this->bagRoot, true));
+        $rootPath = BagUtils::standardizePathSeparators($rootPath);
+        $this->bagRoot = BagUtils::getAbsolute($rootPath, true);
         $this->loaded = (!$new);
         if ($new) {
             $this->createNewBag();
@@ -514,7 +514,7 @@ public function removeFile(string $dest): void
      * @param  string $dest
      *   The name of the file in the bag.
      * @throws \whikloj\BagItTools\Exceptions\BagItException
-     *   Source file does not exist or the destination is outside the data directory.
+     *   Generated source file does not exist or the destination is outside the data directory. Should not occur.
      * @throws \whikloj\BagItTools\Exceptions\FilesystemException
      *   Issues creating/deleting temporary file.
      */
@@ -542,15 +542,15 @@ public function createFile(string $string, string $dest): void
     public function makeAbsolute(string $path): string
     {
         $length = strlen(trim($this->bagRoot));
-        $path = $this->internalPath(BagUtils::getAbsolute($path));
+        $path = BagUtils::getAbsolute($path);
         if (substr($path, 0, $length) === $this->bagRoot) {
-            return mb_ereg_replace('\\\\|/', DIRECTORY_SEPARATOR, $path);
+            return $path;
         }
         $components = array_filter(explode("/", $path));
         $rootComponents = array_filter(explode("/", $this->bagRoot));
         $components = array_merge($rootComponents, $components);
-        $prefix = (preg_match('/^[a-z]:/i', $rootComponents[0] ?? '', $matches) ? '' : DIRECTORY_SEPARATOR);
-        return $prefix . implode(DIRECTORY_SEPARATOR, $components);
+        $prefix = (preg_match('/^[a-z]:/i', $rootComponents[0] ?? '', $matches) ? '' : '/');
+        return $prefix . implode('/', $components);
     }
 
     /**
@@ -563,7 +563,7 @@ public function makeAbsolute(string $path): string
      */
     public function makeRelative(string $path): string
     {
-        $path = $this->internalPath(BagUtils::getAbsolute($path));
+        $path = BagUtils::getAbsolute($path);
         $rootLength = strlen($this->bagRoot);
         if (substr($path, 0, $rootLength) !== $this->bagRoot) {
             // We are not in bag root so return nothing.
@@ -826,7 +826,7 @@ public function getAlgorithms(): array
     public function hasAlgorithm(string $hashAlgorithm): bool
     {
         $internal_name = $this->getHashName($hashAlgorithm);
-        return $this->hashIsSupported($internal_name) ? $this->hasHash($internal_name) : false;
+        return $this->hashIsSupported($internal_name) && $this->hasHash($internal_name);
     }
 
     /**
@@ -1198,13 +1198,7 @@ public function checkForEmptyDir(string $path): void
         $parentPath = dirname($path);
         if (substr($this->makeRelative($parentPath), 0, 5) == "data/") {
             $files = scandir($parentPath);
-            $payload = array_filter(
-                $files,
-                function ($o) {
-                    // Don't count directory specifiers.
-                    return (!BagUtils::isDotDir($o));
-                }
-            );
+            $payload = array_diff($files, [".", ".."]);
             if (count($payload) == 0) {
                 rmdir($parentPath);
             }
@@ -1283,7 +1277,7 @@ private function createNewBag(): void
         if (file_exists($root)) {
             throw new BagItException("New bag directory $root exists");
         }
-        BagUtils::checkedMkdir($root . DIRECTORY_SEPARATOR . "data", 0777, true);
+        BagUtils::checkedMkdir($root . "/data", 0777, true);
         $this->updateBagIt();
         $this->payloadManifests = [
             self::DEFAULT_HASH_ALGORITHM => new PayloadManifest($this, self::DEFAULT_HASH_ALGORITHM)
@@ -1645,7 +1639,7 @@ private static function wrapAtLength(string $text, int $length): array
     private function loadTagManifests(): bool
     {
         $tagManifests = [];
-        $pattern = $this->getBagRoot() . DIRECTORY_SEPARATOR . "tagmanifest-*.txt";
+        $pattern = $this->getBagRoot() . "/tagmanifest-*.txt";
         $files = BagUtils::findAllByPattern($pattern);
         if (count($files) < 1) {
             return false;
@@ -1707,7 +1701,7 @@ private function updateTagManifests(): void
      */
     private function clearTagManifests(): void
     {
-        $pattern = $this->getBagRoot() . DIRECTORY_SEPARATOR . "tagmanifest-*.txt";
+        $pattern = $this->getBagRoot() . "/tagmanifest-*.txt";
         $this->clearFilesOfPattern($pattern);
         unset($this->tagManifests);
     }
@@ -1760,7 +1754,7 @@ private function removeTagManifest(string $internal_name): void
      */
     private function loadPayloadManifests(): void
     {
-        $pattern = $this->getBagRoot() . DIRECTORY_SEPARATOR . "manifest-*.txt";
+        $pattern = $this->getBagRoot() . "/manifest-*.txt";
         $manifests = BagUtils::findAllByPattern($pattern);
         if (count($manifests) == 0) {
             $this->addBagError('manifest-ALG.txt', 'No payload manifest files found.');
@@ -1821,7 +1815,7 @@ private function updatePayloadManifests(): void
      */
     private function clearPayloadManifests(): void
     {
-        $pattern = $this->getBagRoot() . DIRECTORY_SEPARATOR . "manifest-*.txt";
+        $pattern = $this->getBagRoot() . "/manifest-*.txt";
         $this->clearFilesOfPattern($pattern);
     }
 
@@ -2210,14 +2204,11 @@ private static function hasExtension(string $filepath, array $extensions): bool
      */
     private static function getDirectory(string $filepath): string
     {
-        $files = scandir($filepath);
+        $files = array_diff(scandir($filepath), [".", ".."]);
         $dirs = [];
         if (count($files) > 0) {
             foreach ($files as $file) {
-                if (BagUtils::isDotDir($file)) {
-                    continue;
-                }
-                $fullpath = $filepath . DIRECTORY_SEPARATOR . $file;
+                $fullpath = $filepath . '/' . $file;
                 if (is_dir($fullpath)) {
                     $dirs[] = $fullpath;
                 }
@@ -2282,19 +2273,6 @@ private function addBagWarning(string $filename, string $message): void
         ];
     }
 
-    /**
-     * Convert paths from using the OS directory separator to using /.
-     *
-     * @param  string $path
-     *   The external path.
-     * @return string
-     *   The modified path.
-     */
-    private function internalPath(string $path): string
-    {
-        return str_replace(DIRECTORY_SEPARATOR, "/", $path);
-    }
-
     /**
      * Normalize a PHP hash algorithm to a BagIt specification name. Used to alter the incoming $item.
      *