Bug 796953 - [OS.File] writeAtomic should accept an option noOverwrite. r=Yoric

toolkit/components/osfile/osfile_async_front.jsm

@@ -635,6 +635,8 @@ File.read = function read(path, bytes) {
  * - {number} bytes The number of bytes to write. If unspecified,
  * |buffer.byteLength|. Required if |buffer| is a C pointer.
  * - {string} tmpPath The path at which to write the temporary file.
+ * - {bool} noOverwrite - If set, this function will fail if a file already
+ * exists at |path|. The |tmpPath| is not overwritten if |path| exist.
  *
  * @return {promise}
  * @resolves {number} The number of bytes actually written.

toolkit/components/osfile/osfile_shared_front.jsm

@@ -307,6 +307,23 @@ AbstractFile.read = function read(path, bytes) {
   }
 };
 
+/**
+ * Find outs if a file exists.
+ *
+ * @param {string} path The path to the file.
+ *
+ * @return {bool} true if the file exists, false otherwise.
+ */
+AbstractFile.exists = function exists(path) {
+  try {
+    let file = exports.OS.File.open(path);
+    file.close();
+    return true;
+  } catch (x) {
+    return false;
+  }
+};
+
 /**
  * Write a file, atomically.
  *
@@ -324,6 +341,8 @@ AbstractFile.read = function read(path, bytes) {
  * - {number} bytes The number of bytes to write. If unspecified,
  * |buffer.byteLength|. Required if |buffer| is a C pointer.
  * - {string} tmpPath The path at which to write the temporary file.
+ * - {bool} noOverwrite - If set, this function will fail if a file already
+ * exists at |path|. The |tmpPath| is not overwritten if |path| exist.
  *
  * @return {number} The number of bytes actually written.
  */
@@ -335,6 +354,12 @@ AbstractFile.writeAtomic =
   if (!tmpPath) {
     throw new TypeError("Expected option tmpPath");
   }
+
+  let noOverwrite = options.noOverwrite;
+  if (noOverwrite && OS.File.exists(path)) {
+    throw OS.File.Error.exists("writeAtomic");
+  }
+
   let tmpFile = OS.File.open(tmpPath, {write: true, truncate: true});
   let bytesWritten;
   try {

toolkit/components/osfile/osfile_unix_allthreads.jsm

@@ -185,4 +185,8 @@ if (typeof Components != "undefined") {
   OSError.closed = function closed(operation) {
     return new OSError(operation, OS.Constants.libc.EBADF);
   };
+
+  OSError.exists = function exists(operation) {
+    return new OSError(operation, OS.Constants.libc.EEXIST);
+  };
 })(this);

toolkit/components/osfile/osfile_unix_front.jsm

@@ -828,6 +828,7 @@
      };
 
      File.read = exports.OS.Shared.AbstractFile.read;
+     File.exists = exports.OS.Shared.AbstractFile.exists;
      File.writeAtomic = exports.OS.Shared.AbstractFile.writeAtomic;
 
      /**

toolkit/components/osfile/osfile_win_allthreads.jsm

@@ -194,4 +194,7 @@ if (typeof Components != "undefined") {
     return new OSError(operation, exports.OS.Constants.Win.INVALID_HANDLE_VALUE);
   };
 
+  OSError.exists = function exists(operation) {
+    return new OSError(operation, exports.OS.Constants.Win.ERROR_FILE_EXISTS);
+  };
 })(this);

toolkit/components/osfile/osfile_win_front.jsm

@@ -822,6 +822,7 @@
      };
 
      File.read = exports.OS.Shared.AbstractFile.read;
+     File.exists = exports.OS.Shared.AbstractFile.exists;
      File.writeAtomic = exports.OS.Shared.AbstractFile.writeAtomic;
 
      /**

toolkit/components/osfile/tests/mochi/main_test_osfile_async.js

@@ -520,6 +520,36 @@ let test_read_write_all = maketest(
     );
     promise = ensureSuccess(promise, test);
 
+// Check that writeAtomic fails if noOverwrite is true and the destination file already exists!
+
+    promise = promise.then(
+      function check_with_noOverwrite() {
+        let view = new Uint8Array(contents.buffer, 10, 200);
+        options = {tmpPath: tmpPath, noOverwrite: true};
+        return OS.File.writeAtomic(pathDest, view, options);
+      }
+    );
+
+    promise = promise.then(
+      function onSuccess() {
+        test.fail("With noOverwrite, writeAtomic should have refused to overwrite file");
+      },
+      function onFailure(err) {
+        test.info("With noOverwrite, writeAtomic correctly failed");
+        test.ok(err instanceof OS.File.Error, "writeAtomic correctly failed with a file error");
+        test.ok(err.becauseExists, "writeAtomic file error confirmed that the file already exists");
+        return reference_compare_files(pathSource, pathDest, test);
+      }
+    );
+
+    promise = promise.then(
+      function compare_complete() {
+        test.info("With noOverwrite, writeAtomic correctly did not overwrite destination file");
+        test.ok(!(new FileUtils.File(tmpPath).exists()), "Temporary file was removed");
+      }
+    );
+    promise = ensureSuccess(promise, test);
+
 // Now write a subset
 
     let START = 10;
@@ -570,7 +600,9 @@ let test_read_write_all = maketest(
       },
       function onFailure() {
         test.ok("Without a tmpPath, writeAtomic has failed as expected");
-      });
+      }
+    );
+
     return promise;
   }
 );

toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js

@@ -355,6 +355,20 @@ function test_readall_writeall_file()
   compare_files("test_readall_writeall_file (OS.File.readAll + writeAtomic 2)",
                 src_file_name, tmp_file_name);
 
+  // File.writeAtomic on top of existing file but without overwritten the file
+  exn = null;
+  try {
+    let view = new Uint8Array(readResult.buffer, 10, 200);
+    OS.File.writeAtomic(tmp_file_name, view,
+      { tmpPath: tmp_file_name + ".tmp", noOverwrite: true});
+  } catch (x) {
+    exn = x;
+  }
+  ok(exn && exn instanceof OS.File.Error && exn.becauseExists, "writeAtomic fails if file already exists with noOverwrite option");
+  // Check file was not overwritten.
+  compare_files("test_readall_writeall_file (OS.File.readAll + writeAtomic check file was not overwritten)",
+                src_file_name, tmp_file_name);
+
   // Ensure that File.writeAtomic fails if no temporary file name is provided
   // (FIXME: Remove this test as part of bug 793660)
 
@@ -365,7 +379,7 @@ function test_readall_writeall_file()
   } catch (x) {
     exn = x;
   }
-  ok(!!exn && exn instanceof TypeError, "wrietAtomic fails if tmpPath is not provided");
+  ok(!!exn && exn instanceof TypeError, "writeAtomic fails if tmpPath is not provided");
 }
 
 /**