From 00944c7cc25f391c3fbeba1e054a56a62cf0de12 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 19 Dec 2018 14:14:57 -0800 Subject: [PATCH] src: use consistent names for JSStream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its confusing to call a js class with a handle a "Wrap", usually it's the C++ handle that is called a Wrap (tcp_wrap, tls_wrap, ...). Its derived from Socket, and makes a JS stream look like a Socket, so call it that. Also, remove use of lib/_stream_wrap.js so it can be deprecated some time. PR-URL: https://github.com/nodejs/node/pull/25153 Reviewed-By: Anna Henningsen Reviewed-By: Tobias Nießen Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski --- lib/_stream_wrap.js | 2 +- lib/_tls_wrap.js | 8 +++++--- lib/internal/http2/core.js | 4 ++-- .../{wrap_js_stream.js => js_stream_socket.js} | 15 ++++++++------- node.gyp | 2 +- test/parallel/test-stream-wrap-drain.js | 4 ++-- test/parallel/test-stream-wrap-encoding.js | 3 ++- test/parallel/test-stream-wrap.js | 2 +- test/parallel/test-wrap-js-stream-destroy.js | 3 ++- test/parallel/test-wrap-js-stream-duplex.js | 3 ++- test/parallel/test-wrap-js-stream-exceptions.js | 2 +- test/parallel/test-wrap-js-stream-read-stop.js | 2 +- 12 files changed, 28 insertions(+), 22 deletions(-) rename lib/internal/{wrap_js_stream.js => js_stream_socket.js} (94%) diff --git a/lib/_stream_wrap.js b/lib/_stream_wrap.js index 10a0cf57e7789e..5b5f476948b23a 100644 --- a/lib/_stream_wrap.js +++ b/lib/_stream_wrap.js @@ -1,3 +1,3 @@ 'use strict'; -module.exports = require('internal/wrap_js_stream'); +module.exports = require('internal/js_stream_socket'); diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index b07812f1a66fea..a72091b22e97fb 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -29,7 +29,7 @@ const net = require('net'); const tls = require('tls'); const util = require('util'); const common = require('_tls_common'); -const { StreamWrap } = require('_stream_wrap'); +const JSStreamSocket = require('internal/js_stream_socket'); const { Buffer } = require('buffer'); const debug = util.debuglog('tls'); const { TCP, constants: TCPConstants } = internalBinding('tcp_wrap'); @@ -310,12 +310,14 @@ function TLSSocket(socket, opts) { this.authorizationError = null; this[kRes] = null; - // Wrap plain JS Stream into StreamWrap var wrap; if ((socket instanceof net.Socket && socket._handle) || !socket) { wrap = socket; } else { - wrap = new StreamWrap(socket); + // TLS expects to interact from C++ with a net.Socket that has a C++ stream + // handle, but a JS stream doesn't have one. Wrap it up to make it look like + // a socket. + wrap = new JSStreamSocket(socket); wrap.once('close', () => this.destroy()); } diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index f33ec48ff23087..2bca31e24a5473 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -22,7 +22,7 @@ const util = require('util'); const { kIncomingMessage } = require('_http_common'); const { kServerResponse } = require('_http_server'); -const { StreamWrap } = require('_stream_wrap'); +const JSStreamSocket = require('internal/js_stream_socket'); const { defaultTriggerAsyncIdScope, @@ -935,7 +935,7 @@ class Http2Session extends EventEmitter { super(); if (!socket._handle || !socket._handle._externalStream) { - socket = new StreamWrap(socket); + socket = new JSStreamSocket(socket); } // No validation is performed on the input parameters because this diff --git a/lib/internal/wrap_js_stream.js b/lib/internal/js_stream_socket.js similarity index 94% rename from lib/internal/wrap_js_stream.js rename to lib/internal/js_stream_socket.js index cf8f45aa4505ff..8343b6c2645842 100644 --- a/lib/internal/wrap_js_stream.js +++ b/lib/internal/js_stream_socket.js @@ -5,7 +5,7 @@ const util = require('util'); const { Socket } = require('net'); const { JSStream } = internalBinding('js_stream'); const uv = internalBinding('uv'); -const debug = util.debuglog('stream_wrap'); +const debug = util.debuglog('stream_socket'); const { owner_symbol } = require('internal/async_hooks').symbols; const { ERR_STREAM_WRAP } = require('internal/errors').codes; @@ -29,9 +29,9 @@ function onwrite(req, bufs) { return this[owner_symbol].doWrite(req, bufs); } * can skip going through the JS layer and let TLS access the raw C++ handle * of a net.Socket. The flipside of this is that, to maintain composability, * we need a way to create "fake" net.Socket instances that call back into a - * "real" JavaScript stream. JSStreamWrap is exactly this. + * "real" JavaScript stream. JSStreamSocket is exactly this. */ -class JSStreamWrap extends Socket { +class JSStreamSocket extends Socket { constructor(stream) { const handle = new JSStream(); handle.close = (cb) => { @@ -39,7 +39,7 @@ class JSStreamWrap extends Socket { this.doClose(cb); }; // Inside of the following functions, `this` refers to the handle - // and `this[owner_symbol]` refers to this JSStreamWrap instance. + // and `this[owner_symbol]` refers to this JSStreamSocket instance. handle.isClosing = isClosing; handle.onreadstart = onreadstart; handle.onreadstop = onreadstop; @@ -88,9 +88,10 @@ class JSStreamWrap extends Socket { this.read(0); } - // Legacy + // Allow legacy requires in the test suite to keep working: + // const { StreamWrap } = require('internal/js_stream_socket') static get StreamWrap() { - return JSStreamWrap; + return JSStreamSocket; } isClosing() { @@ -223,4 +224,4 @@ class JSStreamWrap extends Socket { } } -module.exports = JSStreamWrap; +module.exports = JSStreamSocket; diff --git a/node.gyp b/node.gyp index 8c42ef446e70b6..e36f9f26aa2f06 100644 --- a/node.gyp +++ b/node.gyp @@ -126,6 +126,7 @@ 'lib/internal/fs/watchers.js', 'lib/internal/http.js', 'lib/internal/inspector_async_hook.js', + 'lib/internal/js_stream_socket.js', 'lib/internal/linkedlist.js', 'lib/internal/modules/cjs/helpers.js', 'lib/internal/modules/cjs/loader.js', @@ -188,7 +189,6 @@ 'lib/internal/streams/state.js', 'lib/internal/streams/pipeline.js', 'lib/internal/streams/end-of-stream.js', - 'lib/internal/wrap_js_stream.js', 'deps/v8/tools/splaytree.js', 'deps/v8/tools/codemap.js', 'deps/v8/tools/consarray.js', diff --git a/test/parallel/test-stream-wrap-drain.js b/test/parallel/test-stream-wrap-drain.js index 068e2d7fd45979..77243399515730 100644 --- a/test/parallel/test-stream-wrap-drain.js +++ b/test/parallel/test-stream-wrap-drain.js @@ -2,12 +2,12 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const { StreamWrap } = require('_stream_wrap'); +const { StreamWrap } = require('internal/js_stream_socket'); const { Duplex } = require('stream'); const { internalBinding } = require('internal/test/binding'); const { ShutdownWrap } = internalBinding('stream_wrap'); -// This test makes sure that when an instance of JSStreamWrap is waiting for +// This test makes sure that when a wrapped stream is waiting for // a "drain" event to `doShutdown`, the instance will work correctly when a // "drain" event emitted. { diff --git a/test/parallel/test-stream-wrap-encoding.js b/test/parallel/test-stream-wrap-encoding.js index ce6f95fa27d68f..72804d77d6bde6 100644 --- a/test/parallel/test-stream-wrap-encoding.js +++ b/test/parallel/test-stream-wrap-encoding.js @@ -1,7 +1,8 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); -const StreamWrap = require('_stream_wrap'); +const StreamWrap = require('internal/js_stream_socket'); const Duplex = require('stream').Duplex; { diff --git a/test/parallel/test-stream-wrap.js b/test/parallel/test-stream-wrap.js index 9a279790d8aceb..670c05fe3f0307 100644 --- a/test/parallel/test-stream-wrap.js +++ b/test/parallel/test-stream-wrap.js @@ -4,7 +4,7 @@ const common = require('../common'); const assert = require('assert'); const { internalBinding } = require('internal/test/binding'); -const StreamWrap = require('_stream_wrap'); +const StreamWrap = require('internal/js_stream_socket'); const { Duplex } = require('stream'); const { ShutdownWrap } = internalBinding('stream_wrap'); diff --git a/test/parallel/test-wrap-js-stream-destroy.js b/test/parallel/test-wrap-js-stream-destroy.js index 16d3e75e2ca7e3..5c1ed1e7e65d10 100644 --- a/test/parallel/test-wrap-js-stream-destroy.js +++ b/test/parallel/test-wrap-js-stream-destroy.js @@ -1,7 +1,8 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); -const StreamWrap = require('_stream_wrap'); +const StreamWrap = require('internal/js_stream_socket'); const net = require('net'); // This test ensures that when we directly call `socket.destroy()` without diff --git a/test/parallel/test-wrap-js-stream-duplex.js b/test/parallel/test-wrap-js-stream-duplex.js index 6bd860e6ba1f56..6a817c1054cf55 100644 --- a/test/parallel/test-wrap-js-stream-duplex.js +++ b/test/parallel/test-wrap-js-stream-duplex.js @@ -1,7 +1,8 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const StreamWrap = require('_stream_wrap'); +const StreamWrap = require('internal/js_stream_socket'); const { PassThrough } = require('stream'); const { Socket } = require('net'); diff --git a/test/parallel/test-wrap-js-stream-exceptions.js b/test/parallel/test-wrap-js-stream-exceptions.js index 57ecd70189d106..cde7c178446a11 100644 --- a/test/parallel/test-wrap-js-stream-exceptions.js +++ b/test/parallel/test-wrap-js-stream-exceptions.js @@ -2,7 +2,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const JSStreamWrap = require('internal/wrap_js_stream'); +const JSStreamWrap = require('internal/js_stream_socket'); const { Duplex } = require('stream'); process.once('uncaughtException', common.mustCall((err) => { diff --git a/test/parallel/test-wrap-js-stream-read-stop.js b/test/parallel/test-wrap-js-stream-read-stop.js index f51b3ecf524e30..6d86dc2c21a519 100644 --- a/test/parallel/test-wrap-js-stream-read-stop.js +++ b/test/parallel/test-wrap-js-stream-read-stop.js @@ -3,7 +3,7 @@ require('../common'); const assert = require('assert'); -const WrapStream = require('internal/wrap_js_stream'); +const WrapStream = require('internal/js_stream_socket'); const Stream = require('stream'); class FakeStream extends Stream {