From 5a31abaed0941e1bfd333917a9ab49d15d1712c3 Mon Sep 17 00:00:00 2001
From: James Sumners <james@sumners.email>
Date: Sun, 29 Sep 2019 08:34:59 -0400
Subject: [PATCH] Clean up options checking and add some tests

---
 lib/client/index.js | 36 ++++++++++------------------
 test/client.test.js | 58 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 24 deletions(-)

diff --git a/lib/client/index.js b/lib/client/index.js
index 40f53cc..0c23052 100644
--- a/lib/client/index.js
+++ b/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]'
+}
diff --git a/test/client.test.js b/test/client.test.js
index 96b2159..0111b42 100644
--- a/test/client.test.js
+++ b/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)