Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove @types node dev dependency from botframework-streaming #1795

Merged
merged 34 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
4cf3fe3
replaced new Server with createNodeServer(); created test INodeSocket…
Zerryth Feb 26, 2020
7231d2b
removed new Func unit test
Zerryth Feb 26, 2020
0fd229b
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 26, 2020
ecd41bc
consolidated server-creation into 1 utils module; added unit tests
Zerryth Feb 26, 2020
f116243
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 26, 2020
e717537
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 26, 2020
a4b8f24
removed await from Promise.all, as it was originally
Zerryth Feb 26, 2020
271262c
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 26, 2020
805b3b5
Update libraries/botframework-streaming/tests/NamedPipe.test.js
Zerryth Feb 26, 2020
dc89d8c
replaced references to @types/node classes with bf-streaming interfac…
Zerryth Feb 27, 2020
ef932ea
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 27, 2020
f8be823
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 27, 2020
45b7292
added more signatures to INodeSocket to remove Socket type dependency…
Zerryth Feb 27, 2020
3bd6815
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 27, 2020
43847f5
cleaned up comment
Zerryth Feb 27, 2020
eba33a5
changed INodeSocket.connect() to return any, as the nested types on s…
Zerryth Feb 27, 2020
91a5bca
changed Buffer to INodeBuffer type hint
Zerryth Feb 27, 2020
31e1074
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 27, 2020
820e8ae
added type guards to ensure we're getting an INodeServer back from cr…
Zerryth Feb 27, 2020
66132ef
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 27, 2020
a40e263
added unit test
Zerryth Feb 27, 2020
9d25bd8
added duck typing checks for server methods
Zerryth Feb 27, 2020
1e852bb
Added methods to INodeSocket to remove dependency on node stream.Dupl…
Zerryth Feb 27, 2020
48e2ad4
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 27, 2020
9c0f058
put server-creation in trycatch block, as we don't know if node 'net'…
Zerryth Feb 28, 2020
0141a89
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 28, 2020
2ce87d8
removed space from top of imports in tests
Zerryth Feb 28, 2020
40c021c
added false path to checks (whoops, forgot them)
Zerryth Feb 28, 2020
66fdb43
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 28, 2020
db5ebc9
moved imort below comment
Zerryth Feb 28, 2020
1c854b0
Merge branch 'Zerryth/removeTypesNodeDevDependency' of https://github…
Zerryth Feb 28, 2020
c271100
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Feb 28, 2020
41fad65
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Zerryth Mar 3, 2020
dd57eee
Merge branch 'master' into Zerryth/removeTypesNodeDevDependency
Stevenic Mar 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libraries/botframework-streaming/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export {
INodeBuffer,
INodeIncomingMessage,
INodeSocket,
INodeSocket2,
IReceiveRequest,
IReceiveResponse,
ISocket,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* This interface supports the framework and is not intended to be called directly for your code.
*/
export interface INodeSocket {
connecting: boolean;
writable: boolean;
write(str: string, cb?: Function): boolean;
destroy(error?: Error): void;
Expand Down
31 changes: 31 additions & 0 deletions libraries/botframework-streaming/src/interfaces/INodeSocket2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @module botframework-streaming
*/
/**
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

/**
* Represents a Socket from the `net` module in Node.js.
*
* This interface supports the framework and is not intended to be called directly for your code.
*/
export interface INodeSocket2 {
connecting: boolean;
destroyed: boolean;
writable: boolean;

end(str: string, cb?: Function): void;
destroy(error?: Error): void;

on(event: string, listener: (...args: any[]) => void): this;
on(event: "close", listener: (had_error: boolean) => void): this;
on(event: "connect", listener: () => void): this;
on(event: "data", listener: (data: Buffer) => void): this;
on(event: "end", listener: () => void): this;
on(event: "error", listener: (err: Error) => void): this;

write(buffer: Buffer, cb?: Function): boolean;
write(str: string, cb?: Function): boolean;
}
1 change: 1 addition & 0 deletions libraries/botframework-streaming/src/interfaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export * from './IBrowserWebSocket';
export * from './INodeBuffer';
export * from './INodeIncomingMessage';
export * from './INodeSocket';
export * from './INodeSocket2';
export * from './IReceiveRequest';
export * from './IReceiveResponse';
export * from './ISocket';
Expand Down
20 changes: 11 additions & 9 deletions libraries/botframework-streaming/src/namedPipe/namedPipeServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { Server, Socket } from 'net';
// import { Socket, Server } from 'net';
import { ProtocolAdapter } from '../protocolAdapter';
import { RequestHandler } from '../requestHandler';
import { StreamingRequest } from '../streamingRequest';
Expand All @@ -15,14 +15,15 @@ import {
PayloadSender
} from '../payloadTransport';
import { NamedPipeTransport } from './namedPipeTransport';
import { IStreamingTransportServer, IReceiveResponse } from '../interfaces';
import { INodeSocket2, IStreamingTransportServer, IReceiveResponse } from '../interfaces';
import { createNodeServer } from '../utilities';

/**
* Streaming transport server implementation that uses named pipes for inter-process communication.
*/
export class NamedPipeServer implements IStreamingTransportServer {
private _outgoingServer: Server;
private _incomingServer: Server;
private _outgoingServer
private _incomingServer
private readonly _baseName: string;
private readonly _requestHandler: RequestHandler;
private readonly _sender: PayloadSender;
Expand Down Expand Up @@ -73,14 +74,15 @@ export class NamedPipeServer implements IStreamingTransportServer {
}

const incoming = new Promise(resolve => {
this._incomingServer = new Server((socket: Socket): void => {
this._receiver.connect(new NamedPipeTransport(socket));
resolve();
});
this._incomingServer = createNodeServer((socket: INodeSocket2): void => {
this._receiver.connect(new NamedPipeTransport(socket));
resolve();
});

});

const outgoing = new Promise(resolve => {
this._outgoingServer = new Server((socket: Socket): void => {
this._outgoingServer = createNodeServer((socket: INodeSocket2): void => {
this._sender.connect(new NamedPipeTransport(socket));
resolve();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { Socket } from 'net';
// import { Socket } from 'net';
import { INodeSocket2 } from '../interfaces/INodeSocket2';
import { ITransportSender } from '../interfaces/ITransportSender';
import { ITransportReceiver } from '../interfaces/ITransportReceiver';


// const testSocket = new Socket();
/**
* Named pipes based transport sender and receiver abstraction
*/
Expand All @@ -18,7 +19,7 @@ export class NamedPipeTransport implements ITransportSender, ITransportReceiver
public static readonly ServerIncomingPath: string = '.incoming';
public static readonly ServerOutgoingPath: string = '.outgoing';

private _socket: Socket;
private _socket: INodeSocket2;
private readonly _queue: Buffer[];
private _active: Buffer;
private _activeOffset: number;
Expand All @@ -31,7 +32,7 @@ export class NamedPipeTransport implements ITransportSender, ITransportReceiver
*
* @param socket The socket object to build this connection on.
*/
public constructor(socket: Socket) {
public constructor(socket: INodeSocket2) {
this._socket = socket;
this._queue = [];
this._activeOffset = 0;
Expand Down
30 changes: 30 additions & 0 deletions libraries/botframework-streaming/src/utilities/createNodeServer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { INodeSocket2 } from "../interfaces/INodeSocket2";

/**
* @module botframework-streaming
*/
/**
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

// What type hint should I put, if not importing 'net'?
export const createNodeServer = function(callback?: (socket: INodeSocket2) => void) {
if (callback && typeof callback !== 'function') {
throw new TypeError(`Invalid callback; callback parameter must be a function to create Node 'net' Server.`);
}

return getNetServerConstructor()(callback);
}

export const getNetServerConstructor = function() {
if (typeof require !== undefined) {
return require('net').Server;
}

throw TypeError(`require is undefined. Must be in a Node module to require 'net' dynamically in order to fetch Server constructor.`)
}

// require is NOT globally defined, therefore Function() cannot access require()
// https://stackoverflow.com/questions/51164425/require-inside-new-function
// export const getNetServerConstructor = new Function(`if (typeof require !== undefined) { return require('net').Server; } throw new Error('Not in a Node environment. Must use Node in order to create a Node Server.')`);
1 change: 1 addition & 0 deletions libraries/botframework-streaming/src/utilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@

export * from './doesGlobalFileReaderExist';
export * from './doesGlobalWebSocketExist';
export * from './createNodeServer';
export * from './protocol-base';
25 changes: 25 additions & 0 deletions libraries/botframework-streaming/tests/NamedPipe.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const net = require('net');
const np = require('../lib');
const npt = require('../lib/namedPipe/namedPipeTransport');
const NodeServerUtils = require('../lib/utilities/createNodeServer');
const protocol = require('../lib');
const chai = require('chai');
var expect = chai.expect;
Expand Down Expand Up @@ -414,5 +415,29 @@ describe('Streaming Extensions NamedPipe Library Tests', () => {
expect(server.disconnect()).to.not.throw;
done();
});


it('calling createNodeServer() should throw if passing in a callback that\'s not a function', () => {
const stringCallback = 'Not a real callback function.';
expect(() => NodeServerUtils.createNodeServer(stringCallback)).to.throw;
});

it('should not throw when choosing not to pass in a callback at all into createNodeServer()', () => {
expect(() => NodeServerUtils.createNodeServer()).to.not.throw;
});

it('should return a Server when calling createNodeServer()', () => {
const server = NodeServerUtils.createNodeServer();

expect(server).to.not.be.null;
expect(server).to.be.instanceOf(Object);
});

it('should return the constructor when calling getNodeServerConstructor()', () => {
const netServerCtor = NodeServerUtils.getNetServerConstructor();

expect(netServerCtor).to.not.be.null;
expect(typeof netServerCtor).to.equal('function');
});
});
});