From 62b5ba5c99113a5c46fe860b367a35c8a1b7dc63 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Thu, 20 Jun 2019 11:26:54 -0500 Subject: [PATCH 1/4] feat(server): check for serverMode implementation problems --- lib/utils/getSocketServerImplementation.js | 44 +++++++++ .../getSocketServerImplementation.test.js | 97 ++++++++++++++++++- 2 files changed, 137 insertions(+), 4 deletions(-) diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js index 6df634085f..045f96332a 100644 --- a/lib/utils/getSocketServerImplementation.js +++ b/lib/utils/getSocketServerImplementation.js @@ -1,5 +1,7 @@ 'use strict'; +const BaseServer = require('../servers/BaseServer'); + function getSocketServerImplementation(options) { let ServerImplementation; let serverImplFound = true; @@ -35,6 +37,48 @@ function getSocketServerImplementation(options) { ); } + if (!(ServerImplementation.prototype instanceof BaseServer)) { + throw new Error( + 'serverMode must extend the class BaseServer, found via require(webpack-dev-server/lib/servers/BaseServer)' + ); + } + + if ( + !ServerImplementation.prototype.constructor || + ServerImplementation.prototype.constructor.length < 1 + ) { + throw new Error( + 'serverMode must have a constructor that takes a single server argument and calls super(server)' + ); + } + + if ( + !ServerImplementation.prototype.send || + ServerImplementation.prototype.send.length < 2 + ) { + throw new Error( + 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' + ); + } + + if ( + !ServerImplementation.prototype.close || + ServerImplementation.prototype.close.length < 1 + ) { + throw new Error( + 'serverMode must have a close(connection) method that closes the provided client connection object' + ); + } + + if ( + !ServerImplementation.prototype.onConnection || + ServerImplementation.prototype.onConnection.length < 1 + ) { + throw new Error( + 'serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made' + ); + } + return ServerImplementation; } diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 0a157c39a9..63203f928a 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -1,10 +1,13 @@ 'use strict'; +/* eslint-disable constructor-super, no-empty-function, no-useless-constructor, no-unused-vars, class-methods-use-this */ + const getSocketServerImplementation = require('../../../lib/utils/getSocketServerImplementation'); +const BaseServer = require('../../../lib/servers/BaseServer'); const SockJSServer = require('../../../lib/servers/SockJSServer'); describe('getSocketServerImplementation util', () => { - it("should works with string serverMode ('sockjs')", () => { + it("should work with string serverMode ('sockjs')", () => { let result; expect(() => { @@ -16,7 +19,7 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); - it('should works with serverMode (SockJSServer class)', () => { + it('should work with serverMode (SockJSServer class)', () => { let result; expect(() => { @@ -40,11 +43,97 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); - it('should throws with serverMode (bad path)', () => { + it('should throw with serverMode (bad path)', () => { expect(() => { getSocketServerImplementation({ serverMode: '/bad/path/to/implementation', }); - }).toThrow(/serverMode must be a string/); + }).toThrow(/serverMode must be a string denoting a default implementation/); + }); + + it('should throw with serverMode (not extending BaseServer)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation {}, + }); + }).toThrow( + 'serverMode must extend the class BaseServer, found via require(webpack-dev-server/lib/servers/BaseServer)' + ); + }); + + it('should throw with serverMode (incorrect constructor)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation extends BaseServer { + constructor() {} + }, + }); + }).toThrow( + 'serverMode must have a constructor that takes a single server argument and calls super(server)' + ); + }); + + it('should throw with serverMode (no send method)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation extends BaseServer { + constructor(server) { + super(server); + } + }, + }); + }).toThrow( + 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' + ); + }); + + it('should throw with serverMode (incorrect send parameters)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation extends BaseServer { + constructor(server) { + super(server); + } + + send(connection) {} + }, + }); + }).toThrow( + 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' + ); + }); + + it('should throw with serverMode (no close method)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation extends BaseServer { + constructor(server) { + super(server); + } + + send(connection, message) {} + }, + }); + }).toThrow( + 'serverMode must have a close(connection) method that closes the provided client connection object' + ); + }); + + it('should throw with serverMode (no onConnection method)', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: class ServerImplementation extends BaseServer { + constructor(server) { + super(server); + } + + send(connection, message) {} + + close(connection) {} + }, + }); + }).toThrow( + 'serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made' + ); }); }); From 557f7e848f484b619d1f3cc370497ae11cbc9910 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Sat, 22 Jun 2019 22:45:56 -0500 Subject: [PATCH 2/4] test(server): remove extends BaseServer requirement --- lib/utils/getSocketServerImplementation.js | 11 ++--------- .../utils/getSocketServerImplementation.test.js | 12 +----------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js index 045f96332a..4e9bd88cb3 100644 --- a/lib/utils/getSocketServerImplementation.js +++ b/lib/utils/getSocketServerImplementation.js @@ -1,7 +1,5 @@ 'use strict'; -const BaseServer = require('../servers/BaseServer'); - function getSocketServerImplementation(options) { let ServerImplementation; let serverImplFound = true; @@ -37,18 +35,13 @@ function getSocketServerImplementation(options) { ); } - if (!(ServerImplementation.prototype instanceof BaseServer)) { - throw new Error( - 'serverMode must extend the class BaseServer, found via require(webpack-dev-server/lib/servers/BaseServer)' - ); - } - if ( !ServerImplementation.prototype.constructor || ServerImplementation.prototype.constructor.length < 1 ) { throw new Error( - 'serverMode must have a constructor that takes a single server argument and calls super(server)' + 'serverMode must have a constructor that takes a single server argument and calls super(server) ' + + "on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')" ); } diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 63203f928a..3708e8f070 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -51,16 +51,6 @@ describe('getSocketServerImplementation util', () => { }).toThrow(/serverMode must be a string denoting a default implementation/); }); - it('should throw with serverMode (not extending BaseServer)', () => { - expect(() => { - getSocketServerImplementation({ - serverMode: class ServerImplementation {}, - }); - }).toThrow( - 'serverMode must extend the class BaseServer, found via require(webpack-dev-server/lib/servers/BaseServer)' - ); - }); - it('should throw with serverMode (incorrect constructor)', () => { expect(() => { getSocketServerImplementation({ @@ -69,7 +59,7 @@ describe('getSocketServerImplementation util', () => { }, }); }).toThrow( - 'serverMode must have a constructor that takes a single server argument and calls super(server)' + "serverMode must have a constructor that takes a single server argument and calls super(server) on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')" ); }); From 2b10ecec65468e4e66e6b2cb75aa085f316a5d7a Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Mon, 24 Jun 2019 22:30:57 -0500 Subject: [PATCH 3/4] feat(server): test socket server implementation with schema utils --- .../socketServerImplementationPrototype.json | 27 +++++ lib/utils/getSocketServerImplementation.js | 40 +------ ...getSocketServerImplementation.test.js.snap | 37 ++++++ .../getSocketServerImplementation.test.js | 105 ++++++++---------- 4 files changed, 113 insertions(+), 96 deletions(-) create mode 100644 lib/schemas/utils/socketServerImplementationPrototype.json create mode 100644 test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap diff --git a/lib/schemas/utils/socketServerImplementationPrototype.json b/lib/schemas/utils/socketServerImplementationPrototype.json new file mode 100644 index 0000000000..fc2a9fb20e --- /dev/null +++ b/lib/schemas/utils/socketServerImplementationPrototype.json @@ -0,0 +1,27 @@ +{ + "type": "object", + "properties": { + "constructor": { + "instanceof": "Function" + }, + "send": { + "instanceof": "Function" + }, + "close": { + "instanceof": "Function" + }, + "onConnection": { + "instanceof": "Function" + } + }, + "errorMessage": { + "properties": { + "constructor": "- serverMode must have a constructor that takes a single server argument and calls super(server) on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')", + "send": "- serverMode must have a send(connection, message) method that sends the message string to the provided client connection object", + "close": "- serverMode must have a close(connection) method that closes the provided client connection object", + "onConnection": "- serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made" + } + }, + "required": ["constructor", "send", "close", "onConnection"], + "additionalProperties": false +} diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js index 4e9bd88cb3..f53f2af539 100644 --- a/lib/utils/getSocketServerImplementation.js +++ b/lib/utils/getSocketServerImplementation.js @@ -1,5 +1,8 @@ 'use strict'; +const validateOptions = require('schema-utils'); +const schema = require('../schemas/utils/socketServerImplementationPrototype.json'); + function getSocketServerImplementation(options) { let ServerImplementation; let serverImplFound = true; @@ -35,42 +38,7 @@ function getSocketServerImplementation(options) { ); } - if ( - !ServerImplementation.prototype.constructor || - ServerImplementation.prototype.constructor.length < 1 - ) { - throw new Error( - 'serverMode must have a constructor that takes a single server argument and calls super(server) ' + - "on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')" - ); - } - - if ( - !ServerImplementation.prototype.send || - ServerImplementation.prototype.send.length < 2 - ) { - throw new Error( - 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' - ); - } - - if ( - !ServerImplementation.prototype.close || - ServerImplementation.prototype.close.length < 1 - ) { - throw new Error( - 'serverMode must have a close(connection) method that closes the provided client connection object' - ); - } - - if ( - !ServerImplementation.prototype.onConnection || - ServerImplementation.prototype.onConnection.length < 1 - ) { - throw new Error( - 'serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made' - ); - } + validateOptions(schema, ServerImplementation.prototype, 'webpack Dev Server'); return ServerImplementation; } diff --git a/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap b/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap new file mode 100644 index 0000000000..7980b66c59 --- /dev/null +++ b/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap @@ -0,0 +1,37 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`getSocketServerImplementation util should throw with serverMode (bad path) 1`] = `[Error: serverMode must be a string denoting a default implementation (e.g. 'sockjs'), a full path to a JS file which exports a class extending BaseServer (webpack-dev-server/lib/servers/BaseServer) via require.resolve(...), or the class itself which extends BaseServer]`; + +exports[`getSocketServerImplementation util should throw with serverMode (no close, onConnection methods) 1`] = ` +[ValidationError: webpack Dev Server Invalid Options + +options should have required property 'close' +options should have required property 'onConnection' +] +`; + +exports[`getSocketServerImplementation util should throw with serverMode (no constructor, send, close, onConnection methods) 1`] = ` +[ValidationError: webpack Dev Server Invalid Options + +options should have required property 'constructor' +options should have required property 'send' +options should have required property 'close' +options should have required property 'onConnection' +] +`; + +exports[`getSocketServerImplementation util should throw with serverMode (no onConnection method) 1`] = ` +[ValidationError: webpack Dev Server Invalid Options + +options should have required property 'onConnection' +] +`; + +exports[`getSocketServerImplementation util should throw with serverMode (no send, close, onConnection methods) 1`] = ` +[ValidationError: webpack Dev Server Invalid Options + +options should have required property 'send' +options should have required property 'close' +options should have required property 'onConnection' +] +`; diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 3708e8f070..21810c4b89 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -43,59 +43,38 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); - it('should throw with serverMode (bad path)', () => { - expect(() => { - getSocketServerImplementation({ + const ClassWithoutConstructor = class ClassWithoutConstructor {}; + // eslint-disable-next-line no-undefined + ClassWithoutConstructor.prototype.constructor = undefined; + + const badSetups = [ + { + title: 'should throw with serverMode (bad path)', + config: { serverMode: '/bad/path/to/implementation', - }); - }).toThrow(/serverMode must be a string denoting a default implementation/); - }); - - it('should throw with serverMode (incorrect constructor)', () => { - expect(() => { - getSocketServerImplementation({ - serverMode: class ServerImplementation extends BaseServer { - constructor() {} - }, - }); - }).toThrow( - "serverMode must have a constructor that takes a single server argument and calls super(server) on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')" - ); - }); - - it('should throw with serverMode (no send method)', () => { - expect(() => { - getSocketServerImplementation({ - serverMode: class ServerImplementation extends BaseServer { - constructor(server) { - super(server); - } - }, - }); - }).toThrow( - 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' - ); - }); - - it('should throw with serverMode (incorrect send parameters)', () => { - expect(() => { - getSocketServerImplementation({ + }, + }, + { + title: + 'should throw with serverMode (no constructor, send, close, onConnection methods)', + config: { + serverMode: ClassWithoutConstructor, + }, + }, + { + title: + 'should throw with serverMode (no send, close, onConnection methods)', + config: { serverMode: class ServerImplementation extends BaseServer { constructor(server) { super(server); } - - send(connection) {} }, - }); - }).toThrow( - 'serverMode must have a send(connection, message) method that sends the message string to the provided client connection object' - ); - }); - - it('should throw with serverMode (no close method)', () => { - expect(() => { - getSocketServerImplementation({ + }, + }, + { + title: 'should throw with serverMode (no close, onConnection methods)', + config: { serverMode: class ServerImplementation extends BaseServer { constructor(server) { super(server); @@ -103,15 +82,11 @@ describe('getSocketServerImplementation util', () => { send(connection, message) {} }, - }); - }).toThrow( - 'serverMode must have a close(connection) method that closes the provided client connection object' - ); - }); - - it('should throw with serverMode (no onConnection method)', () => { - expect(() => { - getSocketServerImplementation({ + }, + }, + { + title: 'should throw with serverMode (no onConnection method)', + config: { serverMode: class ServerImplementation extends BaseServer { constructor(server) { super(server); @@ -121,9 +96,19 @@ describe('getSocketServerImplementation util', () => { close(connection) {} }, - }); - }).toThrow( - 'serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made' - ); + }, + }, + ]; + + badSetups.forEach((setup) => { + it(setup.title, () => { + let thrown = false; + try { + getSocketServerImplementation(setup.config); + } catch (e) { + thrown = true; + expect(e).toMatchSnapshot(); + } + }); }); }); From cf6e74a49639bd63c4e19430a31200ae18a7eeb0 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Mon, 24 Jun 2019 23:10:10 -0500 Subject: [PATCH 4/4] fix(server): allow any properties for server impl schema --- .../socketServerImplementationPrototype.json | 8 +++++++- ...getSocketServerImplementation.test.js.snap | 20 +++++++++---------- .../getSocketServerImplementation.test.js | 18 +++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/schemas/utils/socketServerImplementationPrototype.json b/lib/schemas/utils/socketServerImplementationPrototype.json index fc2a9fb20e..a17dfe538c 100644 --- a/lib/schemas/utils/socketServerImplementationPrototype.json +++ b/lib/schemas/utils/socketServerImplementationPrototype.json @@ -20,8 +20,14 @@ "send": "- serverMode must have a send(connection, message) method that sends the message string to the provided client connection object", "close": "- serverMode must have a close(connection) method that closes the provided client connection object", "onConnection": "- serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made" + }, + "required": { + "constructor": "- serverMode must have a constructor that takes a single server argument and calls super(server) on the superclass BaseServer, found via require('webpack-dev-server/lib/servers/BaseServer')", + "send": "- serverMode must have a send(connection, message) method that sends the message string to the provided client connection object", + "close": "- serverMode must have a close(connection) method that closes the provided client connection object", + "onConnection": "- serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made" } }, "required": ["constructor", "send", "close", "onConnection"], - "additionalProperties": false + "additionalProperties": true } diff --git a/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap b/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap index 7980b66c59..d67f1e59f6 100644 --- a/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap +++ b/test/server/utils/__snapshots__/getSocketServerImplementation.test.js.snap @@ -5,33 +5,33 @@ exports[`getSocketServerImplementation util should throw with serverMode (bad pa exports[`getSocketServerImplementation util should throw with serverMode (no close, onConnection methods) 1`] = ` [ValidationError: webpack Dev Server Invalid Options -options should have required property 'close' -options should have required property 'onConnection' +options - serverMode must have a close(connection) method that closes the provided client connection object +options - serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made ] `; exports[`getSocketServerImplementation util should throw with serverMode (no constructor, send, close, onConnection methods) 1`] = ` [ValidationError: webpack Dev Server Invalid Options -options should have required property 'constructor' -options should have required property 'send' -options should have required property 'close' -options should have required property 'onConnection' +options [object Object] +options - serverMode must have a send(connection, message) method that sends the message string to the provided client connection object +options - serverMode must have a close(connection) method that closes the provided client connection object +options - serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made ] `; exports[`getSocketServerImplementation util should throw with serverMode (no onConnection method) 1`] = ` [ValidationError: webpack Dev Server Invalid Options -options should have required property 'onConnection' +options - serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made ] `; exports[`getSocketServerImplementation util should throw with serverMode (no send, close, onConnection methods) 1`] = ` [ValidationError: webpack Dev Server Invalid Options -options should have required property 'send' -options should have required property 'close' -options should have required property 'onConnection' +options - serverMode must have a send(connection, message) method that sends the message string to the provided client connection object +options - serverMode must have a close(connection) method that closes the provided client connection object +options - serverMode must have a onConnection(f) method that calls f(connection) whenever a new client connection is made ] `; diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 21810c4b89..50b7c5bd5c 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -43,6 +43,24 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); + it('should work with serverMode (additional class methods)', () => { + let result; + + const ExtendedSockJSServer = class ExtendedSockJSServer extends SockJSServer { + myMethod() { + this.test = true; + } + }; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: ExtendedSockJSServer, + }); + }).not.toThrow(); + + expect(result).toEqual(ExtendedSockJSServer); + }); + const ClassWithoutConstructor = class ClassWithoutConstructor {}; // eslint-disable-next-line no-undefined ClassWithoutConstructor.prototype.constructor = undefined;