From 4a8d25646aa58e3e59d622e69c41822b40415c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 25 Apr 2020 00:45:55 +0200 Subject: [PATCH] BREAKING CHANGE: remove Deno.OpenMode (#4884) This commit removes Deno.OpenMode along with overloaded variants of Deno.open() and Deno.openSync() that used OpenMode. --- cli/js/deno.ts | 1 - cli/js/files.ts | 55 ++++++++--------------- cli/js/lib.deno.ns.d.ts | 39 ---------------- cli/js/ops/fs/open.ts | 17 ++----- cli/js/tests/files_test.ts | 54 +++++++++++++--------- cli/js/tests/process_test.ts | 7 ++- cli/js/write_file.ts | 12 +++-- cli/ops/fs.rs | 85 +++++++---------------------------- cli/tests/044_bad_resource.ts | 2 +- std/archive/tar.ts | 6 +-- std/fs/walk_test.ts | 4 +- std/io/util.ts | 7 ++- std/log/handlers.ts | 18 ++++++-- std/mime/multipart_test.ts | 4 +- 14 files changed, 113 insertions(+), 198 deletions(-) diff --git a/cli/js/deno.ts b/cli/js/deno.ts index fd53b58e7e..71d0fdf997 100644 --- a/cli/js/deno.ts +++ b/cli/js/deno.ts @@ -35,7 +35,6 @@ export { seek, seekSync, OpenOptions, - OpenMode, } from "./files.ts"; export { read, readSync, write, writeSync } from "./ops/io.ts"; export { FsEvent, watchFs } from "./ops/fs_events.ts"; diff --git a/cli/js/files.ts b/cli/js/files.ts index d09fcf8db5..97a8c1e2b1 100644 --- a/cli/js/files.ts +++ b/cli/js/files.ts @@ -18,60 +18,43 @@ import { open as opOpen, openSync as opOpenSync, OpenOptions, - OpenMode, } from "./ops/fs/open.ts"; -export { OpenOptions, OpenMode } from "./ops/fs/open.ts"; +export { OpenOptions } from "./ops/fs/open.ts"; -export function openSync(path: string, options?: OpenOptions): File; -export function openSync(path: string, openMode?: OpenMode): File; - -/**@internal*/ export function openSync( path: string, - modeOrOptions: OpenOptions | OpenMode = "r" + options: OpenOptions = { read: true } ): File { - let openMode = undefined; - let options = undefined; - - if (typeof modeOrOptions === "string") { - openMode = modeOrOptions; - } else { - checkOpenOptions(modeOrOptions); - options = modeOrOptions as OpenOptions; - } - - const rid = opOpenSync(path, openMode as OpenMode, options); + checkOpenOptions(options); + const rid = opOpenSync(path, options); return new File(rid); } -export async function open(path: string, options?: OpenOptions): Promise; -export async function open(path: string, openMode?: OpenMode): Promise; - -/**@internal*/ export async function open( path: string, - modeOrOptions: OpenOptions | OpenMode = "r" + options: OpenOptions = { read: true } ): Promise { - let openMode = undefined; - let options = undefined; - - if (typeof modeOrOptions === "string") { - openMode = modeOrOptions; - } else { - checkOpenOptions(modeOrOptions); - options = modeOrOptions as OpenOptions; - } - - const rid = await opOpen(path, openMode as OpenMode, options); + checkOpenOptions(options); + const rid = await opOpen(path, options); return new File(rid); } export function createSync(path: string): File { - return openSync(path, "w+"); + return openSync(path, { + read: true, + write: true, + truncate: true, + create: true, + }); } export function create(path: string): Promise { - return open(path, "w+"); + return open(path, { + read: true, + write: true, + truncate: true, + create: true, + }); } export class File diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index 7145751c72..84e287fca9 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -653,18 +653,6 @@ declare namespace Deno { */ export function openSync(path: string, options?: OpenOptions): File; - /** Synchronously open a file and return an instance of `Deno.File`. The file - * may be created depending on the mode passed in. It is the callers responsibility - * to close the file when finished with it. - * - * const file = Deno.openSync("/foo/bar.txt", "r"); - * // Do work with file - * Deno.close(file.rid); - * - * Requires `allow-read` and/or `allow-write` permissions depending on openMode. - */ - export function openSync(path: string, openMode?: OpenMode): File; - /** Open a file and resolve to an instance of `Deno.File`. The * file does not need to previously exist if using the `create` or `createNew` * open options. It is the callers responsibility to close the file when finished @@ -678,18 +666,6 @@ declare namespace Deno { */ export function open(path: string, options?: OpenOptions): Promise; - /** Open a file and resolve to an instance of `Deno.File`. The file may be - * created depending on the mode passed in. It is the callers responsibility - * to close the file when finished with it. - * - * const file = await Deno.open("/foo/bar.txt", "w+"); - * // Do work with file - * Deno.close(file.rid); - * - * Requires `allow-read` and/or `allow-write` permissions depending on openMode. - */ - export function open(path: string, openMode?: OpenMode): Promise; - /** Creates a file if none exists or truncates an existing file and returns * an instance of `Deno.File`. * @@ -890,21 +866,6 @@ declare namespace Deno { mode?: number; } - /** A set of string literals which specify how to open a file. - * - * |Value |Description | - * |------|--------------------------------------------------------------------------------------------------| - * |`"r"` |Read-only. Default. Starts at beginning of file. | - * |`"r+"`|Read-write. Start at beginning of file. | - * |`"w"` |Write-only. Opens and truncates existing file or creates new one for writing only. | - * |`"w+"`|Read-write. Opens and truncates existing file or creates new one for writing and reading. | - * |`"a"` |Write-only. Opens existing file or creates new one. Each write appends content to the end of file.| - * |`"a+"`|Read-write. Behaves like `"a"` and allows to read from file. | - * |`"x"` |Write-only. Exclusive create - creates new file only if one doesn't exist already. | - * |`"x+"`|Read-write. Behaves like `x` and allows reading from file. | - */ - export type OpenMode = "r" | "r+" | "w" | "w+" | "a" | "a+" | "x" | "x+"; - /** **UNSTABLE**: new API, yet to be vetted * * Check if a given resource id (`rid`) is a TTY. diff --git a/cli/js/ops/fs/open.ts b/cli/js/ops/fs/open.ts index b587f491dd..afe713db82 100644 --- a/cli/js/ops/fs/open.ts +++ b/cli/js/ops/fs/open.ts @@ -15,27 +15,16 @@ export interface OpenOptions { mode?: number; } -export type OpenMode = "r" | "r+" | "w" | "w+" | "a" | "a+" | "x" | "x+"; - -export function openSync( - path: string, - openMode: OpenMode | undefined, - options: OpenOptions | undefined -): number { +export function openSync(path: string, options: OpenOptions): number { const mode: number | undefined = options?.mode; - return sendSync("op_open", { path, options, openMode, mode }); + return sendSync("op_open", { path, options, mode }); } -export function open( - path: string, - openMode: OpenMode | undefined, - options: OpenOptions | undefined -): Promise { +export function open(path: string, options: OpenOptions): Promise { const mode: number | undefined = options?.mode; return sendAsync("op_open", { path, options, - openMode, mode, }); } diff --git a/cli/js/tests/files_test.ts b/cli/js/tests/files_test.ts index 11a2e08ae1..39f4d6ce26 100644 --- a/cli/js/tests/files_test.ts +++ b/cli/js/tests/files_test.ts @@ -201,11 +201,11 @@ unitTest( { perms: { write: false } }, async function writePermFailure(): Promise { const filename = "tests/hello.txt"; - const writeModes: Deno.OpenMode[] = ["w", "a", "x"]; - for (const mode of writeModes) { + const openOptions: Deno.OpenOptions[] = [{ write: true }, { append: true }]; + for (const options of openOptions) { let err; try { - await Deno.open(filename, mode); + await Deno.open(filename, options); } catch (e) { err = e; } @@ -266,8 +266,8 @@ unitTest({ perms: { read: false } }, async function readPermFailure(): Promise< > { let caughtError = false; try { - await Deno.open("package.json", "r"); - await Deno.open("cli/tests/fixture.json", "r"); + await Deno.open("package.json", { read: true }); + await Deno.open("cli/tests/fixture.json", { read: true }); } catch (e) { caughtError = true; assert(e instanceof Deno.errors.PermissionDenied); @@ -308,7 +308,12 @@ unitTest( async function readNullBufferFailure(): Promise { const tempDir = Deno.makeTempDirSync(); const filename = tempDir + "hello.txt"; - const file = await Deno.open(filename, "w+"); + const file = await Deno.open(filename, { + read: true, + write: true, + truncate: true, + create: true, + }); // reading into an empty buffer should return 0 immediately const bytesRead = await file.read(new Uint8Array(0)); @@ -334,18 +339,15 @@ unitTest( { perms: { write: false, read: false } }, async function readWritePermFailure(): Promise { const filename = "tests/hello.txt"; - const writeModes: Deno.OpenMode[] = ["r+", "w+", "a+", "x+"]; - for (const mode of writeModes) { - let err; - try { - await Deno.open(filename, mode); - } catch (e) { - err = e; - } - assert(!!err); - assert(err instanceof Deno.errors.PermissionDenied); - assertEquals(err.name, "PermissionDenied"); + let err; + try { + await Deno.open(filename, { read: true }); + } catch (e) { + err = e; } + assert(!!err); + assert(err instanceof Deno.errors.PermissionDenied); + assertEquals(err.name, "PermissionDenied"); } ); @@ -377,7 +379,11 @@ unitTest( const encoder = new TextEncoder(); const filename = tempDir + "hello.txt"; const data = encoder.encode("Hello world!\n"); - let file = await Deno.open(filename, "w"); + let file = await Deno.open(filename, { + create: true, + write: true, + truncate: true, + }); // assert file was created let fileInfo = Deno.statSync(filename); assert(fileInfo.isFile); @@ -398,7 +404,10 @@ unitTest( } file.close(); // assert that existing file is truncated on open - file = await Deno.open(filename, "w"); + file = await Deno.open(filename, { + write: true, + truncate: true, + }); file.close(); const fileSize = Deno.statSync(filename).size; assertEquals(fileSize, 0); @@ -414,7 +423,12 @@ unitTest( const filename = tempDir + "hello.txt"; const data = encoder.encode("Hello world!\n"); - const file = await Deno.open(filename, "w+"); + const file = await Deno.open(filename, { + write: true, + truncate: true, + create: true, + read: true, + }); const seekPosition = 0; // assert file was created let fileInfo = Deno.statSync(filename); diff --git a/cli/js/tests/process_test.ts b/cli/js/tests/process_test.ts index 47efd86d5a..1bcdf26da8 100644 --- a/cli/js/tests/process_test.ts +++ b/cli/js/tests/process_test.ts @@ -229,7 +229,10 @@ unitTest( async function runRedirectStdoutStderr(): Promise { const tempDir = await makeTempDir(); const fileName = tempDir + "/redirected_stdio.txt"; - const file = await open(fileName, "w"); + const file = await open(fileName, { + create: true, + write: true, + }); const p = run({ cmd: [ @@ -261,7 +264,7 @@ unitTest( const fileName = tempDir + "/redirected_stdio.txt"; const encoder = new TextEncoder(); await writeFile(fileName, encoder.encode("hello")); - const file = await open(fileName, "r"); + const file = await open(fileName); const p = run({ cmd: ["python", "-c", "import sys; assert 'hello' == sys.stdin.read();"], diff --git a/cli/js/write_file.ts b/cli/js/write_file.ts index ed64141d2d..af3f672514 100644 --- a/cli/js/write_file.ts +++ b/cli/js/write_file.ts @@ -24,8 +24,10 @@ export function writeFileSync( } } - const openMode = !!options.append ? "a" : "w"; - const file = openSync(path, openMode); + const openOptions = !!options.append + ? { write: true, create: true, append: true } + : { write: true, create: true, truncate: true }; + const file = openSync(path, openOptions); if ( options.mode !== undefined && @@ -52,8 +54,10 @@ export async function writeFile( } } - const openMode = !!options.append ? "a" : "w"; - const file = await open(path, openMode); + const openOptions = !!options.append + ? { write: true, create: true, append: true } + : { write: true, create: true, truncate: true }; + const file = await open(path, openOptions); if ( options.mode !== undefined && diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs index a7f7fbf087..6b259e033a 100644 --- a/cli/ops/fs.rs +++ b/cli/ops/fs.rs @@ -50,8 +50,7 @@ fn into_string(s: std::ffi::OsString) -> Result { struct OpenArgs { promise_id: Option, path: String, - options: Option, - open_mode: Option, + options: OpenOptions, mode: Option, } @@ -91,76 +90,22 @@ fn op_open( let _ = mode; // avoid unused warning } - if let Some(options) = args.options { - if options.read { - state.check_read(&path)?; - } + let options = args.options; + if options.read { + state.check_read(&path)?; + } - if options.write || options.append { - state.check_write(&path)?; - } + if options.write || options.append { + state.check_write(&path)?; + } - 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(open_mode) = args.open_mode { - let open_mode = open_mode.as_ref(); - match open_mode { - "r" => { - state.check_read(&path)?; - } - "w" | "a" | "x" => { - state.check_write(&path)?; - } - &_ => { - state.check_read(&path)?; - state.check_write(&path)?; - } - }; - - match open_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); - } - &_ => { - // TODO: this should be type error - return Err(OpError::other("Unknown open mode.".to_string())); - } - } - } else { - return Err(OpError::other( - "Open requires either openMode or options.".to_string(), - )); - }; + open_options + .read(options.read) + .create(options.create) + .write(options.write) + .truncate(options.truncate) + .append(options.append) + .create_new(options.create_new); let is_sync = args.promise_id.is_none(); diff --git a/cli/tests/044_bad_resource.ts b/cli/tests/044_bad_resource.ts index 39ca3d1204..03da1fdc96 100644 --- a/cli/tests/044_bad_resource.ts +++ b/cli/tests/044_bad_resource.ts @@ -1,5 +1,5 @@ async function main(): Promise { - const file = await Deno.open("044_bad_resource.ts", "r"); + const file = await Deno.open("044_bad_resource.ts", { read: true }); file.close(); await file.seek(10, 0); } diff --git a/std/archive/tar.ts b/std/archive/tar.ts index 6bc2b92d0b..699b982a92 100644 --- a/std/archive/tar.ts +++ b/std/archive/tar.ts @@ -39,15 +39,15 @@ const ustar = "ustar\u000000"; class FileReader implements Deno.Reader { private file?: Deno.File; - constructor(private filePath: string, private mode: Deno.OpenMode = "r") {} + constructor(private filePath: string) {} public async read(p: Uint8Array): Promise { if (!this.file) { - this.file = await Deno.open(this.filePath, this.mode); + this.file = await Deno.open(this.filePath, { read: true }); } const res = await Deno.read(this.file.rid, p); if (res === Deno.EOF) { - await Deno.close(this.file.rid); + Deno.close(this.file.rid); this.file = undefined; } return res; diff --git a/std/fs/walk_test.ts b/std/fs/walk_test.ts index 6a44f5514f..b054db4cf0 100644 --- a/std/fs/walk_test.ts +++ b/std/fs/walk_test.ts @@ -1,4 +1,4 @@ -const { cwd, chdir, makeTempDir, mkdir, open, symlink } = Deno; +const { cwd, chdir, makeTempDir, mkdir, create, symlink } = Deno; const { remove } = Deno; import { walk, walkSync, WalkOptions, WalkEntry } from "./walk.ts"; import { assert, assertEquals, assertThrowsAsync } from "../testing/asserts.ts"; @@ -46,7 +46,7 @@ export async function walkArray( } export async function touch(path: string): Promise { - const f = await open(path, "w"); + const f = await create(path); f.close(); } diff --git a/std/io/util.ts b/std/io/util.ts index 28688ae915..18ddb4def3 100644 --- a/std/io/util.ts +++ b/std/io/util.ts @@ -47,6 +47,11 @@ export async function tempFile( `${dir}/${opts.prefix || ""}${r}${opts.postfix || ""}` ); await mkdir(path.dirname(filepath), { recursive: true }); - const file = await open(filepath, "a"); + const file = await open(filepath, { + create: true, + read: true, + write: true, + append: true, + }); return { file, filepath }; } diff --git a/std/log/handlers.ts b/std/log/handlers.ts index eea7f32ffb..041f101ed6 100644 --- a/std/log/handlers.ts +++ b/std/log/handlers.ts @@ -2,6 +2,7 @@ const { open, openSync, close, renameSync, statSync } = Deno; type File = Deno.File; type Writer = Deno.Writer; +type OpenOptions = Deno.OpenOptions; import { getLevelByName, LogLevel } from "./levels.ts"; import { LogRecord } from "./logger.ts"; import { red, yellow, blue, bold } from "../fmt/colors.ts"; @@ -101,6 +102,7 @@ export class FileHandler extends WriterHandler { protected _file!: File; protected _filename: string; protected _mode: LogMode; + protected _openOptions: OpenOptions; #encoder = new TextEncoder(); constructor(levelName: string, options: FileHandlerOptions) { @@ -108,10 +110,17 @@ export class FileHandler extends WriterHandler { this._filename = options.filename; // default to append mode, write only this._mode = options.mode ? options.mode : "a"; + this._openOptions = { + createNew: this._mode === "x", + create: this._mode !== "x", + append: this._mode === "a", + truncate: this._mode !== "a", + write: true, + }; } async setup(): Promise { - this._file = await open(this._filename, this._mode); + this._file = await open(this._filename, this._openOptions); this._writer = this._file; } @@ -119,8 +128,9 @@ export class FileHandler extends WriterHandler { Deno.writeSync(this._file.rid, this.#encoder.encode(msg + "\n")); } - async destroy(): Promise { - await this._file.close(); + destroy(): Promise { + this._file.close(); + return Promise.resolve(); } } @@ -193,7 +203,7 @@ export class RotatingFileHandler extends FileHandler { } } - this._file = openSync(this._filename, this._mode); + this._file = openSync(this._filename, this._openOptions); this._writer = this._file; } } diff --git a/std/mime/multipart_test.ts b/std/mime/multipart_test.ts index aa3b7524eb..c0cf012ec7 100644 --- a/std/mime/multipart_test.ts +++ b/std/mime/multipart_test.ts @@ -93,7 +93,9 @@ test(async function multipartMultipartWriter(): Promise { const mw = new MultipartWriter(buf); await mw.writeField("foo", "foo"); await mw.writeField("bar", "bar"); - const f = await open(path.resolve("./mime/testdata/sample.txt"), "r"); + const f = await open(path.resolve("./mime/testdata/sample.txt"), { + read: true, + }); await mw.writeFile("file", "sample.txt", f); await mw.close(); f.close();