Merge pull request #2 from trooba/connection

cache connection with client instance
trooba · Jan 23, 2019 · ae761f7 · ae761f7
2 parents 2c92e3f + 80566a5
commit ae761f7
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 95 deletions.
diff --git a/README.md b/README.md
@@ -19,7 +19,6 @@ var memcachedTransport = require('trooba-memcached-transport');
 
 const client = require('trooba')
                 .use(memcachedTransport, {
-                clientId: 'dev-memcached',
                 servers: [ 
                     'memcached1.dev.myorg.com:11211',
                     'memcached2.dev.myorg.com:11211'],
@@ -42,14 +41,12 @@ console.log(retrievedValue);
     - Questions/comments can also be posted as [github issues](https://github.com/trooba/trooba-memcached-transport/issues)
 
 ## Setting up memcached client
-You can setup memcached client by configuring three properties `clientId`, `servers` and `options`. 
-* `clientid` - an unique identifier associated with memcached client.
+You can setup memcached client by configuring three properties `servers` and `options`. 
 * `servers` - location of memcached servers. Please refer to the [documentation here](https://www.npmjs.com/package/memcached#server-locations) on various ways to configure server locations.
 * `options` - various options to configure the connection. Please refere to the [documentation here](https://www.npmjs.com/package/memcached#options) for various options to configure.
 
 ```js
 const configs = {
-                clientId: '<an unique id>',
                 servers: '<location of memcached servers>',
                 options: '<options to configure the client>'               
                 };

diff --git a/index.js b/index.js
@@ -1,13 +1,15 @@
 
+const Memcached = require('memcached');
 const client = require('./lib/client');
-const connectionPool = require('./lib/connection-pool')();
 const constants = require('./lib/constants');
 const commandExecutor = require('./lib/command-executor')();
 
 module.exports = function transportFactory() {
     function transport(pipe, config) {
         pipe.on('request', (options) => {
-            const connection = connectionPool.get(config);
+            const connection = pipe.context.$connection =
+                pipe.context.$connection || config.connection ||
+                    new Memcached(config.servers, config.options);
 
             switch (options.method) {
                 case constants.TOUCH_OP:
@@ -68,7 +70,6 @@ module.exports = function transportFactory() {
                     commandExecutor.items(pipe, connection);
                     break;
                 case constants.END_OP:
-                    connectionPool.remove(config);
                     commandExecutor.end(pipe, connection);
                     break;
                 default:

diff --git a/lib/client.js b/lib/client.js
@@ -14,56 +14,56 @@ module.exports = function clientFactory(pipe) {
     }
     // touch
     members.touch = function touch(key, lifetime) {
-        return request(constants.TOUCH_OP, {key, lifetime});
+        return request(constants.TOUCH_OP, { key, lifetime });
     };
     // get
     members.get = function get(key) {
-        return request(constants.GET_OP, {key});
+        return request(constants.GET_OP, { key });
     };
     // gets
     members.gets = function gets(key) {
-        return request(constants.GETS_OP, {key});
+        return request(constants.GETS_OP, { key });
     };
     // getMulti
     members.getMulti = function getMulti(keys) {
-        return request(constants.GETMULTI_OP, {keys});
+        return request(constants.GETMULTI_OP, { keys });
     };
     // set
     members.set = function set(key, value, lifetime) {
-        return request(constants.SET_OP, {key, value, lifetime});
+        return request(constants.SET_OP, { key, value, lifetime });
     };
     // replace
     members.replace = function replace(key, value, lifetime) {
-        return request(constants.REPLACE_OP, {key, value, lifetime});
+        return request(constants.REPLACE_OP, { key, value, lifetime });
     };
 
     // add
     members.add = function add(key, value, lifetime) {
-        return request(constants.ADD_OP, {key, value, lifetime});
+        return request(constants.ADD_OP, { key, value, lifetime });
     };
     // cas
-    members.cas = function cas(key, value, lifetime, cas) {
-        return request(constants.CAS_OP, {key, value, lifetime, cas});
+    members.cas = function (key, value, lifetime, cas) {
+        return request(constants.CAS_OP, { key, value, lifetime, cas });
     };
     // append
     members.append = function append(key, value) {
-        return request(constants.APPEND_OP, {key, value});
+        return request(constants.APPEND_OP, { key, value });
     };
     // prepend
     members.prepend = function prepend(key, value) {
-        return request(constants.PREPEND_OP, {key, value});
+        return request(constants.PREPEND_OP, { key, value });
     };
     // incr
     members.incr = function incr(key, amount) {
-        return request(constants.INCR_OP, {key, amount});
+        return request(constants.INCR_OP, { key, amount });
     };
     // decr
     members.decr = function decr(key, amount) {
-        return request(constants.DECR_OP, {key, amount});
+        return request(constants.DECR_OP, { key, amount });
     };
     // del
     members.del = function del(key) {
-        return request(constants.DEL_OP, {key});
+        return request(constants.DEL_OP, { key });
     };
     // version
     members.version = function version() {

diff --git a/lib/command-executor.js b/lib/command-executor.js
@@ -107,7 +107,7 @@ module.exports = function moduleDef() {
     members.items = function items(pipe, connection) {
         connection.items((err, result) => handler(pipe, err, result));
     };
-    // end 
+    // end
     members.end = function end(pipe, connection) {
         connection.end((err, result) => handler(pipe, err, result));
     };

diff --git a/lib/connection-pool.js b/lib/connection-pool.js
diff --git a/package.json b/package.json
@@ -26,7 +26,6 @@
   },
   "homepage": "https://github.com/trooba/trooba-memcached-transport#readme",
   "devDependencies": {
-    "app-root-path": "^2.1.0",
     "async": "^2.5.0",
     "eslint": "^5.10.0",
     "eslint-config-ebay": "^1.0.0",

diff --git a/test/index.js b/test/index.js
@@ -6,6 +6,35 @@ const sinon = require('sinon');
 const memcacheTransport = require('..');
 
 describe(__filename, () => {
+    it('should create a separate connection for new client instance', async () => {
+        const memcacheMock1 = {
+            append: sinon.stub().yields(null, true)
+        };
+        const memcacheMock2 = {
+            append: sinon.stub().yields(null, true)
+        };
+        const client1 = getTroobaClient({
+            connection: memcacheMock1
+        });
+        const client2 = getTroobaClient({
+            connection: memcacheMock2
+        });
+        Assert.deepEqual(client1.connection, client2.connection);
+    });
+    it('should re-use connection stored in client instance', async () => {
+        const append = sinon.stub().yields(null, true)
+        const memcacheMock = {
+            append
+        };
+        const memcachedConfigs = {
+            connection: memcacheMock
+        };
+        const client = getTroobaClient(memcachedConfigs);
+        await client.append('test-key1', 'some-data1');
+        await client.append('test-key1', 'some-data2');
+        Assert.ok(2, append.calledTwice);
+
+    });
     it('verify touch method', async () => {
         const memcacheMock = {
             touch: sinon.stub().yields(null, true)
@@ -257,40 +286,40 @@ describe(__filename, () => {
         assertCalledOnceWithNoArgs(memcacheMock.end);
     });    
 
-    function assertCalledOnceWithNoArgs(stub){
+    function assertCalledOnceWithNoArgs(stub) {
         Assert.equal(stub.calledOnce, true);        
         Assert.equal(stub.getCall(0).args.length, 1);
-        Assert.equal(typeof(stub.getCall(0).args[0]), "function");
+        Assert.equal(typeof(stub.getCall(0).args[0]), 'function');
     }
-    function assertCalledOnceWithOneArg(stub, arg1){
+    function assertCalledOnceWithOneArg(stub, arg1) {
         Assert.equal(stub.calledOnce, true);        
         Assert.equal(stub.getCall(0).args.length, 2);
         Assert.deepEqual(stub.getCall(0).args[0], arg1);                
-        Assert.equal(typeof(stub.getCall(0).args[1]), "function");        
+        Assert.equal(typeof(stub.getCall(0).args[1]), 'function');        
     }
-    function assertCalledOnceWithTwoArgs(stub, arg1, arg2){
+    function assertCalledOnceWithTwoArgs(stub, arg1, arg2) {
         Assert.equal(stub.calledOnce, true);       
         Assert.equal(stub.getCall(0).args.length, 3); 
         Assert.deepEqual(stub.getCall(0).args[0], arg1);     
         Assert.deepEqual(stub.getCall(0).args[1], arg2);           
-        Assert.equal(typeof(stub.getCall(0).args[2]), "function");        
+        Assert.equal(typeof(stub.getCall(0).args[2]), 'function');        
     }
-    function assertCalledOnceWithThreeArgs(stub, arg1, arg2, arg3){
+    function assertCalledOnceWithThreeArgs(stub, arg1, arg2, arg3) {
         Assert.equal(stub.calledOnce, true);       
         Assert.equal(stub.getCall(0).args.length, 4); 
         Assert.deepEqual(stub.getCall(0).args[0], arg1);     
         Assert.deepEqual(stub.getCall(0).args[1], arg2);           
         Assert.deepEqual(stub.getCall(0).args[2], arg3);           
-        Assert.equal(typeof(stub.getCall(0).args[3]), "function");        
+        Assert.equal(typeof(stub.getCall(0).args[3]), 'function');        
     }
-    function assertCalledOnceWithFourArgs(stub, arg1, arg2, arg3, arg4){
+    function assertCalledOnceWithFourArgs(stub, arg1, arg2, arg3, arg4) {
         Assert.equal(stub.calledOnce, true);        
         Assert.equal(stub.getCall(0).args.length, 5);
         Assert.deepEqual(stub.getCall(0).args[0], arg1);     
         Assert.deepEqual(stub.getCall(0).args[1], arg2);           
         Assert.deepEqual(stub.getCall(0).args[2], arg3);
         Assert.deepEqual(stub.getCall(0).args[3], arg4);
-        Assert.equal(typeof(stub.getCall(0).args[4]), "function");        
+        Assert.equal(typeof(stub.getCall(0).args[4]), 'function');        
     }
 
 

diff --git a/test/unit-tests/lib/command-executor.js b/test/unit-tests/lib/command-executor.js
@@ -1,9 +1,7 @@
 
 const Assert = require('assert');
-const Trooba = require('trooba');
 const sinon = require('sinon');
-var appRoot = require('app-root-path');
-const commandExecutor = require(appRoot +'/lib/command-executor')();
+const commandExecutor = require(`${process.cwd()}/lib/command-executor`)();
 
 describe(__filename, () => {
     it('should call pipe.throw when an error occurs', async () => {

diff --git a/test/unit-tests/lib/connection-pool.js b/test/unit-tests/lib/connection-pool.js