feat(connection): align connection options

Both REST and XML-RPC now have the same set of default options.
The boolean option `secure` can still be used
to control XMLRPC client, but `protocol` is the preferred way.
This allows to use the same set of options for both client constructors.

Add test to ensure legacy option is still working.

Author: line-o

components/connection.js

@@ -8,116 +8,163 @@ const promisedMethodCall = require('./promisedMethodCall')
 
 /**
  * @typedef {Object} NodeExistConnectionOptions
+ * @prop {{user:string, pass:string}} [basic_auth] database user credentials, default: {"user":"guest","pass":"guest"}
+ * @prop {"http:"|"https:"} [protocol] "http:" or "https:", default: "https:"
  * @prop {string} [host] database host, default: "localhost"
  * @prop {string} [port] database port, default: "8443"
- * @prop {boolean} [secure] use HTTPS? default: true
- * @prop {boolean} [rejectUnauthorized] enforce valid SSL connection, default: true
  * @prop {string} [path] path to XMLRPC, default: "/exist/xmlrpc"
- * @prop {{user:string, pass:string}} [basic_auth] database user credentials, default: {"user":"guest","pass":"guest"}
+ * @prop {boolean} [rejectUnauthorized] enforce valid SSL certs, default: true for remote hosts
+ */
+
+/**
+ * @typedef {Object} MergedOptions
+ * @prop {{user:string, pass:string}} basic_auth database user credentials
+ * @prop {"http:"|"https:"} protocol "http:" or "https:"
+ * @prop {string} host database host
+ * @prop {string} port database port
+ * @prop {string} path path to XMLRPC, default: "/exist/xmlrpc"
+ * @prop {boolean} [rejectUnauthorized] enforce valid SSL certs, if https: is used
+ */
+
+/**
+ * Default REST endpoint
+ * @type {string}
  */
+const defaultRestEndpoint = '/exist/rest'
+
+/**
+ * Default XML-RPC endpoint
+ * @type {string}
+ */
+const defaultXmlrpcEndpoint = '/exist/xmlrpc'
 
 /**
  * Default connection options
  * @type {NodeExistConnectionOptions}
  */
-const defaultRPCoptions = {
-  host: 'localhost',
-  port: '8443',
-  path: '/exist/xmlrpc',
+const defaultConnectionOptions = {
   basic_auth: {
     user: 'guest',
     pass: 'guest'
-  }
-}
-
-const defaultRestOptions = {
-  host: 'localhost',
+  },
   protocol: 'https:',
-  port: '8443',
-  path: '/exist/rest',
-  basic_auth: {
-    user: 'guest',
-    pass: 'guest'
-  }
-}
-
-function isLocalDB (host) {
-  return (
-    host === 'localhost' ||
-    host === '127.0.0.1' ||
-    host === '[::1]'
-  )
+  host: 'localhost',
+  port: '8443'
 }
 
-function useSecureConnection (options) {
-  if (options && 'secure' in options) {
-    return Boolean(options.secure)
+/**
+ * get REST client
+ * @param {NodeExistConnectionOptions} [options] connection options
+ * @returns {got} Extended HTTP client instance
+ */
+async function restConnection (options) {
+  const { got } = await import('got')
+  /* eslint camelcase: "off" */
+  const { basic_auth, protocol, host, port, path, rejectUnauthorized } = mergeOptions(defaultRestEndpoint, options)
+
+  const prefixUrl = protocol + '//' + host + (port ? ':' + port : '') + path
+
+  const httpClientOptions = {
+    prefixUrl,
+    headers: {
+      'user-agent': 'node-exist',
+      authorization: basicAuth(basic_auth)
+    },
+    https: { rejectUnauthorized }
   }
-  return true
+
+  return got.extend(httpClientOptions)
 }
 
-function basicAuth (name, pass) {
-  const payload = pass ? `${name}:${pass}` : name
+/**
+ * Basic authorization header value
+ * @prop {{user:string, pass:string}} auth database user credentials
+ * @returns {string} header value
+ */
+function basicAuth (auth) {
+  const payload = auth.pass ? `${auth.user}:${auth.pass}` : auth.user
   return 'Basic ' + Buffer.from(payload).toString('base64')
 }
 
 /**
  * Connect to database via XML-RPC
- * @param {NodeExistConnectionOptions} options
+ * @param {NodeExistConnectionOptions} [options] connection options
  * @returns {XMLRPCClient} XMLRPC-client
  */
 function connect (options) {
-  const _options = assign({}, defaultRPCoptions, options)
-  delete _options.secure // prevent pollution of XML-RPC options
-
-  let client
-  if (useSecureConnection(options)) {
-    // allow invalid and self-signed certificates on localhost, if not explicitly
-    // enforced by setting options.rejectUnauthorized to true
-    _options.rejectUnauthorized = ('rejectUnauthorized' in _options)
-      ? _options.rejectUnauthorized
-      : !isLocalDB(_options.host)
-
-    client = xmlrpc.createSecureClient(_options)
-  } else {
-    if (!isLocalDB(_options.host)) {
-      console.warn('Connecting to DB using an unencrypted channel.')
-    }
-    client = xmlrpc.createClient(_options)
-  }
+  const mergedOptions = mergeOptions(defaultXmlrpcEndpoint, options)
+  const client = getXMLRPCClient(mergedOptions)
   client.promisedMethodCall = promisedMethodCall(client)
   return client
 }
 
-async function restConnection (options) {
-  const { got } = await import('got')
-  const _options = assign({}, defaultRestOptions, options)
-  const authorization = basicAuth(_options.basic_auth.user, _options.basic_auth.pass)
+/**
+ *
+ * @param {MergedOptions} options
+ * @returns {XMLRPCClient} XMLRPC-client
+ */
+function getXMLRPCClient (options) {
+  if (useSecureConnection(options.protocol)) {
+    return xmlrpc.createSecureClient(options)
+  }
+  return xmlrpc.createClient(options)
+}
+
+/**
+ * Merge options with defaults
+ *
+ * Allow invalid and self-signed certificates on localhost,
+ * if not explicitly set to be enforced.
+ * @param {string} path default endpoint
+ * @param {NodeExistConnectionOptions} [options] given options
+ * @returns {MergedOptions} merged options
+ */
+function mergeOptions (path, options) {
+  const mergedOptions = assign({ path }, defaultConnectionOptions, options)
 
-  const rejectUnauthorized = ('rejectUnauthorized' in _options)
-    ? _options.rejectUnauthorized
-    : !isLocalDB(_options.host)
+  // compatibility for older setups
+  if ('secure' in mergedOptions) {
+    mergedOptions.protocol = mergedOptions.secure ? 'https:' : 'http:'
+    delete mergedOptions.secure // remove legacy option
+  }
 
-  if (!isLocalDB(_options.host) && _options.protocol === 'http') {
-    console.warn('Connecting to remote DB using an unencrypted channel.')
+  const isLocalDb = checkIfLocalHost(mergedOptions.host)
+  const isSecureClient = useSecureConnection(mergedOptions.protocol)
+  if (isLocalDb && isSecureClient && !('rejectUnauthorized' in mergedOptions)) {
+    mergedOptions.rejectUnauthorized = false
   }
 
-  const port = _options.port ? ':' + _options.port : ''
-  const path = _options.path.startsWith('/') ? _options.path : '/' + _options.path
-  const prefixUrl = `${_options.protocol}//${_options.host}${port}${path}`
-
-  const client = got.extend(
-    {
-      prefixUrl,
-      headers: {
-        'user-agent': 'node-exist',
-        authorization
-      },
-      https: { rejectUnauthorized }
+  if (!isLocalDb) {
+    if (!isSecureClient) {
+      console.warn('Connecting to remote DB using an unencrypted channel.')
+    }
+    if (!mergedOptions.rejectUnauthorized) {
+      console.warn('Connecting to remote DB allowing invalid certificate.')
     }
+  }
+  return mergedOptions
+}
+
+/**
+ * Is the host considered a local host
+ * @param {string} host hostname
+ * @returns {boolean} true, if host is local
+ */
+function checkIfLocalHost (host) {
+  return (
+    host === 'localhost' ||
+    host === '127.0.0.1' || // TODO: 127.0.1.1 is also local
+    host === '[::1]' // TODO: match all ipv6 addresses considered local
   )
+}
 
-  return client
+/**
+ * SSL or not?
+ * @param {string} protocol must end in colon
+ * @returns {boolean} true, if encrypted connection
+ */
+function useSecureConnection (protocol) {
+  return protocol === 'https:'
 }
 
 /**
@@ -144,10 +191,9 @@ function readOptionsFromEnv () {
       throw new Error('Unknown protocol: "' + protocol + '"!')
     }
 
-    environmentOptions.secure = protocol === 'https:'
+    environmentOptions.protocol = protocol
     environmentOptions.host = hostname
     environmentOptions.port = port
-    environmentOptions.protocol = protocol
   }
 
   return environmentOptions
@@ -157,6 +203,7 @@ module.exports = {
   connect,
   readOptionsFromEnv,
   restConnection,
-  defaultRPCoptions,
-  defaultRestOptions
+  defaultConnectionOptions,
+  defaultXmlrpcEndpoint,
+  defaultRestEndpoint
 }

spec/tests/index.js

@@ -59,7 +59,13 @@ test('create connection with default settings', function (t) {
   t.end()
 })
 
-test('create connection using http://', function (t) {
+test('create connection using http:', function (t) {
+  const db = connect({ protocol: 'http:', port: 8080 })
+  t.equal(db.client.isSecure, false, 'insecure client used')
+  t.end()
+})
+
+test('create insecure client using legacy option', function (t) {
   const db = connect({ secure: false, port: 8080 })
   t.equal(db.client.isSecure, false, 'insecure client used')
   t.end()