Merge pull request #569 from ldapjs/client-options-tweaks

Clean up options checking and add some tests

Author: jsumners

lib/client/index.js

@@ -1,35 +1,23 @@
-// Copyright 2012 Mark Cavage, Inc.  All rights reserved.
+'use strict'
 
 const logger = require('../logger')
 const Client = require('./client')
 
-/// --- Functions
-
-function xor () {
-  var b = false
-  for (var i = 0; i < arguments.length; i++) {
-    if (arguments[i] && !b) {
-      b = true
-    } else if (arguments[i] && b) {
-      return false
-    }
-  }
-  return b
-}
-
-/// --- Exports
-
 module.exports = {
   Client: Client,
   createClient: function createClient (options) {
-    if (typeof (options) !== 'object') { throw new TypeError('options (object) required') }
-    if (options.url && typeof (options.url) !== 'string') { throw new TypeError('options.url (string) required') }
-    if (options.socketPath && typeof (options.socketPath) !== 'string') { throw new TypeError('options.socketPath must be a string') }
-    if (!xor(options.url, options.socketPath)) { throw new TypeError('options.url ^ options.socketPath (String) required') }
-    if (!options.log) { options.log = logger }
-    if (!options.log.child) { options.log.child = function () { return options.log } }
-    if (typeof (options.log) !== 'object') { throw new TypeError('options.log must be an object') }
+    if (isObject(options) === false) throw TypeError('options (object) required')
+    if (options.url && typeof options.url !== 'string') throw TypeError('options.url (string) required')
+    if (options.socketPath && typeof options.socketPath !== 'string') throw TypeError('options.socketPath must be a string')
+    if ((options.url && options.socketPath) || !(options.url || options.socketPath)) throw TypeError('options.url ^ options.socketPath (String) required')
+    if (!options.log) options.log = logger
+    if (isObject(options.log) !== true) throw TypeError('options.log must be an object')
+    if (!options.log.child) options.log.child = function () { return options.log }
 
     return new Client(options)
   }
 }
+
+function isObject (input) {
+  return Object.prototype.toString.apply(input) === '[object Object]'
+}

test/client.test.js

@@ -331,6 +331,64 @@ tap.afterEach((done, t) => {
   })
 })
 
+tap.test('createClient', t => {
+  t.test('requires an options object', async t => {
+    const match = /options.+required/
+    t.throws(() => ldap.createClient(), match)
+    t.throws(() => ldap.createClient([]), match)
+    t.throws(() => ldap.createClient(''), match)
+    t.throws(() => ldap.createClient(42), match)
+  })
+
+  t.test('url must be a string', async t => {
+    const match = /options\.url \(string\) required/
+    t.throws(() => ldap.createClient({ url: {} }), match)
+    t.throws(() => ldap.createClient({ url: [] }), match)
+    t.throws(() => ldap.createClient({ url: 42 }), match)
+  })
+
+  t.test('socketPath must be a string', async t => {
+    const match = /options\.socketPath must be a string/
+    t.throws(() => ldap.createClient({ socketPath: {} }), match)
+    t.throws(() => ldap.createClient({ socketPath: [] }), match)
+    t.throws(() => ldap.createClient({ socketPath: 42 }), match)
+  })
+
+  t.test('cannot supply both url and socketPath', async t => {
+    t.throws(
+      () => ldap.createClient({ url: 'foo', socketPath: 'bar' }),
+      /options\.url \^ options\.socketPath \(String\) required/
+    )
+  })
+
+  t.test('must supply at least url or socketPath', async t => {
+    t.throws(
+      () => ldap.createClient({}),
+      /options\.url \^ options\.socketPath \(String\) required/
+    )
+  })
+
+  // TODO: this test is really flaky. It would be better if we could validate
+  // the options _withouth_ having to connect to a server.
+  // t.test('attaches a child function to logger', async t => {
+  //   /* eslint-disable-next-line */
+  //   let client
+  //   const logger = Object.create(require('abstract-logging'))
+  //   const socketPath = getSock()
+  //   const server = ldap.createServer()
+  //   server.listen(socketPath, () => {})
+  //   t.tearDown(() => {
+  //     client.unbind(() => server.close())
+  //   })
+
+  //   client = ldap.createClient({ socketPath, log: logger })
+  //   t.ok(logger.child)
+  //   t.true(typeof client.log.child === 'function')
+  // })
+
+  t.end()
+})
+
 tap.test('simple bind failure', function (t) {
   t.context.client.bind(BIND_DN, uuid(), function (err, res) {
     t.ok(err)