FileSystemUtils: Only create backup copies if files differ

The FileSystemUtils class has a helper method to create files with
a .new suffix, in case the file, which should be created already
exists. If you install plugins and those have configuration files,
even without changes, you will end up with tons of .new files.

This commit checks the file size and sha-256 sum, and only if those
differ, a .new file is actually being created.

src/main/java/org/elasticsearch/common/io/FileSystemUtils.java

@@ -32,6 +32,9 @@ import java.nio.charset.Charset;
 import java.nio.charset.CharsetDecoder;
 import java.nio.file.*;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import static java.nio.file.FileVisitResult.CONTINUE;
@@ -203,16 +206,44 @@ public final class FileSystemUtils {
 
                 if (!Files.exists(path)) {
                     // We just move the new file to new dir
-                    Files.move(file, path);
+                    move(file, path);
                 } else if (suffix != null) {
-                    // If it already exists we try to copy this new version appending suffix to its name
-                    path = Paths.get(path.toString().concat(suffix));
-                    // We just move the file to new dir but with a new name (appended with suffix)
-                    Files.move(file, path, StandardCopyOption.REPLACE_EXISTING);
+                    if (!isSameFile(file, path)) {
+                        // If it already exists we try to copy this new version appending suffix to its name
+                        path = Paths.get(path.toString().concat(suffix));
+                        // We just move the file to new dir but with a new name (appended with suffix)
+                        Files.move(file, path, StandardCopyOption.REPLACE_EXISTING);
+                    }
                 }
 
                 return FileVisitResult.CONTINUE;
             }
+
+            /**
+             * Compares the content of two paths by comparing their SHA-SUM
+             */
+            private boolean isSameFile(Path first, Path second) throws IOException {
+                // do quick file size comparison before hashing
+                boolean sameFileSize = Files.size(first) == Files.size(second);
+                if (!sameFileSize) {
+                    return false;
+                }
+
+                try {
+                    MessageDigest messageDigest = MessageDigest.getInstance("SHA-256");
+
+                    messageDigest.update(Files.readAllBytes(first));
+                    byte[] pathSha = messageDigest.digest();
+                    messageDigest.reset();
+
+                    messageDigest.update(Files.readAllBytes(second));
+                    byte[] fileSha = messageDigest.digest();
+
+                    return Arrays.equals(pathSha, fileSha);
+                } catch (NoSuchAlgorithmException e) {
+                    return false;
+                }
+            }
         });
     }
 

src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common.io;
 
+import com.google.common.base.Charsets;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Assert;
 import org.junit.Before;
@@ -32,6 +33,7 @@ import java.nio.file.Paths;
 import java.util.Properties;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileNotExists;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
@@ -41,14 +43,13 @@ import static org.hamcrest.CoreMatchers.is;
  */
 public class FileSystemUtilsTests extends ElasticsearchTestCase {
 
-    Path src;
-    Path dst;
+    private Path src;
+    private Path dst;
 
     @Before
     public void copySourceFilesToTarget() throws IOException {
-        Path globalTempDir = globalTempDirPath();
-        src = globalTempDir.resolve("iocopyappend-src");
-        dst = globalTempDir.resolve("iocopyappend-dst");
+        src = newTempDirPath();
+        dst = newTempDirPath();
         Files.createDirectories(src);
         Files.createDirectories(dst);
 
@@ -117,6 +118,33 @@ public class FileSystemUtilsTests extends ElasticsearchTestCase {
         assertFileContent(dest, "dir/subdir/file5.txt", "version1");
     }
 
+    @Test
+    public void testMoveFilesDoesNotCreateSameFileWithSuffix() throws Exception {
+        Path[] dirs = new Path[] { newTempDirPath(), newTempDirPath(), newTempDirPath()};
+        for (Path dir : dirs) {
+            Files.write(dir.resolve("file1.txt"), "file1".getBytes(Charsets.UTF_8));
+            Files.createDirectory(dir.resolve("dir"));
+            Files.write(dir.resolve("dir").resolve("file2.txt"), "file2".getBytes(Charsets.UTF_8));
+        }
+
+        FileSystemUtils.moveFilesWithoutOverwriting(dirs[0], dst, ".new");
+        assertFileContent(dst, "file1.txt", "file1");
+        assertFileContent(dst, "dir/file2.txt", "file2");
+
+        // do the same operation again, make sure, no .new files have been added
+        FileSystemUtils.moveFilesWithoutOverwriting(dirs[1], dst, ".new");
+        assertFileContent(dst, "file1.txt", "file1");
+        assertFileContent(dst, "dir/file2.txt", "file2");
+        assertFileNotExists(dst.resolve("file1.txt.new"));
+        assertFileNotExists(dst.resolve("dir").resolve("file2.txt.new"));
+
+        // change file content, make sure it gets updated
+        Files.write(dirs[2].resolve("dir").resolve("file2.txt"), "UPDATED".getBytes(Charsets.UTF_8));
+        FileSystemUtils.moveFilesWithoutOverwriting(dirs[2], dst, ".new");
+        assertFileContent(dst, "file1.txt", "file1");
+        assertFileContent(dst, "dir/file2.txt", "file2");
+        assertFileContent(dst, "dir/file2.txt.new", "UPDATED");
+    }
 
     /**
      * Check that a file contains a given String

src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java

@@ -796,6 +796,13 @@ public class ElasticsearchAssertions {
         assertThat("file/dir [" + file + "] should exist.", Files.exists(file), is(true));
     }
 
+    /**
+     * Check if a file does not exist
+     */
+    public static void assertFileNotExists(Path file) {
+        assertThat("file/dir [" + file + "] should not exist.", Files.exists(file), is(false));
+    }
+
     /**
      * Check if a directory exists
      */