Fix Driver.onCompleted callback

Setting this callback will now trigger connectivity verification.
Provided callback function will be invoked if verification succeeds.
When `onCompleted` callback is not set no verification will happen.
This is achieved using getter/setter on `onCompleted` property.

This callback has previously been broken when session was decoupled
from connection.

Removed `Driver.onCompleted` and `Driver.onError` from README. These
callbacks were introduced specifically for neo4j-browser that needs to
verify connectivity and be notified about broken WebSockets. So they
were always a "special" not fully thought through part of driver's
public API. It is preferable to do connectivity verification by running
a dummy query like `RETURN 1`. Note that it's not always required.

Author: lutovich

README.md

@@ -61,23 +61,12 @@ driver.close();
 
 ## Usage examples
 
-Driver creation:
+Driver lifecycle:
 ```javascript
 // Create a driver instance, for the user neo4j with password neo4j.
 // It should be enough to have a single driver per database per application.
 var driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j"));
 
-// Register a callback to know if driver creation was successful:
-driver.onCompleted = function () {
-  // proceed with using the driver, it was successfully instantiated
-};
-
-// Register a callback to know if driver creation failed.
-// This could happen due to wrong credentials or database unavailability:
-driver.onError = function (error) {
-  console.log('Driver instantiation failed', error);
-};
-
 // Close the driver when application exits.
 // This closes all used network connections.
 driver.close();

src/v1/driver.js

@@ -24,6 +24,7 @@ import StreamObserver from './internal/stream-observer';
 import {newError, SERVICE_UNAVAILABLE} from './error';
 import {DirectConnectionProvider} from './internal/connection-providers';
 import Bookmark from './internal/bookmark';
+import ConnectivityVerifier from './internal/connectivity-verifier';
 
 const READ = 'READ', WRITE = 'WRITE';
 /**
@@ -72,6 +73,21 @@ class Driver {
      * @protected
      */
     this._connectionProvider = null;
+
+    this._onCompleted = null;
+  }
+
+  get onCompleted() {
+    return this._onCompleted;
+  }
+
+  set onCompleted(callback) {
+    this._onCompleted = callback;
+    if (this._onCompleted) {
+      const connectionProvider = this._getOrCreateConnectionProvider();
+      const connectivityVerifier = new ConnectivityVerifier(connectionProvider, this._onCompleted);
+      connectivityVerifier.verify();
+    }
   }
 
   /**
@@ -219,16 +235,6 @@ class _ConnectionStreamObserver extends StreamObserver {
       this._hasFailed = true;
     }
   }
-
-  onCompleted(message) {
-    if (this._driver.onCompleted) {
-      this._driver.onCompleted(message);
-    }
-
-    if (this._observer && this._observer.onComplete) {
-      this._observer.onCompleted(message);
-    }
-  }
 }
 
 /**

src/v1/internal/connectivity-verifier.js

@@ -0,0 +1,71 @@
+/**
+ * Copyright (c) 2002-2017 "Neo Technology,","
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import ConnectionHolder from './connection-holder';
+import {READ} from '../driver';
+import StreamObserver from './stream-observer';
+
+/**
+ * Verifies connectivity using the given connection provider.
+ */
+export default class ConnectivityVerifier {
+
+  /**
+   * @constructor
+   * @param {ConnectionProvider} connectionProvider the provider to obtain connections from.
+   * @param {function} successCallback a callback to invoke when verification succeeds.
+   */
+  constructor(connectionProvider, successCallback) {
+    this._connectionProvider = connectionProvider;
+    this._successCallback = successCallback;
+  }
+
+  verify() {
+    acquireAndReleaseDummyConnection(this._connectionProvider).then(serverInfo => {
+      if (this._successCallback) {
+        this._successCallback(serverInfo);
+      }
+    }).catch(ignoredError => {
+    });
+  }
+}
+
+/**
+ * @private
+ * @param {ConnectionProvider} connectionProvider the provider to obtain connections from.
+ * @return {Promise<object>} promise resolved with server info or rejected with error.
+ */
+function acquireAndReleaseDummyConnection(connectionProvider) {
+  const connectionHolder = new ConnectionHolder(READ, connectionProvider);
+  connectionHolder.initializeConnection();
+  const dummyObserver = new StreamObserver();
+  const connectionPromise = connectionHolder.getConnection(dummyObserver);
+
+  return connectionPromise.then(connection => {
+    // able to establish a connection
+    return connectionHolder.close().then(() => connection.server);
+  }).catch(error => {
+    // failed to establish a connection
+    return connectionHolder.close().catch(ignoredError => {
+      // ignore connection release error
+    }).then(() => {
+      return Promise.reject(error);
+    });
+  });
+}

src/v1/internal/server-version.js

@@ -91,10 +91,12 @@ function compareInts(x, y) {
   return (x < y) ? -1 : ((x === y) ? 0 : 1);
 }
 
+const VERSION_3_1_0 = new ServerVersion(3, 1, 0);
 const VERSION_3_2_0 = new ServerVersion(3, 2, 0);
 
 export {
   ServerVersion,
+  VERSION_3_1_0,
   VERSION_3_2_0
 };
 

test/internal/connectivity-verifier.test.js

@@ -0,0 +1,37 @@
+/**
+ * Copyright (c) 2002-2017 "Neo Technology,","
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import ConnectivityVerifier from '../../src/v1/internal/connectivity-verifier';
+import {SingleConnectionProvider} from '../../src/v1/internal/connection-providers';
+import FakeConnection from './fake-connection';
+
+describe('ConnectivityVerifier', () => {
+
+  it('should call success callback when able to acquire and release a connection', done => {
+    const connectionPromise = Promise.resolve(new FakeConnection());
+    const connectionProvider = new SingleConnectionProvider(connectionPromise);
+
+    const verifier = new ConnectivityVerifier(connectionProvider, () => {
+      done();
+    });
+
+    verifier.verify();
+  });
+
+});

test/types/v1/driver.test.ts

@@ -30,6 +30,7 @@ import Driver, {
 import {Parameters} from "../../../types/v1/statement-runner";
 import Session from "../../../types/v1/session";
 import {Neo4jError} from "../../../types/v1/error";
+import {ServerInfo} from "../../../types/v1/result-summary";
 
 const dummy: any = null;
 
@@ -87,11 +88,12 @@ session1.run("RETURN 1").then(result => {
 
 const close: void = driver.close();
 
-driver.onCompleted = (metadata: { server: string }) => {
-  console.log(metadata.server);
+driver.onCompleted = (serverInfo: ServerInfo) => {
+  console.log(serverInfo.version);
+  console.log(serverInfo.address);
 };
 
-driver.onCompleted({server: "Neo4j/3.2.0"});
+driver.onCompleted({version: "Neo4j/3.2.0", address: "localhost:7687"});
 
 driver.onError = (error: Neo4jError) => {
   console.log(error);

test/v1/driver.test.js

@@ -112,51 +112,43 @@ describe('driver', () => {
     driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken);
 
     // Expect
-    driver.onCompleted = meta => {
+    driver.onCompleted = server => {
+      expect(server.address).toBeDefined();
       done();
     };
-
-    // When
-    startNewTransaction(driver);
   });
 
   it('should be possible to pass a realm with basic auth tokens', done => {
     // Given
     driver = neo4j.driver("bolt://localhost", neo4j.auth.basic(sharedNeo4j.username, sharedNeo4j.password, "native"));
 
     // Expect
-    driver.onCompleted = meta => {
+    driver.onCompleted = server => {
+      expect(server.address).toBeDefined();
       done();
     };
-
-    // When
-    startNewTransaction(driver);
   });
 
   it('should be possible to create custom auth tokens', done => {
     // Given
     driver = neo4j.driver("bolt://localhost", neo4j.auth.custom(sharedNeo4j.username, sharedNeo4j.password, "native", "basic"));
 
     // Expect
-    driver.onCompleted = meta => {
+    driver.onCompleted = server => {
+      expect(server.address).toBeDefined();
       done();
     };
-
-    // When
-    startNewTransaction(driver);
   });
 
   it('should be possible to create custom auth tokens with additional parameters', done => {
     // Given
     driver = neo4j.driver("bolt://localhost", neo4j.auth.custom(sharedNeo4j.username, sharedNeo4j.password, "native", "basic", {secret: 42}));
 
     // Expect
-    driver.onCompleted = () => {
+    driver.onCompleted = server => {
+      expect(server.address).toBeDefined();
       done();
     };
-
-    // When
-    startNewTransaction(driver);
   });
 
   it('should fail nicely when connecting with routing to standalone server', done => {

test/v1/examples.test.js

@@ -102,14 +102,9 @@ describe('examples', () => {
     // end::basic-auth[]
 
     driver.onCompleted = () => {
+      driver.close();
       done();
     };
-
-    const session = driver.session();
-    session.run('RETURN 1').then(() => {
-      session.close();
-      driver.close();
-    });
   });
 
   it('config max retry time example', done => {
@@ -123,14 +118,9 @@ describe('examples', () => {
     // end::config-max-retry-time[]
 
     driver.onCompleted = () => {
+      driver.close();
       done();
     };
-
-    const session = driver.session();
-    session.run('RETURN 1').then(() => {
-      session.close();
-      driver.close();
-    });
   });
 
   it('config trust example', done => {
@@ -144,19 +134,9 @@ describe('examples', () => {
     // end::config-trust[]
 
     driver.onCompleted = () => {
+      driver.close();
       done();
     };
-
-    driver.onError = error => {
-      console.log(error);
-    };
-
-    const session = driver.session();
-    session.run('RETURN 1').then(() => {
-      session.close();
-      driver.close();
-    }).catch(error => {
-    });
   });
 
   it('config unencrypted example', done => {
@@ -169,14 +149,9 @@ describe('examples', () => {
     // end::config-unencrypted[]
 
     driver.onCompleted = () => {
+      driver.close();
       done();
     };
-
-    const session = driver.session();
-    session.run('RETURN 1').then(() => {
-      session.close();
-      driver.close();
-    });
   });
 
   it('custom auth example', done => {
@@ -191,14 +166,9 @@ describe('examples', () => {
     // end::custom-auth[]
 
     driver.onCompleted = () => {
+      driver.close();
       done();
     };
-
-    const session = driver.session();
-    session.run('RETURN 1').then(() => {
-      session.close();
-      driver.close();
-    });
   });
 
   it('kerberos auth example', () => {
@@ -239,7 +209,7 @@ describe('examples', () => {
     // tag::driver-lifecycle[]
     const driver = neo4j.driver(uri, neo4j.auth.basic(user, password));
 
-    driver.onCompleted = metadata => {
+    driver.onCompleted = () => {
       console.log('Driver created');
     };