Merge pull request #9 from ph0bos/get-client-connection-check

Check connection before returning client
ph0bos · Sep 5, 2015 · 2279bd1 · 2279bd1
2 parents aee8934 + 5d8da04
commit 2279bd1
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 73 deletions.
diff --git a/lib/GetChildrenBuilder.js b/lib/GetChildrenBuilder.js
@@ -37,7 +37,7 @@ GetChildrenBuilder.prototype.client = function(client) {
  * @method close
  */
 GetChildrenBuilder.prototype.forPath = function(path, callback) {
-  var client = Promise.promisifyAll(this.client.getClient());
+  var client = Promise.promisifyAll(this.client.getClientwithConnectionCheck());
   var services = [];
 
   var recurse = function (path) {

diff --git a/lib/ServiceDiscovery.js b/lib/ServiceDiscovery.js
@@ -65,7 +65,7 @@ ServiceDiscovery.prototype.registerService = function (callback) {
     var servicePath = [ self.basePath, self.serviceInstance.name ].join('/');
 
     self.client
-      .getClient()
+      .getClientwithConnectionCheck()
       .mkdirp(servicePath, cb);
   }
 
@@ -110,7 +110,7 @@ ServiceDiscovery.prototype.unRegisterService = function (serviceId, callback) {
   var serviceInstancePath = [ this.basePath, this.serviceInstance.name, serviceId ].join('/');
 
   this.client
-    .getClient()
+    .getClientwithConnectionCheck()
     .transaction()
     .remove(serviceInstancePath)
     .commit(callback);
@@ -151,7 +151,7 @@ ServiceDiscovery.prototype.serviceProviderBuilder = function () {
  */
 ServiceDiscovery.prototype.queryForInstances = function (serviceName, callback) {
   var servicePath = [ this.basePath, serviceName ].join('/');
-  this.client.getClient().getChildren(servicePath, null, callback);
+  this.client.getClientwithConnectionCheck().getChildren(servicePath, null, callback);
 };
 
 module.exports = ServiceDiscovery;
diff --git a/lib/ServiceProvider.js b/lib/ServiceProvider.js
@@ -84,7 +84,9 @@ ServiceProvider.prototype.getInstance = function(callback) {
 
   // List available service instances
   function getAvailableServices (cb) {
-    self.serviceDiscovery.client.getClient().getChildren(absPath, null, cb);
+    self.serviceDiscovery.client.getClientwithConnectionCheck().getChildren(absPath, null, function(err, serviceList, stat) {
+      cb(err, serviceList, stat);
+    });
   }
 
   // Select a random service instance

diff --git a/lib/Zoologist.js b/lib/Zoologist.js
@@ -21,7 +21,7 @@ var ZooKeeperError = customErr('ZooKeeperError', Error);
 util.inherits(Zoologist, EventEmitter);
 
 // Defaults
-var DEFAULT_RETRY_COUNT        = 9999;
+var DEFAULT_RETRY_COUNT        = 5;
 var DEFAULT_RETRY_WAIT_MS      = 1000;
 var DEFAULT_CONNECT_TIMEOUT_MS = 1000;
 var DEFAULT_SESSION_TIMEOUT_MS = 5000;
@@ -60,12 +60,27 @@ function newClient(connectionString, retryCount, retryWait, connectTimeout, sess
  * @param zkClient {Object} ZooKeeper client.
  */
 function Zoologist(zkClient) {
+  this.started = false;
   this.client = zkClient;
 
   this.client.on('connected', this.emit.bind(this, 'connected'));
   this.client.on('disconnected', this.emit.bind(this, 'disconnected'));
 };
 
+/**
+ * Return the ZooKeeper client.
+ *
+ * @public
+ * @method getClient
+ */
+Zoologist.prototype.getClientwithConnectionCheck = function() {
+  if (this.client.getState() !== zkClient.State.SYNC_CONNECTED) {
+    throw new ZooKeeperError('Connection Not Established');
+  }
+
+  return this.client;
+};
+
 /**
  * Return the ZooKeeper client.
  *
@@ -83,7 +98,11 @@ Zoologist.prototype.getClient = function() {
  * @method start
  */
 Zoologist.prototype.start = function() {
+  var self = this;
   this.client.connect();
+  this.client.once('connected', function () {
+    self.started = true;
+  });
 };
 
 /**
@@ -93,7 +112,7 @@ Zoologist.prototype.start = function() {
  * @method close
  */
 Zoologist.prototype.close = function() {
-  // Not implemented
+  this.started = false;
 };
 
 /**

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "zoologist",
-  "version": "0.4.3",
+  "version": "0.4.4",
   "description": "A Curator-esque framework for ZooKeeper",
   "main": "index.js",
   "keywords": [

diff --git a/test/GetChildrenBuilder.test.js b/test/GetChildrenBuilder.test.js
@@ -13,26 +13,28 @@ describe('GetChildrenBuilder', function() {
     client  = Zoologist.newClient('127.0.0.1:2181');
     client.start();
 
-    serviceInstance =
-      ServiceInstanceBuilder
-        .builder()
-        .address('localhost')
-        .port(12345)
-        .name('my/service/v2')
-        .build();
-
-    serviceDiscovery =
-      ServiceDiscoveryBuilder
-        .builder()
-        .client(client)
-        .thisInstance(serviceInstance)
-        .basePath('services')
-        .build();
-
-    serviceProvider = new ServiceProvider(serviceDiscovery, 'my/service/v2');
-
-    serviceDiscovery.registerService(function onRegister(err, data) {
-      done();
+    client.once('connected', function () {
+      serviceInstance =
+        ServiceInstanceBuilder
+          .builder()
+          .address('localhost')
+          .port(12345)
+          .name('my/service/v2')
+          .build();
+
+      serviceDiscovery =
+        ServiceDiscoveryBuilder
+          .builder()
+          .client(client)
+          .thisInstance(serviceInstance)
+          .basePath('services')
+          .build();
+
+      serviceProvider = new ServiceProvider(serviceDiscovery, 'my/service/v2');
+
+      serviceDiscovery.registerService(function onRegister(err, data) {
+        done();
+      });
     });
   });
 

diff --git a/test/ServiceDiscovery.test.js b/test/ServiceDiscovery.test.js
@@ -12,25 +12,27 @@ describe('ServiceDiscovery', function() {
     client  = Zoologist.newClient('127.0.0.1:2181');
     client.start();
 
-    builder = ServiceDiscoveryBuilder.builder();
-
-    serviceInstance =
-      ServiceInstanceBuilder
-        .builder()
-        .address('localhost')
-        .port(12345)
-        .name('my/service/v1')
-        .build();
-
-    serviceDiscovery =
-      builder
-        .client(client)
-        .thisInstance(serviceInstance)
-        .basePath('services')
-        .build();
-
-    serviceDiscovery.registerService(function onRegister(err, data) {
-      done();
+    client.once('connected', function () {
+      builder = ServiceDiscoveryBuilder.builder();
+
+      serviceInstance =
+        ServiceInstanceBuilder
+          .builder()
+          .address('localhost')
+          .port(12345)
+          .name('my/service/v1')
+          .build();
+
+      serviceDiscovery =
+        builder
+          .client(client)
+          .thisInstance(serviceInstance)
+          .basePath('services')
+          .build();
+
+      serviceDiscovery.registerService(function onRegister(err, data) {
+        done();
+      });
     });
   });
 

diff --git a/test/ServiceDiscoveryBuilder.test.js b/test/ServiceDiscoveryBuilder.test.js
@@ -8,18 +8,22 @@ var ServiceDiscoveryBuilder = require('..').ServiceDiscoveryBuilder;
 
 describe('ServiceDiscoveryBuilder', function() {
 
-  beforeEach(function(){
+  beforeEach(function(done){
     client  = Zoologist.newClient('127.0.0.1:2181');
     client.start();
 
-    builder = ServiceDiscoveryBuilder.builder();
-
-    serviceInstance =
-      ServiceInstanceBuilder
-        .builder()
-        .address('localhost')
-        .port(12345)
-        .build();
+    client.once('connected', function () {
+      builder = ServiceDiscoveryBuilder.builder();
+
+      serviceInstance =
+        ServiceInstanceBuilder
+          .builder()
+          .address('localhost')
+          .port(12345)
+          .build();
+
+      done();
+    });
   });
 
   it('should instantiate an instance of ServiceDiscoveryBuilder when calling builder()', function() {

diff --git a/test/ServiceProvider.test.js b/test/ServiceProvider.test.js
@@ -13,26 +13,28 @@ describe('ServiceProvider', function() {
     client  = Zoologist.newClient('127.0.0.1:2181');
     client.start();
 
-    serviceInstance =
-      ServiceInstanceBuilder
-        .builder()
-        .address('localhost')
-        .port(12345)
-        .name('my/service/v1')
-        .build();
+    client.once('connected', function () {
+      serviceInstance =
+        ServiceInstanceBuilder
+          .builder()
+          .address('localhost')
+          .port(12345)
+          .name('my/service/v1')
+          .build();
 
-    serviceDiscovery =
-      ServiceDiscoveryBuilder
-        .builder()
-        .client(client)
-        .thisInstance(serviceInstance)
-        .basePath('services')
-        .build();
+      serviceDiscovery =
+        ServiceDiscoveryBuilder
+          .builder()
+          .client(client)
+          .thisInstance(serviceInstance)
+          .basePath('services')
+          .build();
 
-    serviceProvider = new ServiceProvider(serviceDiscovery, 'my/service/v1');
+      serviceProvider = new ServiceProvider(serviceDiscovery, 'my/service/v1');
 
-    serviceDiscovery.registerService(function onRegister(err, data) {
-      done();
+      serviceDiscovery.registerService(function onRegister(err, data) {
+        done();
+      });
     });
   });
 

diff --git a/test/Zoologist.test.js b/test/Zoologist.test.js
@@ -18,6 +18,20 @@ describe('Zoologist', function() {
     zoologist.start();
   });
 
+  it('should be classified as started once start() has been called', function() {
+    zoologist.start();
+  });
+
+  it('should be classified as not started if the start() function has not been called', function() {
+    zoologist.getStarted().should.equal(false);
+  });
+
+  it('should be classified as not started if the close() function has been called', function() {
+    zoologist.start();
+    zoologist.close();
+    zoologist.getStarted().should.equal(false);
+  });
+
   it('should close the framework when calling close()', function() {
     zoologist.close();
   });