refactor!: generalize concepts of server updater and extra method map
basedriver shouldn't really know about plugins or drivers.
it should just expose the ability to update the server or add methods
MODIFY
lib/express/server.js
lib/express/server.js
@@ -33,7 +33,8 @@ async function server (opts = {}) {
hostname = null,
allowCors = true,
basePath = DEFAULT_BASE_PATH,
- plugins = [],
+ extraMethodMap = {},
+ serverUpdaters = [],
keepAliveTimeout = KEEP_ALIVE_TIMEOUT_MS,
} = opts;
@@ -48,12 +49,15 @@ async function server (opts = {}) {
// try/catch so any errors can be passed to reject.
try {
configureHttp({httpServer, reject, keepAliveTimeout});
- configureServer({app, addRoutes: routeConfiguringFunction, allowCors, basePath, plugins});
- await configureServerPlugins({app, httpServer, plugins});
+ configureServer({app, addRoutes: routeConfiguringFunction, allowCors, basePath, extraMethodMap});
+ // allow extensions to update the app and http server objects
+ for (const updater of serverUpdaters) {
+ await updater(app, httpServer);
+ }
- // once all configurations and plugins have been applied, make sure to set up a catchall
+ // once all configurations and updaters have been applied, make sure to set up a catchall
// handler so that anything unknown 404s. But do this after everything else since we don't
- // want to block plugins' ability to add routes if they want.
+ // want to block extensions' ability to add routes if they want.
app.all('*', catch404Handler);
await startServer({httpServer, hostname, port, keepAliveTimeout});
@@ -70,7 +74,7 @@ function configureServer ({
addRoutes,
allowCors = true,
basePath = DEFAULT_BASE_PATH,
- plugins = [],
+ extraMethodMap = {},
}) {
basePath = normalizeBasePath(basePath);
@@ -103,15 +107,7 @@ function configureServer ({
// 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});
+ addRoutes(app, {basePath, extraMethodMap});
// dynamic routes for testing, etc.
app.all('/welcome', welcome);
@@ -183,23 +179,6 @@ function configureHttp ({httpServer, reject, keepAliveTimeout}) {
});
}
-async function configureServerPlugins ({plugins, app, httpServer}) {
- // allow plugins to update the app and http server objects
- for (const plugin of plugins.filter((p) => _.isFunction(p.updateServer))) {
- 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, keepAliveTimeout}) {
const serverArgs = [port];
if (hostname) {

MODIFY
lib/protocol/protocol.js
lib/protocol/protocol.js
@@ -200,12 +200,12 @@ function routeConfiguringFunction (driver) {
// 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 = {}}) {
+ return function addRoutes (app, {basePath = DEFAULT_BASE_PATH, extraMethodMap = {}}) {
// store basePath on the driver instance so it can use it if necessary
// for example in determining proxy avoidance
driver.basePath = basePath;
- const allMethods = Object.assign({}, METHOD_MAP, ...extraMethods);
+ const allMethods = {...METHOD_MAP, ...extraMethodMap};
for (const [path, methods] of _.toPairs(allMethods)) {
for (const [method, spec] of _.toPairs(methods)) {

MODIFY
test/express/server-e2e-specs.js
test/express/server-e2e-specs.js
@@ -116,11 +116,10 @@ describe('server plugins', function () {
hwServer = await server({
routeConfiguringFunction: _.noop,
port: 8181,
- plugins: [{
- updateServer: updaterWithGetRoute('plugin1', 'res from plugin1 route'),
- }, {
- updateServer: updaterWithGetRoute('plugin2', 'res from plugin2 route'),
- }]
+ serverUpdaters: [
+ updaterWithGetRoute('plugin1', 'res from plugin1 route'),
+ updaterWithGetRoute('plugin2', 'res from plugin2 route'),
+ ]
});
let {data} = await axios.get('http://localhost:8181/plugin1');
data.should.eql('res from plugin1 route');
@@ -133,9 +132,7 @@ describe('server plugins', function () {
await server({
routeConfiguringFunction: _.noop,
port: 8181,
- plugins: [{
- updateServer: () => { throw new Error('ugh'); }
- }]
+ serverUpdaters: [() => { throw new Error('ugh');}]
}).should.eventually.be.rejectedWith(/ugh/);
});
});

MODIFY
test/express/server-specs.js
test/express/server-specs.js
@@ -25,21 +25,17 @@ function fakeApp () {
return app;
}
-function fakePlugin () {
- return {
- name: 'fake',
- newMethodMap: {
+const newMethodMap = {
'/session/:sessionId/fake': {
GET: {command: 'fakeGet'},
POST: {command: 'fakePost', payloadParams: {required: ['fakeParam']}}
},
- },
- updateServer: (app, httpServer) => {
+};
+
+const updateServer = (app, httpServer) => {
app.updated = true;
httpServer.updated = true;
- }
};
-}
function fakeDriver () {
return {sessionExists: () => {}, executeCommand: () => {}};
@@ -60,7 +56,7 @@ describe('server configuration', function () {
const driver = fakeDriver();
const addRoutes = routeConfiguringFunction(driver);
configureServer({app: app1, addRoutes});
- configureServer({app: app2, addRoutes, plugins: [fakePlugin()]});
+ configureServer({app: app2, addRoutes, extraMethodMap: newMethodMap});
app2.totalCount().should.eql(app1.totalCount() + 2);
});
@@ -69,20 +65,18 @@ describe('server configuration', function () {
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]});
+ configureServer({app: app2, addRoutes, extraMethodMap: []});
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,
+ extraMethodMap: newMethodMap,
+ serverUpdaters: [updateServer]
});
try {
_server.updated.should.be.true;

refactor!: generalize concepts of server updater and extra method map
basedriver shouldn't really know about plugins or drivers.
it should just expose the ability to update the server or add methods