Implement unordered directory hash (#6197) [ci fast]

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

Author: pditommaso

modules/nf-commons/src/main/nextflow/util/HashBuilder.java

@@ -267,7 +267,7 @@ static private Hasher hashFile( Hasher hasher, Path path, HashMode mode, Path ba
 
         if( (mode==HashMode.STANDARD || mode==HashMode.LENIENT) && isAssetFile(path) ) {
             if( attrs==null ) {
-                // when file attributes are not avail or it's a directory
+                // when file attributes are not avail, or it's a directory
                 // hash the file using the file name path and the repository
                 log.warn("Unable to fetch attribute for file: {} - Hash is inferred from Git repository commit Id", FilesEx.toUriString(path));
                 return hashFileAsset(hasher, path);
@@ -322,18 +322,36 @@ static protected Hasher hashFileSha256( Hasher hasher, Path path, Path base ) {
         return hasher;
     }
 
+    /**
+     * Compute an, order independent, hash of a directory path traversing recursively the directory content.
+     *
+     * @param hasher
+     *      The {@link Hasher} object to which the resulting directory hash will be added.
+     * @param dir
+     *      The target directory path to be hashed.
+     * @param base
+     *      The "base" directory path against which resolve relative paths.
+     * @return
+     *      The resulting {@link Hasher} object updated with the directory path.
+     */
     static protected Hasher hashDirSha256( Hasher hasher, Path dir, Path base ) {
+        if( base==null )
+            throw new IllegalArgumentException("Argument 'base' cannot be null");
+        // the byte array used as "accumulator" for
+        final byte[] resultBytes = new byte[HASH_BYTES];
         try {
             Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {
                 public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
                     log.trace("Hash sha-256 dir content [FILE] path={} - base={}", path, base);
                     try {
                         // the file relative base
-                        if( base!=null )
-                            hasher.putUnencodedChars(base.relativize(path).toString());
+                        final String relPath = base.relativize(path).toString();
+                        // compute the file path hash and sum to the result hash
+                        // since the sum is commutative, the traverse order does not matter
+                        sumBytes(resultBytes, hashBytes(relPath, HashMode.STANDARD));
                         // the file content sha-256 checksum
-                        String sha256 = sha256Cache.get(path);
-                        hasher.putUnencodedChars(sha256);
+                        final String sha256 = sha256Cache.get(path);
+                        sumBytes(resultBytes, hashBytes(sha256, HashMode.STANDARD));
                         return FileVisitResult.CONTINUE;
                     }
                     catch (ExecutionException t) {
@@ -344,12 +362,15 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IO
                 public FileVisitResult preVisitDirectory(Path path, BasicFileAttributes attrs) {
                     log.trace("Hash sha-256 dir content [DIR] path={} - base={}", path, base);
                     // the file relative base
-                    if( base!=null )
-                        hasher.putUnencodedChars(base.relativize(path).toString());
-                    hasher.putUnencodedChars(base.relativize(path).toString());
+                    final String relPath = base.relativize(path).toString();
+                    // compute the file path hash and sum to the result hash
+                    // since the sum is commutative, the traverse order does not matter
+                    sumBytes(resultBytes, hashBytes(relPath, HashMode.STANDARD));
                     return FileVisitResult.CONTINUE;
                 }
             });
+            // finally put the result bytes in the hashing
+            hasher.putBytes(resultBytes);
         }
         catch (IOException t) {
             Throwable err = t.getCause()!=null ? t.getCause() : t;
@@ -441,21 +462,48 @@ static HashCode hashContent( Path file, HashFunction function ) {
     }
 
     static private Hasher hashUnorderedCollection(Hasher hasher, Collection collection, HashMode mode)  {
-
         byte[] resultBytes = new byte[HASH_BYTES];
         for (Object item : collection) {
-            byte[] nextBytes = HashBuilder.hasher(defaultHasher(), item, mode).hash().asBytes();
-            if( nextBytes.length != resultBytes.length )
-                throw new IllegalStateException("All hash codes must have the same bit length");
-
-            for (int i = 0; i < nextBytes.length; i++) {
-                resultBytes[i] += nextBytes[i];
-            }
+            // hash ghe collection item
+            byte[] nextBytes = hashBytes(item, mode);
+            // sum the hash bytes to the "resultBytes" accumulator
+            // since the sum is a commutative operation the order does not matter
+            sumBytes(resultBytes, nextBytes);
         }
-
+        // add the result bytes and return the resulting object
         return hasher.putBytes(resultBytes);
     }
 
+    static private byte[] hashBytes(Object item, HashMode mode) {
+        return hasher(defaultHasher(), item, mode).hash().asBytes();
+    }
+
+    /**
+     * Sum two arras of bytes having the same length, required to compute hash of unordered collections.
+     *
+     * - For each byte position, add the corresponding byte from nextBytes into resultBytes
+     * - Order doesn't matter: addition is commutative (a + b = b + a), so the final result is
+     *   the same no matter the order of items.
+     * - This is what makes it suitable for unordered collections
+     *
+     * @param resultBytes
+     *      The first argument to be summed. This array is used as the accumulator array (i.e. the result)
+     * @param nextBytes
+     *      The second argument to be summed.
+     * @return
+     *      The array resulting adding the bytes in the second array to the first one. Note,
+     *      the result array instance is the same object passed as first argument.
+     *
+     */
+    static private byte[] sumBytes(byte[] resultBytes, byte[] nextBytes) {
+        if( nextBytes.length != resultBytes.length )
+            throw new IllegalStateException("All hash codes must have the same bit length");
+        for (int i = 0; i < nextBytes.length; i++) {
+            resultBytes[i] += nextBytes[i];
+        }
+        return resultBytes;
+    }
+
     /**
      * Check if the argument is an asset file i.e. a file that makes part of the
      * pipeline Git repository

modules/nf-commons/src/test/nextflow/util/HashBuilderTest.groovy

@@ -110,13 +110,18 @@ class HashBuilderTest extends Specification {
         folder.resolve('dir1/bar').text = "I'm bar"
         folder.resolve('dir1/xxx/yyy').mkdirs()
         folder.resolve('dir1/xxx/foo1').text = "I'm foo within xxx"
-        folder.resolve('dir1/xxx/yyy/bar1').text = "I'm bar within yyy"
+        folder.resolve('dir1/xxx/yyy/bar1').text = "I'm bar 1 within yyy"
+        folder.resolve('dir1/xxx/yyy/bar2').text = "I'm bar 2 within yyy"
         and:
-        folder.resolve('dir2/foo').text = "I'm foo"
+        // create the same directory structure using a different
+        // creation order, the resulting hash should be the same 
         folder.resolve('dir2/bar').text = "I'm bar"
-        folder.resolve('dir2/xxx/yyy').mkdirs()
+        folder.resolve('dir2/foo').text = "I'm foo"
+        folder.resolve('dir2/xxx').mkdirs()
         folder.resolve('dir2/xxx/foo1').text = "I'm foo within xxx"
-        folder.resolve('dir2/xxx/yyy/bar1').text = "I'm bar within yyy"
+        folder.resolve('dir2/xxx/yyy').mkdirs()
+        folder.resolve('dir2/xxx/yyy/bar2').text = "I'm bar 2 within yyy"
+        folder.resolve('dir2/xxx/yyy/bar1').text = "I'm bar 1 within yyy"
 
         when:
         def hash1 = HashBuilder.hashDirSha256(HashBuilder.defaultHasher(), folder.resolve('dir1'), folder.resolve('dir1'))
@@ -125,6 +130,5 @@ class HashBuilderTest extends Specification {
 
         then:
         hash1.hash() == hash2.hash()
-
     }
 }