feat: allow plugins to modify the server and method map
MODIFY
lib/express/server.js
lib/express/server.js
@@ -33,13 +33,96 @@ async function server (opts = {}) {
hostname = null,
allowCors = true,
basePath = DEFAULT_BASE_PATH,
+ plugins = []
} = opts;
// create the actual http server
const app = express();
- let httpServer = http.createServer(app);
- const serverStateNotifier = new EventEmitter();
- let isServerClosed = false;
+ const httpServer = http.createServer(app);
+ return await new B(async (resolve, reject) => {
+ // we put an async function as the promise constructor because we want some things to happen in
+ // serial (application of plugin updates, for example). But we still need to use a promise here
+ // because some elements of server start failure only happen in httpServer listeners. So the
+ // way we resolve it is to use an async function here but to wrap all the inner logic in
+ // try/catch so any errors can be passed to reject.
+ try {
+ configureHttp({httpServer, reject});
+ configureServer({app, addRoutes: routeConfiguringFunction, allowCors, basePath, plugins});
+ await configureServerPlugins({app, httpServer, plugins});
+
+ // catch this last, after plugins have had a chance to modify app, so anything that falls
+ // through is 404ed
+ app.use(catch404Handler);
+ await startServer({httpServer, hostname, port});
+ resolve(httpServer);
+ } catch (err) {
+ reject(err);
+ }
+ });
+
+}
+
+function configureServer ({
+ app,
+ addRoutes,
+ allowCors = true,
+ basePath = DEFAULT_BASE_PATH,
+ plugins = [],
+}) {
+ basePath = normalizeBasePath(basePath);
+
+ app.use(endLogFormatter);
+
+ // set up static assets
+ app.use(favicon(path.resolve(STATIC_DIR, 'favicon.ico')));
+ app.use(express.static(STATIC_DIR));
+
+ // crash routes, for testing
+ app.use(`${basePath}/produce_error`, produceError);
+ app.use(`${basePath}/crash`, produceCrash);
+
+ // add middlewares
+ if (allowCors) {
+ app.use(allowCrossDomain);
+ } else {
+ app.use(allowCrossDomainAsyncExecute(basePath));
+ }
+ app.use(handleIdempotency);
+ app.use(fixPythonContentType(basePath));
+ app.use(defaultToJSONContentType);
+ app.use(bodyParser.urlencoded({extended: true}));
+ app.use(methodOverride());
+ app.use(catch4XXHandler);
+ app.use(catchAllHandler);
+
+ // make sure appium never fails because of a file size upload limit
+ app.use(bodyParser.json({limit: '1gb'}));
+
+ // set up start logging (which depends on bodyParser doing its thing)
+ app.use(startLogFormatter);
+
+ const extraMethods = plugins.filter((p) => {
+ if (!_.isPlainObject(p.newMethodMap)) {
+ log.warn(`Tried to apply newMethodMap from plugin '${p.name}' but it was invalid. Will ` +
+ `skip adding these methods to the method map.`);
+ return false;
+ }
+ return true;
+ }).map((p) => p.newMethodMap);
+ addRoutes(app, {basePath, extraMethods});
+
+ // dynamic routes for testing, etc.
+ app.all('/welcome', welcome);
+ app.all('/test/guinea-pig', guineaPig);
+ app.all('/test/guinea-pig-scrollable', guineaPigScrollable);
+ app.all('/test/guinea-pig-app-banner', guineaPigAppBanner);
+}
+
+function configureHttp ({httpServer, reject}) {
+ const serverState = {
+ notifier: new EventEmitter(),
+ closed: false,
+ };
httpServer.addWebSocketHandler = addWebSocketHandler;
httpServer.removeWebSocketHandler = removeWebSocketHandler;
httpServer.removeAllWebSocketHandlers = removeAllWebSocketHandlers;
@@ -50,15 +133,14 @@ async function server (opts = {}) {
const close = httpServer.close.bind(httpServer);
httpServer.close = async () => await new B((resolve, reject) => {
// https://github.com/nodejs/node-v0.x-archive/issues/9066#issuecomment-124210576
- isServerClosed = true;
- serverStateNotifier.emit('shutdown');
+ serverState.closed = true;
+ serverState.notifier.emit('shutdown');
httpServer.on('close', resolve);
close((err) => {
if (err) reject(err); // eslint-disable-line curly
});
});
- return await new B((resolve, reject) => {
httpServer.on('error', (err) => {
if (err.code === 'EADDRNOTAVAIL') {
log.error('Could not start REST http interface listener. ' +
@@ -70,6 +152,7 @@ async function server (opts = {}) {
}
reject(err);
});
+
httpServer.on('connection', (socket) => {
socket.setTimeout(KEEP_ALIVE_TIMEOUT_MS);
socket.on('error', reject);
@@ -80,37 +163,51 @@ async function server (opts = {}) {
}
}
socket._openReqCount = 0;
- socket.once('close', () => serverStateNotifier.removeListener('shutdown', destroy));
- serverStateNotifier.once('shutdown', destroy);
+ socket.once('close', () => serverState.notifier.removeListener('shutdown', destroy));
+ serverState.notifier.once('shutdown', destroy);
});
+
httpServer.on('request', function (req, res) {
const socket = req.connection || req.socket;
socket._openReqCount++;
res.on('finish', function () {
socket._openReqCount--;
- if (isServerClosed && socket._openReqCount === 0) {
+ if (serverState.closed && socket._openReqCount === 0) {
socket.destroy();
}
});
});
- configureServer(app, routeConfiguringFunction, allowCors, basePath);
+}
- let serverArgs = [port];
+async function configureServerPlugins ({plugins, app, httpServer}) {
+ // allow plugins to update the app and http server objects
+ for (const plugin of plugins.filter((p) => p.updatesServer)) {
+ log.info(`Allowing plugin ${plugin.name} to modify the Appium server`);
+ try {
+ // wrap this in error handling in case the plugin forgot to mark the function async or
+ // forgot to implement it
+ await plugin.updateServer(app, httpServer);
+ } catch (err) {
+ log.error(`Plugin '${plugin.name}' tried to update the server but the updateServer ` +
+ `method was not implemented, did not return a promise, or threw an ` +
+ `error. Original message: ${err}.`);
+ throw err;
+ }
+ }
+}
+
+async function startServer ({httpServer, port, hostname}) {
+ const serverArgs = [port];
if (hostname) {
// If the hostname is omitted, the server will accept
// connections on any IP address
serverArgs.push(hostname);
}
- httpServer.listen(...serverArgs, (err) => {
- if (err) {
- reject(err);
- }
- resolve(httpServer);
- });
+ const startPromise = B.promisify(httpServer.listen, {context: httpServer})(...serverArgs);
httpServer.keepAliveTimeout = KEEP_ALIVE_TIMEOUT_MS;
// headers timeout must be greater than keepAliveTimeout
httpServer.headersTimeout = KEEP_ALIVE_TIMEOUT_MS + 5 * 1000;
- });
+ await startPromise;
}
function normalizeBasePath (basePath) {
@@ -131,49 +228,4 @@ function normalizeBasePath (basePath) {
return basePath;
}
-function configureServer (app, routeConfiguringFunction, allowCors = true, basePath = DEFAULT_BASE_PATH) {
- basePath = normalizeBasePath(basePath);
-
- app.use(endLogFormatter);
-
- // set up static assets
- app.use(favicon(path.resolve(STATIC_DIR, 'favicon.ico')));
- app.use(express.static(STATIC_DIR));
-
- // crash routes, for testing
- app.use(`${basePath}/produce_error`, produceError);
- app.use(`${basePath}/crash`, produceCrash);
-
- // add middlewares
- if (allowCors) {
- app.use(allowCrossDomain);
- } else {
- app.use(allowCrossDomainAsyncExecute(basePath));
- }
- app.use(handleIdempotency);
- app.use(fixPythonContentType(basePath));
- app.use(defaultToJSONContentType);
- app.use(bodyParser.urlencoded({extended: true}));
- app.use(methodOverride());
- app.use(catch4XXHandler);
- app.use(catchAllHandler);
-
- // make sure appium never fails because of a file size upload limit
- app.use(bodyParser.json({limit: '1gb'}));
-
- // set up start logging (which depends on bodyParser doing its thing)
- app.use(startLogFormatter);
-
- routeConfiguringFunction(app, basePath);
-
- // dynamic routes for testing, etc.
- app.all('/welcome', welcome);
- app.all('/test/guinea-pig', guineaPig);
- app.all('/test/guinea-pig-scrollable', guineaPigScrollable);
- app.all('/test/guinea-pig-app-banner', guineaPigAppBanner);
-
- // catch this last, so anything that falls through is 404ed
- app.use(catch404Handler);
-}
-
export { server, configureServer, normalizeBasePath };

MODIFY
lib/protocol/protocol.js
lib/protocol/protocol.js
@@ -210,13 +210,16 @@ function routeConfiguringFunction (driver) {
throw new Error('Drivers used with MJSONWP must implement `executeCommand` or `execute`');
}
- // return a function which will add all the routes to the driver
- return function addRoutes (app, basePath = DEFAULT_BASE_PATH) {
+ // return a function which will add all the routes to the driver. Here extraMethods might be
+ // passed in as defined by Appium plugins, so we need to add those to the default list
+ return function addRoutes (app, {basePath = DEFAULT_BASE_PATH, extraMethods = {}}) {
// store basePath on the driver instance so it can use it if necessary
// for example in determining proxy avoidance
driver.basePath = basePath;
- for (const [path, methods] of _.toPairs(METHOD_MAP)) {
+ const allMethods = Object.assign({}, METHOD_MAP, ...extraMethods);
+
+ for (const [path, methods] of _.toPairs(allMethods)) {
for (const [method, spec] of _.toPairs(methods)) {
// set up the express route handler
buildHandler(app, method, `${basePath}${path}`, spec, driver, isSessionCommand(spec.command));

MODIFY
test/express/server-e2e-specs.js
test/express/server-e2e-specs.js
@@ -93,3 +93,65 @@ describe('server', function () {
}).should.be.rejectedWith(/EADDRNOTAVAIL/);
});
});
+
+describe('server plugins', function () {
+ let hwServer;
+ afterEach(async function () {
+ try {
+ await hwServer.close();
+ } catch (ign) {}
+ });
+
+ function updaterWithGetRoute (route, reply) {
+ return async (app, httpServer) => { // eslint-disable-line require-await
+ app.get(`/${route}`, (req, res) => {
+ res.header['content-type'] = 'text/html';
+ res.status(200).send(reply);
+ });
+ httpServer[`_updated_${route}`] = true;
+ };
+ }
+
+ it('should not allow a plugin to update server if it has updatesServer set to false', async function () {
+ hwServer = await server({
+ routeConfiguringFunction: _.noop,
+ port: 8181,
+ plugins: [
+ {
+ updatesServer: false,
+ updateServer: updaterWithGetRoute('no', 'Should never see this'),
+ }
+ ]
+ });
+ await axios.get('http://localhost:8181/no').should.eventually.be.rejectedWith(/404/);
+ });
+ it('should allow one or more plugins to update the server', async function () {
+ hwServer = await server({
+ routeConfiguringFunction: _.noop,
+ port: 8181,
+ plugins: [{
+ updatesServer: true,
+ updateServer: updaterWithGetRoute('plugin1', 'res from plugin1 route'),
+ }, {
+ updatesServer: true,
+ updateServer: updaterWithGetRoute('plugin2', 'res from plugin2 route'),
+ }]
+ });
+ let {data} = await axios.get('http://localhost:8181/plugin1');
+ data.should.eql('res from plugin1 route');
+ ({data} = await axios.get('http://localhost:8181/plugin2'));
+ data.should.eql('res from plugin2 route');
+ hwServer._updated_plugin1.should.be.true;
+ hwServer._updated_plugin2.should.be.true;
+ });
+ it('should not allow a plugin to update server if it has incorrect updateServer method', async function () {
+ await server({
+ routeConfiguringFunction: _.noop,
+ port: 8181,
+ plugins: [{
+ updatesServer: true,
+ updateServer: false
+ }]
+ }).should.eventually.be.rejectedWith(/is not a function/);
+ });
+});

MODIFY
test/express/server-specs.js
test/express/server-specs.js
// transpile:mocha
-import { server } from '../..';
+import { server, routeConfiguringFunction } from '../..';
import { configureServer, normalizeBasePath } from '../../lib/express/server';
import chai from 'chai';
import chaiAsPromised from 'chai-as-promised';
@@ -10,15 +10,88 @@ import sinon from 'sinon';
const should = chai.should();
chai.use(chaiAsPromised);
+function fakeApp () {
+ const app = {
+ use: sinon.spy(),
+ all: sinon.spy(),
+ get: sinon.spy(),
+ post: sinon.spy(),
+ delete: sinon.spy(),
+ totalCount: () => (
+ app.use.callCount + app.all.callCount + app.get.callCount + app.post.callCount +
+ app.delete.callCount
+ )
+ };
+ return app;
+}
+
+function fakePlugin () {
+ return {
+ name: 'fake',
+ newMethodMap: {
+ '/session/:sessionId/fake': {
+ GET: {command: 'fakeGet'},
+ POST: {command: 'fakePost', payloadParams: {required: ['fakeParam']}}
+ },
+ },
+ updatesServer: true,
+ updateServer: (app, httpServer) => {
+ app.updated = true;
+ httpServer.updated = true;
+ }
+ };
+}
+
+function fakeDriver () {
+ return {sessionExists: () => {}, executeCommand: () => {}};
+}
+
describe('server configuration', function () {
it('should actually use the middleware', function () {
- let app = {use: sinon.spy(), all: sinon.spy()};
- let configureRoutes = () => {};
- configureServer(app, configureRoutes);
- app.use.callCount.should.equal(16);
+ const app = fakeApp();
+ const configureRoutes = () => {};
+ configureServer({app, addRoutes: configureRoutes});
+ app.use.callCount.should.equal(15);
app.all.callCount.should.equal(4);
});
+ it('should apply new methods in plugins to the standard method map', function () {
+ const app1 = fakeApp();
+ const app2 = fakeApp();
+ const driver = fakeDriver();
+ const addRoutes = routeConfiguringFunction(driver);
+ configureServer({app: app1, addRoutes});
+ configureServer({app: app2, addRoutes, plugins: [fakePlugin()]});
+ app2.totalCount().should.eql(app1.totalCount() + 2);
+ });
+
+ it('should silently reject new methods in plugins if not plain objects', function () {
+ const app1 = fakeApp();
+ const app2 = fakeApp();
+ const driver = fakeDriver();
+ const addRoutes = routeConfiguringFunction(driver);
+ const plugin = fakePlugin();
+ plugin.newMethodMap = [];
+ configureServer({app: app1, addRoutes});
+ configureServer({app: app2, addRoutes, plugins: [plugin]});
+ app2.totalCount().should.eql(app1.totalCount());
+ });
+
+ it('should allow plugins to update the server', async function () {
+ const plugins = [fakePlugin()];
+ const driver = fakeDriver();
+ const _server = await server({
+ routeConfiguringFunction: routeConfiguringFunction(driver),
+ port: 8181,
+ plugins,
+ });
+ try {
+ _server.updated.should.be.true;
+ } finally {
+ await _server.close();
+ }
+ });
+
it('should reject if error thrown in configureRoutes parameter', async function () {
const configureRoutes = () => {
throw new Error('I am Mr. MeeSeeks look at me!');

feat: allow plugins to modify the server and method map (#422)