Proposing minors improvements and more test coverage

README.md

@@ -1,6 +1,6 @@
 # epfl-ldap
 
-Simple wrapper to query the epfl ldap
+Simple wrapper to query the EPFL LDAP.
 
 ## Usage
 ```javascript
@@ -43,17 +43,23 @@ customLdapContext.options.modelsMapper = customModelsMapper;
 customLdapContext.users.getUserBySciper(169419, function (err, data) {
     console.log(JSON.stringify(data, null, 2));
 });
-```
 
-## Notes
 
-⚠ Carefull with the results formats (Object or Array)
+/*
+ * Custom Options
+ */
+var publicLdapContext = require('epfl-ldap')({
+        searchBase:'ou=si-idev,ou=si,o=epfl,c=ch',
+        poolSize: 4
+    });
+publicLdapContext.users.getUserBySciper(169419, function(err, data) {
+ console.log(JSON.stringify(data, null, 2));
+});
+```
 
-⚠ this library *"try"* to use ES2015 (or ES6) capabilities, don't use it with nodejs under 4.x?
 
+## Notes
 
-## TODO
+⚠ Carefull with the results formats (Object or Array)
 
-- [ ] add tests
-- [ ] add samples
-- [ ] implement query validation with the package `epfl-exceptions`
+⚠ this library *"try"* to use ES2015 (or ES6) capabilities, don't use it with nodejs under 5.x?

client.js

@@ -1,82 +1,78 @@
 'use strict';
 
-module.exports = function ldapClient(context) {
-
-    let ldap = require('ldapjs');
-    let client = ldap.createClient({
-        url: 'ldap://ldap.epfl.ch',
-        timeLimit: 1,
-        sizeLimit: 10
-    });
-
-    function cacheQuery(ldapQuery, objectFactory, modelMapper, isResultUniq, next) { 
-        let opts = {
-            filter: ldapQuery,
-            scope: 'sub'
-        };
-
-        client.search(context.options.searchBase, opts, function (err, ldapRes) {
-            let groupedObject = {};
+const _executeQueryPromise = require('./ldapjs-promise');
+const ConnectionPool = require('./connection-pool.js');
 
-            ldapRes.on('searchEntry', function (entry) {
-                if (typeof entry.json != 'undefined') {
-                    let objectIdentifier = entry.object.uniqueIdentifier;
-                    if (groupedObject[objectIdentifier] === undefined) {
-                        groupedObject[objectIdentifier] = Array();
-                    }
-                    groupedObject[objectIdentifier].push(entry.object);
-                } else {
-                    next(null, groupedObject);
+function _executeQueryInPool(pool, searchBase, ldapQuery, next) {
+    pool.add(function(client) {
+        _executeQueryPromise(client, searchBase, ldapQuery)
+            .then(
+                (data) => {
+                    next(null, data);
                 }
-            });
-            ldapRes.on('searchReference', function (referral) {
-                //console.log('referral: ' + referral.uris.join());
-            });
-            ldapRes.on('error', function (err) {
-                console.error('error: ' + err.message);
-                next(err, null);
-            });
-            ldapRes.on('timeout', function (err) {
-                console.error('error: ' + err.message);
-                next(err, null);
-            });
-            ldapRes.on('end', function () {
-                let objectsGroup = Array();
-
-                for (let userEntry in groupedObject) {
-                    if (groupedObject.hasOwnProperty(userEntry)) {
-                        if (isResultUniq) {
-                            objectsGroup = modelMapper(objectFactory(groupedObject[userEntry]));
-                        } else {
-                            objectsGroup.push(modelMapper(objectFactory(groupedObject[userEntry])));
-                        }
-                    }
+            )
+            .catch(
+                (err) => {
+                    next(err, null);
                 }
-                next(null, objectsGroup);
-            });
+            )
         });
-    }
+}
+
+function createPool(clientFactory, opts) {
+    const pool = new ConnectionPool(
+        opts.poolSize,
+        clientFactory,
+        (client) => client.unbind()
+    );
+    return pool;
+}
 
-    client.executeQuery = function(ldapQuery, objectFactory, modelMapper, isResultUniq, next) {  
-        context.memoryCache.get(ldapQuery, function (err, data) {
-            if (!err) {
-                if (data == undefined) {
-                    cacheQuery(ldapQuery, objectFactory, modelMapper, isResultUniq, function(err, data) {
-                        context.memoryCache.set(ldapQuery, data, function (err, success) {
-                            if (!err && success) {
-                                next(null, data);
+module.exports = function ldapClient(context) {
+
+    let ldap = require('ldapjs');
+    let pool = createPool(() => ldap.createClient({
+                url: 'ldap://ldap.epfl.ch',
+                timeLimit: 1,
+                sizeLimit: 10
+            }),
+            { poolSize : context.poolSize }
+    );
+
+    const client = {
+        executeQuery: function(ldapQuery, objectFactory, modelMapper, isResultUniq, next) {
+            let objectsGroup = context.memoryCache.get(ldapQuery+isResultUniq)
+            if (objectsGroup == undefined) {
+                let searchBase = context.options.searchBase;
+                let opts = {
+                    filter: ldapQuery,
+                    scope: 'sub'
+                };
+                _executeQueryInPool(pool, searchBase, opts, function(err, data) {
+
+                    let objectsGroup = Array();
+
+                    for (let userEntry in data) {
+                        if (data.hasOwnProperty(userEntry)) {
+                            if (isResultUniq) {
+                                objectsGroup = modelMapper(objectFactory(data[userEntry]));
                             } else {
-                                next({ Error: "aararrggghhh!" }, null);
+                                objectsGroup.push(modelMapper(objectFactory(data[userEntry])));
                             }
-                        });
-                    });
-                } else {
-                    next(null, data);
-                }
+                        }
+                    }
+
+                    let success = context.memoryCache.set(ldapQuery+isResultUniq, objectsGroup);
+                    if (success) {
+                        next(null, objectsGroup);
+                    } else {
+                        next({ Error: "Error setting cache" }, null);
+                    }
+                });
             } else {
-                next({ Error: "aararrggghhh!" }, null);
+                next(null, objectsGroup);
             }
-        });
+        }
     };
 
     return client;

connection-pool.js

@@ -0,0 +1,60 @@
+const debug = require('debug')('connection-pool');
+const PSemaphore = require('promise-semaphore');
+
+module.exports = class ConnectionPool {
+
+    constructor(n, create, destroy) {
+        this.roomsLength = n;
+        this.pSemaphore = new PSemaphore({rooms: this.roomsLength});
+        this.create = create;
+        this.destroy = destroy;
+        this.rooms = [
+            //{ free: false, client: null}
+        ];
+    }
+
+    add(f) {
+        return this.pSemaphore.add(async () => {
+            debug("GET");
+            let client = this._get();
+            try {
+                return await f(client);
+            } finally {
+                debug("PUT");
+                this._put(client);
+            }
+        });
+    }
+
+    _get() {
+        for(let i=0; i < this.roomsLength; i++) {
+            if (!this.rooms[i]) {
+                this.rooms[i] = {
+                    free: true,
+                    client: this.create()
+                }
+            }
+            // Room is furnished
+            if (this.rooms[i].free) {
+                this.rooms[i].free = false;
+                return this.rooms[i].client;
+            } else {
+                // continue
+            }
+        }
+    }
+
+    _put(client) {
+        this.rooms.forEach(room => {
+            if (room.client === client) {
+                room.free = true;
+            }
+        })
+    }
+
+    close() {
+        this.rooms.forEach(room => {
+            this.destroy(room.client);
+        })
+    }
+}

context.js

@@ -1,17 +1,25 @@
 'use strict';
+let NodeCache = require("node-cache");
 module.exports = function ldapContext(options) {
-    let NodeCache = require("node-cache");
     let context = {};
+    // Define the pool size
+    if (options == undefined || options.poolSize == undefined ) {
+        context.poolSize = 3;
+    } else {
+        context.poolSize = options.poolSize;
+    }
+    context.options = require('./options')(options);
     context.client = require('./client')(context);
-    context.options = require('./options')();
     context.users = require('./repositories/users')(context);
     context.units = require('./repositories/units')(context);
     context.viewModelsMappers = require('./viewModels/mappers')();
 
-    if (options == undefined || options.memoryCache == undefined ) {
-        context.memoryCache = new NodeCache({ stdTTL: 14400 }); // 4 hour of cache
-    } else {
-        context.memoryCache = new NodeCache(options.memoryCache);
+    if (context.memoryCache == undefined) {
+        if (options == undefined || options.memoryCache == undefined ) {
+            context.memoryCache = new NodeCache({ stdTTL: 14400 }); // 4 hour of cache
+        } else {
+            context.memoryCache = new NodeCache(options.memoryCache);
+        }
     }
     return context;
 };

ldapjs-promise.js

@@ -0,0 +1,32 @@
+module.exports = function _executeQueryPromise(client, searchBase, opts) { 
+    return new Promise((resolve, reject) => {
+        client.search(searchBase, opts, function (err, ldapRes) {
+            let groupedObject = {};
+            ldapRes.on('searchEntry', function (entry) {
+                if (typeof entry.json != 'undefined') {
+                    let objectIdentifier = entry.object.uniqueIdentifier;
+                    if (groupedObject[objectIdentifier] === undefined) {
+                        groupedObject[objectIdentifier] = Array();
+                    }
+                    groupedObject[objectIdentifier].push(entry.object);
+                } else {
+                    resolve(groupedObject);
+                }
+            });
+            ldapRes.on('searchReference', function (referral) {
+                //console.log('referral: ' + referral.uris.join());
+            });
+            ldapRes.on('error', function (err) {
+                console.error('error: ' + err.message);
+                reject(err);
+            });
+            ldapRes.on('timeout', function (err) {
+                console.error('error: ' + err.message);
+                reject(err);
+            });
+            ldapRes.on('end', function () {
+                resolve(groupedObject);
+            });
+        });
+    });
+}

models/user.js

@@ -10,9 +10,8 @@ module.exports = function User(ldapUserArray) {
     userModel.accreds = Array();
     userModel.memberOf = Array();
     userModel.photoUrl = 'http://people.epfl.ch/cgi-bin/people/getPhoto?id=' + userModel.sciper;
-    
+
     if (ldapUserArray[0].memberOf !== undefined) {
-
         // Note: if only one group, typeof string
         //       if groups, typeof object.
         //       Username banla have only one group
@@ -25,13 +24,12 @@ module.exports = function User(ldapUserArray) {
             userModel.memberOf = ldapUserArray[0].memberOf;
         }
     }
-    
+
     ldapUserArray.map(function (userEntry) {
         if (userEntry.mail != undefined) {
             if (userEntry.mail instanceof Array) {
-                userEntry.mail.map(function(mail) {
-                    userModel.emails.push(mail);
-                });
+                // This remove duplicated entries and allow more than one email
+                userModel.emails = [...new Set(userEntry.mail)];
             } else {
                 userModel.emails.push(userEntry.mail);
             }
@@ -49,8 +47,8 @@ module.exports = function User(ldapUserArray) {
             }
         );
     });
-    
-    //All ldap properties
+
+    // All ldap properties
     userModel.optionalProperties = ldapUserArray;
 
     return userModel;

options.js

@@ -1,7 +1,12 @@
 'use strict';
-module.exports = function () {
+module.exports = function (options) {
     let ldapOptions = {};
-    ldapOptions.searchBase = 'c=ch';
+    // Define the searchBase
+    if (options == undefined || options.searchBase == undefined ) {
+        ldapOptions.searchBase = 'c=ch';
+    } else {
+        ldapOptions.searchBase = options.searchBase;
+    }
     ldapOptions.modelsMapper = require('./viewModels/public/modelsMapper')();
     return ldapOptions;
 };