From 21cc9cb7a76d805fbb7b53583448aa101c294e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sabiniarz?= <31597105+mhvsa@users.noreply.github.com> Date: Tue, 21 Jan 2020 10:49:42 +0100 Subject: [PATCH] Implemented alternative open mode in files (#3119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartek Iwańczuk --- cli/js/deno.ts | 1 + cli/js/files.ts | 112 +++++++++++++++++++++++++++++-- cli/js/files_test.ts | 62 ++++++++++++++++- cli/js/lib.deno_runtime.d.ts | 53 ++++++++++++++- cli/ops/files.rs | 125 +++++++++++++++++++++++------------ 5 files changed, 298 insertions(+), 55 deletions(-) diff --git a/cli/js/deno.ts b/cli/js/deno.ts index 513d9f2842..e53f9a63af 100644 --- a/cli/js/deno.ts +++ b/cli/js/deno.ts @@ -19,6 +19,7 @@ export { seek, seekSync, close, + OpenOptions, OpenMode } from "./files.ts"; export { diff --git a/cli/js/files.ts b/cli/js/files.ts index 1d04c0e702..c966b9fa04 100644 --- a/cli/js/files.ts +++ b/cli/js/files.ts @@ -20,22 +20,69 @@ import { /** Open a file and return an instance of the `File` object * synchronously. * - * const file = Deno.openSync("/foo/bar.txt"); + * const file = Deno.openSync("/foo/bar.txt", { read: true, write: true }); */ -export function openSync(filename: string, mode: OpenMode = "r"): File { - const rid = sendSyncJson(dispatch.OP_OPEN, { filename, mode }); +export function openSync(filename: string, capability?: OpenOptions): File; +/** Open a file and return an instance of the `File` object + * synchronously. + * + * const file = Deno.openSync("/foo/bar.txt", "r"); + */ +export function openSync(filename: string, mode?: OpenMode): File; + +export function openSync( + filename: string, + modeOrOptions: OpenOptions | OpenMode = "r" +): File { + let mode = null; + let options = null; + + if (typeof modeOrOptions === "string") { + mode = modeOrOptions; + } else { + checkOpenOptions(modeOrOptions); + options = modeOrOptions; + } + + const rid = sendSyncJson(dispatch.OP_OPEN, { filename, options, mode }); return new File(rid); } /** Open a file and return an instance of the `File` object. * - * const file = await Deno.open("/foo/bar.txt"); + * const file = await Deno.open("/foo/bar.txt", { read: true, write: true }); */ export async function open( filename: string, - mode: OpenMode = "r" + options?: OpenOptions +): Promise; + +/** Open a file and return an instance of the `File` object. + * + * const file = await Deno.open("/foo/bar.txt, "w+"); + */ +export async function open(filename: string, mode?: OpenMode): Promise; + +/**@internal*/ +export async function open( + filename: string, + modeOrOptions: OpenOptions | OpenMode = "r" ): Promise { - const rid = await sendAsyncJson(dispatch.OP_OPEN, { filename, mode }); + let mode = null; + let options = null; + + if (typeof modeOrOptions === "string") { + mode = modeOrOptions; + } else { + checkOpenOptions(modeOrOptions); + options = modeOrOptions; + } + + const rid = await sendAsyncJson(dispatch.OP_OPEN, { + filename, + options, + mode + }); return new File(rid); } @@ -216,6 +263,36 @@ export const stdout = new File(1); /** An instance of `File` for stderr. */ export const stderr = new File(2); +export interface OpenOptions { + /** Sets the option for read access. This option, when true, will indicate that the file should be read-able if opened. */ + read?: boolean; + /** Sets the option for write access. + * This option, when true, will indicate that the file should be write-able if opened. + * If the file already exists, any write calls on it will overwrite its contents, without truncating it. + */ + write?: boolean; + /** Sets the option for creating a new file. + * This option indicates whether a new file will be created if the file does not yet already exist. + * In order for the file to be created, write or append access must be used. + */ + create?: boolean; + /** Sets the option for truncating a previous file. + * If a file is successfully opened with this option set it will truncate the file to 0 length if it already exists. + * The file must be opened with write access for truncate to work. + */ + truncate?: boolean; + /**Sets the option for the append mode. + * This option, when true, means that writes will append to a file instead of overwriting previous contents. + * Note that setting { write: true, append: true } has the same effect as setting only { append: true }. + */ + append?: boolean; + /** Sets the option to always create a new file. + * This option indicates whether a new file will be created. No file is allowed to exist at the target location, also no (dangling) symlink. + * If { createNew: true } is set, create and truncate are ignored. + */ + createNew?: boolean; +} + export type OpenMode = /** Read-only. Default. Starts at beginning of file. */ | "r" @@ -241,3 +318,26 @@ export type OpenMode = | "x" /** Read-write. Behaves like `x` and allows to read from file. */ | "x+"; + +/** Check if OpenOptions is set to valid combination of options. + * @returns Tuple representing if openMode is valid and error message if it's not + * @internal + */ +function checkOpenOptions(options: OpenOptions): void { + if (Object.values(options).filter(val => val === true).length === 0) { + throw new Error("OpenOptions requires at least one option to be true"); + } + + if (options.truncate && !options.write) { + throw new Error("'truncate' option requires 'write' option"); + } + + const createOrCreateNewWithoutWriteOrAppend = + (options.create || options.createNew) && !(options.write || options.append); + + if (createOrCreateNewWithoutWriteOrAppend) { + throw new Error( + "'create' or 'createNew' options require 'write' or 'append' option" + ); + } +} diff --git a/cli/js/files_test.ts b/cli/js/files_test.ts index a4881296a3..fca9a38593 100644 --- a/cli/js/files_test.ts +++ b/cli/js/files_test.ts @@ -1,5 +1,11 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -import { test, testPerm, assert, assertEquals } from "./test_util.ts"; +import { + test, + testPerm, + assert, + assertEquals, + assertStrContains +} from "./test_util.ts"; test(function filesStdioFileDescriptors(): void { assertEquals(Deno.stdin.rid, 0); @@ -78,9 +84,55 @@ testPerm({ write: false }, async function writePermFailure(): Promise { } }); +test(async function openOptions(): Promise { + const filename = "cli/tests/fixture.json"; + let err; + try { + await Deno.open(filename, { write: false }); + } catch (e) { + err = e; + } + assert(!!err); + assertStrContains( + err.message, + "OpenOptions requires at least one option to be true" + ); + + try { + await Deno.open(filename, { truncate: true, write: false }); + } catch (e) { + err = e; + } + assert(!!err); + assertStrContains(err.message, "'truncate' option requires 'write' option"); + + try { + await Deno.open(filename, { create: true, write: false }); + } catch (e) { + err = e; + } + assert(!!err); + assertStrContains( + err.message, + "'create' or 'createNew' options require 'write' or 'append' option" + ); + + try { + await Deno.open(filename, { createNew: true, append: false }); + } catch (e) { + err = e; + } + assert(!!err); + assertStrContains( + err.message, + "'create' or 'createNew' options require 'write' or 'append' option" + ); +}); + testPerm({ read: false }, async function readPermFailure(): Promise { let caughtError = false; try { + await Deno.open("package.json", "r"); await Deno.open("cli/tests/fixture.json", "r"); } catch (e) { caughtError = true; @@ -95,7 +147,12 @@ testPerm({ write: true }, async function writeNullBufferFailure(): Promise< > { const tempDir = Deno.makeTempDirSync(); const filename = tempDir + "hello.txt"; - const file = await Deno.open(filename, "w"); + const w = { + write: true, + truncate: true, + create: true + }; + const file = await Deno.open(filename, w); // writing null should throw an error let err; @@ -183,7 +240,6 @@ testPerm({ read: true, write: true }, async function openModeWrite(): Promise< const encoder = new TextEncoder(); const filename = tempDir + "hello.txt"; const data = encoder.encode("Hello world!\n"); - let file = await Deno.open(filename, "w"); // assert file was created let fileInfo = Deno.statSync(filename); diff --git a/cli/js/lib.deno_runtime.d.ts b/cli/js/lib.deno_runtime.d.ts index 05553ffb7f..c867213ee9 100644 --- a/cli/js/lib.deno_runtime.d.ts +++ b/cli/js/lib.deno_runtime.d.ts @@ -312,7 +312,16 @@ declare namespace Deno { /** Open a file and return an instance of the `File` object * synchronously. * - * const file = Deno.openSync("/foo/bar.txt"); + * const file = Deno.openSYNC("/foo/bar.txt", { read: true, write: true }); + * + * Requires allow-read or allow-write or both depending on mode. + */ + export function openSync(filename: string, options?: OpenOptions): File; + + /** Open a file and return an instance of the `File` object + * synchronously. + * + * const file = Deno.openSync("/foo/bar.txt", "r"); * * Requires allow-read or allow-write or both depending on mode. */ @@ -320,7 +329,15 @@ declare namespace Deno { /** Open a file and return an instance of the `File` object. * - * const file = await Deno.open("/foo/bar.txt"); + * const file = await Deno.open("/foo/bar.txt", { read: true, write: true }); + * + * Requires allow-read or allow-write or both depending on mode. + */ + export function open(filename: string, options?: OpenOptions): Promise; + + /** Open a file and return an instance of the `File` object. + * + * const file = await Deno.open("/foo/bar.txt, "w+"); * * Requires allow-read or allow-write or both depending on mode. */ @@ -439,8 +456,38 @@ declare namespace Deno { /** An instance of `File` for stderr. */ export const stderr: File; - /** UNSTABLE: merge https://github.com/denoland/deno/pull/3119 */ + export interface OpenOptions { + /** Sets the option for read access. This option, when true, will indicate that the file should be read-able if opened. */ + read?: boolean; + /** Sets the option for write access. + * This option, when true, will indicate that the file should be write-able if opened. + * If the file already exists, any write calls on it will overwrite its contents, without truncating it. + */ + write?: boolean; + /* Sets the option for creating a new file. + * This option indicates whether a new file will be created if the file does not yet already exist. + * In order for the file to be created, write or append access must be used. + */ + create?: boolean; + /** Sets the option for truncating a previous file. + * If a file is successfully opened with this option set it will truncate the file to 0 length if it already exists. + * The file must be opened with write access for truncate to work. + */ + truncate?: boolean; + /**Sets the option for the append mode. + * This option, when true, means that writes will append to a file instead of overwriting previous contents. + * Note that setting { write: true, append: true } has the same effect as setting only { append: true }. + */ + append?: boolean; + /** Sets the option to always create a new file. + * This option indicates whether a new file will be created. No file is allowed to exist at the target location, also no (dangling) symlink. + * If { createNew: true } is set, create and truncate are ignored. + */ + createNew?: boolean; + } + export type OpenMode = + /** Read-only. Default. Starts at beginning of file. */ | "r" /** Read-write. Start at beginning of file. */ | "r+" diff --git a/cli/ops/files.rs b/cli/ops/files.rs index 8bb3c8acb9..3c17a607e8 100644 --- a/cli/ops/files.rs +++ b/cli/ops/files.rs @@ -26,7 +26,20 @@ pub fn init(i: &mut Isolate, s: &ThreadSafeState) { struct OpenArgs { promise_id: Option, filename: String, - mode: String, + options: Option, + mode: Option, +} + +#[derive(Deserialize, Default, Debug)] +#[serde(rename_all = "camelCase")] +#[serde(default)] +struct OpenOptions { + read: bool, + write: bool, + create: bool, + truncate: bool, + append: bool, + create_new: bool, } fn op_open( @@ -36,56 +49,82 @@ fn op_open( ) -> Result { let args: OpenArgs = serde_json::from_value(args)?; let filename = deno_fs::resolve_from_cwd(Path::new(&args.filename))?; - let mode = args.mode.as_ref(); let state_ = state.clone(); let mut open_options = tokio::fs::OpenOptions::new(); - match mode { - "r" => { - open_options.read(true); + if let Some(options) = args.options { + if options.read { + state.check_read(&filename)?; } - "r+" => { - open_options.read(true).write(true); - } - "w" => { - open_options.create(true).write(true).truncate(true); - } - "w+" => { - open_options - .read(true) - .create(true) - .write(true) - .truncate(true); - } - "a" => { - open_options.create(true).append(true); - } - "a+" => { - open_options.read(true).create(true).append(true); - } - "x" => { - open_options.create_new(true).write(true); - } - "x+" => { - open_options.create_new(true).read(true).write(true); - } - &_ => { - panic!("Unknown file open mode."); - } - } - match mode { - "r" => { - state.check_read(&filename)?; - } - "w" | "a" | "x" => { + if options.write || options.append { state.check_write(&filename)?; } - &_ => { - state.check_read(&filename)?; - state.check_write(&filename)?; + + open_options + .read(options.read) + .create(options.create) + .write(options.write) + .truncate(options.truncate) + .append(options.append) + .create_new(options.create_new); + } else if let Some(mode) = args.mode { + let mode = mode.as_ref(); + match mode { + "r" => { + state.check_read(&filename)?; + } + "w" | "a" | "x" => { + state.check_write(&filename)?; + } + &_ => { + state.check_read(&filename)?; + state.check_write(&filename)?; + } + }; + + match mode { + "r" => { + open_options.read(true); + } + "r+" => { + open_options.read(true).write(true); + } + "w" => { + open_options.create(true).write(true).truncate(true); + } + "w+" => { + open_options + .read(true) + .create(true) + .write(true) + .truncate(true); + } + "a" => { + open_options.create(true).append(true); + } + "a+" => { + open_options.read(true).create(true).append(true); + } + "x" => { + open_options.create_new(true).write(true); + } + "x+" => { + open_options.create_new(true).read(true).write(true); + } + &_ => { + return Err(ErrBox::from(DenoError::new( + ErrorKind::Other, + "Unknown open mode.".to_string(), + ))); + } } - } + } else { + return Err(ErrBox::from(DenoError::new( + ErrorKind::Other, + "Open requires either mode or options.".to_string(), + ))); + }; let is_sync = args.promise_id.is_none();