Deprecate connectionPoolSize config setting

In favour of `maxConnectionPoolSize`. Extracted pool config in a
dedicated class which contains logic of handling now deprecated
`connectionPoolSize` and new `maxConnectionPoolSize`. Added
`maxConnectionPoolSize` to TypeScript declaration. Made code use
`console.warn` instead of `console.log` for deprecation warnings.
Updated test neo4j version to 3.2.5 and allowed stress test run a
bit longer.

Author: lutovich

src/v1/driver.js

@@ -25,6 +25,7 @@ import {newError, SERVICE_UNAVAILABLE} from './error';
 import {DirectConnectionProvider} from './internal/connection-providers';
 import Bookmark from './internal/bookmark';
 import ConnectivityVerifier from './internal/connectivity-verifier';
+import PoolConfig from './internal/pool-config';
 
 const READ = 'READ', WRITE = 'WRITE';
 /**
@@ -60,11 +61,7 @@ class Driver {
       this._createConnection.bind(this),
       this._destroyConnection.bind(this),
       this._validateConnection.bind(this),
-      {
-        maxIdleSize: config.connectionPoolSize,
-        maxSize: config.maxConnectionPoolSize,
-        acquisitionTimeout: config.connectionAcquisitionTimeout
-      }
+      PoolConfig.fromDriverConfig(config)
     );
 
     /**
@@ -243,18 +240,17 @@ class _ConnectionStreamObserver extends StreamObserver {
 function sanitizeConfig(config) {
   config.maxConnectionLifetime = sanitizeIntValue(config.maxConnectionLifetime);
   config.maxConnectionPoolSize = sanitizeIntValue(config.maxConnectionPoolSize);
-  config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout, 60000);
+  config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout);
 }
 
-function sanitizeIntValue(value, defaultValue=null) {
+function sanitizeIntValue(value) {
   if (value) {
       const sanitizedValue = parseInt(value, 10);
       if (sanitizedValue && sanitizedValue > 0) {
         return sanitizedValue;
       }
   }
-
-  return defaultValue;
+  return null;
 }
 
 export {Driver, READ, WRITE}

src/v1/index.js

@@ -90,7 +90,7 @@ const USER_AGENT = "neo4j-javascript/" + VERSION;
  *       // as trusted. In the web bundle, this list of trusted certificates is maintained
  *       // by the web browser. In NodeJS, you configure the list with the next config option.
  *       //
- *       // TRUST_SYSTEM_CA_SIGNED_CERTIFICATES meand that you trust whatever certificates
+ *       // TRUST_SYSTEM_CA_SIGNED_CERTIFICATES means that you trust whatever certificates
  *       // are in the default certificate chain of th
  *       trust: "TRUST_ALL_CERTIFICATES" | "TRUST_ON_FIRST_USE" | "TRUST_SIGNED_CERTIFICATES" |
  *       "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES" | "TRUST_SYSTEM_CA_SIGNED_CERTIFICATES",
@@ -110,6 +110,7 @@ const USER_AGENT = "neo4j-javascript/" + VERSION;
  *
  *       // The max number of connections that are allowed idle in the pool at any time.
  *       // Connection will be destroyed if this threshold is exceeded.
+ *       // <b>Deprecated:</b> please use <code>maxConnectionPoolSize</code> instead.
  *       connectionPoolSize: 50,
  *
  *       // The maximum total number of connections allowed to be managed by the connection pool, per host.

src/v1/internal/ch-node.js

@@ -108,7 +108,7 @@ const TrustStrategy = {
    * @deprecated Since version 1.0. Will be deleted in a future version. {@link #TRUST_CUSTOM_CA_SIGNED_CERTIFICATES}.
    */
   TRUST_SIGNED_CERTIFICATES: function( config, onSuccess, onFailure ) {
-    console.log("`TRUST_SIGNED_CERTIFICATES` has been deprecated as option and will be removed in a future version of " +
+    console.warn('`TRUST_SIGNED_CERTIFICATES` has been deprecated as option and will be removed in a future version of ' +
       "the driver. Please use `TRUST_CUSTOM_CA_SIGNED_CERTIFICATES` instead.");
     return TrustStrategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES(config, onSuccess, onFailure);
   },
@@ -172,7 +172,7 @@ const TrustStrategy = {
    * @deprecated in 1.1 in favour of {@link #TRUST_ALL_CERTIFICATES}. Will be deleted in a future version.
    */
   TRUST_ON_FIRST_USE : function( config, onSuccess, onFailure ) {
-      console.log("`TRUST_ON_FIRST_USE` has been deprecated as option and will be removed in a future version of " +
+    console.warn('`TRUST_ON_FIRST_USE` has been deprecated as option and will be removed in a future version of ' +
           "the driver. Please use `TRUST_ALL_CERTIFICATES` instead.");
 
     let tlsOpts = {

src/v1/internal/pool-config.js

@@ -0,0 +1,82 @@
+/**
+ * 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.
+ */
+
+const DEFAULT_SIZE = 50;
+const DEFAULT_ACQUISITION_TIMEOUT = 60000;
+
+export default class PoolConfig {
+
+  constructor(maxIdleSize, maxSize, acquisitionTimeout) {
+    this.maxIdleSize = valueOrDefault(maxIdleSize, DEFAULT_SIZE);
+    this.maxSize = valueOrDefault(maxSize, DEFAULT_SIZE);
+    this.acquisitionTimeout = valueOrDefault(acquisitionTimeout, DEFAULT_ACQUISITION_TIMEOUT);
+  }
+
+  static defaultConfig() {
+    return new PoolConfig(DEFAULT_SIZE, DEFAULT_SIZE, DEFAULT_ACQUISITION_TIMEOUT);
+  }
+
+  static fromDriverConfig(config) {
+    const maxIdleSizeConfigured = isConfigured(config.connectionPoolSize);
+    const maxSizeConfigured = isConfigured(config.maxConnectionPoolSize);
+
+    if (maxIdleSizeConfigured) {
+      console.warn('WARNING: neo4j-driver setting "connectionPoolSize" is deprecated, please use "maxConnectionPoolSize" instead');
+    }
+
+    let maxIdleSize;
+    let maxSize;
+
+    if (maxIdleSizeConfigured && maxSizeConfigured) {
+      // both settings are configured - use configured values
+      maxIdleSize = config.connectionPoolSize;
+      maxSize = config.maxConnectionPoolSize;
+    } else if (!maxIdleSizeConfigured && maxSizeConfigured) {
+      // only maxSize is configured - use it's value for both
+      maxIdleSize = config.maxConnectionPoolSize;
+      maxSize = config.maxConnectionPoolSize;
+    } else if (maxIdleSizeConfigured && !maxSizeConfigured) {
+      // only maxIdleSize is configured - use it's value for both
+      maxIdleSize = config.connectionPoolSize;
+      maxSize = config.connectionPoolSize;
+    } else {
+      // none configured - use default values
+      maxIdleSize = DEFAULT_SIZE;
+      maxSize = DEFAULT_SIZE;
+    }
+
+    const acquisitionTimeoutConfigured = isConfigured(config.connectionAcquisitionTimeout);
+    const acquisitionTimeout = acquisitionTimeoutConfigured ? config.connectionAcquisitionTimeout : DEFAULT_ACQUISITION_TIMEOUT;
+
+    return new PoolConfig(maxIdleSize, maxSize, acquisitionTimeout);
+  }
+}
+
+function valueOrDefault(value, defaultValue) {
+  return value === 0 || value ? value : defaultValue;
+}
+
+function isConfigured(value) {
+  return value === 0 || value;
+}
+
+export {
+  DEFAULT_SIZE,
+  DEFAULT_ACQUISITION_TIMEOUT
+};

src/v1/internal/pool.js

@@ -17,23 +17,22 @@
  * limitations under the License.
  */
 
-import { newError } from "../error";
-import { promiseOrTimeout } from "./util";
+import {promiseOrTimeout} from './util';
+import PoolConfig from './pool-config';
 
 class Pool {
   /**
-   * @param create  an allocation function that creates a new resource. It's given
+   * @param {function} create  an allocation function that creates a new resource. It's given
    *                a single argument, a function that will return the resource to
    *                the pool if invoked, which is meant to be called on .dispose
    *                or .close or whatever mechanism the resource uses to finalize.
-   * @param destroy called with the resource when it is evicted from this pool
-   * @param validate called at various times (like when an instance is acquired and
+   * @param {function} destroy called with the resource when it is evicted from this pool
+   * @param {function} validate called at various times (like when an instance is acquired and
    *                 when it is returned). If this returns false, the resource will
    *                 be evicted
-   * @param maxIdle the max number of resources that are allowed idle in the pool at
-   *                any time. If this threshold is exceeded, resources will be evicted.
+   * @param {PoolConfig} config configuration for the new driver.
    */
-  constructor(create, destroy=(()=>true), validate=(()=>true), config={}) {
+  constructor(create, destroy = (() => true), validate = (() => true), config = PoolConfig.defaultConfig()) {
     this._create = create;
     this._destroy = destroy;
     this._validate = validate;

test/internal/pool-config.test.js

@@ -0,0 +1,121 @@
+/**
+ * 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 PoolConfig, {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_SIZE} from '../../src/v1/internal/pool-config';
+
+describe('PoolConfig', () => {
+
+  let originalConsoleWarn;
+
+  beforeAll(() => {
+    originalConsoleWarn = console.warn;
+    console.warn = () => {
+    };
+  });
+
+  afterAll(() => {
+    console.warn = originalConsoleWarn;
+  });
+
+  it('should respect zero values', () => {
+    const config = new PoolConfig(0, 0, 0);
+
+    expect(config.maxIdleSize).toEqual(0);
+    expect(config.maxSize).toEqual(0);
+    expect(config.acquisitionTimeout).toEqual(0);
+  });
+
+  it('should expose default config', () => {
+    const config = PoolConfig.defaultConfig();
+
+    expect(config.maxIdleSize).toEqual(DEFAULT_SIZE);
+    expect(config.maxSize).toEqual(DEFAULT_SIZE);
+    expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT);
+  });
+
+  it('should convert from empty driver config', () => {
+    const driverConfig = {};
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    expect(config.maxIdleSize).toEqual(DEFAULT_SIZE);
+    expect(config.maxSize).toEqual(DEFAULT_SIZE);
+    expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT);
+  });
+
+  it('should convert from full driver config', () => {
+    const driverConfig = {
+      connectionPoolSize: 42,
+      maxConnectionPoolSize: 4242,
+      connectionAcquisitionTimeout: 424242
+    };
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    expect(config.maxIdleSize).toEqual(42);
+    expect(config.maxSize).toEqual(4242);
+    expect(config.acquisitionTimeout).toEqual(424242);
+  });
+
+  it('should convert from driver config with both connectionPoolSize and maxConnectionPoolSize', () => {
+    const driverConfig = {
+      connectionPoolSize: 42,
+      maxConnectionPoolSize: 4242
+    };
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    expect(config.maxIdleSize).toEqual(42);
+    expect(config.maxSize).toEqual(4242);
+    expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT);
+  });
+
+  it('should convert from driver config without connectionPoolSize and maxConnectionPoolSize', () => {
+    const driverConfig = {
+      connectionAcquisitionTimeout: 42
+    };
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    expect(config.maxIdleSize).toEqual(DEFAULT_SIZE);
+    expect(config.maxSize).toEqual(DEFAULT_SIZE);
+    expect(config.acquisitionTimeout).toEqual(42);
+  });
+
+  it('should convert from driver config with only connectionPoolSize', () => {
+    const driverConfig = {
+      connectionPoolSize: 42
+    };
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    // both connectionPoolSize and maxConnectionPoolSize should be set to the same value
+    expect(config.maxIdleSize).toEqual(42);
+    expect(config.maxSize).toEqual(42);
+    expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT);
+  });
+
+  it('should convert from driver config with only maxConnectionPoolSize', () => {
+    const driverConfig = {
+      maxConnectionPoolSize: 42
+    };
+    const config = PoolConfig.fromDriverConfig(driverConfig);
+
+    // both connectionPoolSize and maxConnectionPoolSize should be set to the same value
+    expect(config.maxIdleSize).toEqual(42);
+    expect(config.maxSize).toEqual(42);
+    expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT);
+  });
+
+});

test/internal/pool.test.js

@@ -18,6 +18,7 @@
  */
 
 import Pool from '../../src/v1/internal/pool';
+import PoolConfig from '../../src/v1/internal/pool-config';
 
 describe('Pool', () => {
 
@@ -114,9 +115,7 @@ describe('Pool', () => {
         destroyed.push(resource);
       },
       resource => true,
-      {
-        maxIdleSize: 2
-      }
+      new PoolConfig(2, 100, 60000)
     );
 
     // When
@@ -152,9 +151,7 @@ describe('Pool', () => {
         destroyed.push(resource);
       },
       resource => false,
-      {
-        maxIdleSize: 1000
-      }
+      new PoolConfig(1000, 1000, 60000)
     );
 
     // When
@@ -440,10 +437,7 @@ describe('Pool', () => {
         (url, release) => new Resource(url, counter++, release),
         resource => {},
         resource => true,
-        {
-          maxSize: 2,
-          acquisitionTimeout: 5000
-        }
+      new PoolConfig(2, 2, 5000)
     );
 
     const p0 = pool.acquire(key);
@@ -473,10 +467,7 @@ describe('Pool', () => {
         (url, release) => new Resource(url, counter++, release),
         resource => {},
         resource => true,
-        {
-          maxSize: 2,
-          acquisitionTimeout: 1000
-        }
+      new PoolConfig(2, 2, 1000)
     );
 
     const p0 = pool.acquire(key);

test/internal/shared-neo4j.js

@@ -95,7 +95,7 @@ const password = 'password';
 const authToken = neo4j.auth.basic(username, password);
 
 const neoCtrlVersionParam = '-e';
-const defaultNeo4jVersion = '3.2.0';
+const defaultNeo4jVersion = '3.2.5';
 const defaultNeoCtrlArgs = `${neoCtrlVersionParam} ${defaultNeo4jVersion}`;
 
 function neo4jCertPath(dir) {

test/v1/stress.test.js

@@ -30,7 +30,7 @@ describe('stress tests', () => {
     fast: {
       commandsCount: 5000,
       parallelism: 8,
-      maxRunTimeMs: 120000 // 2 minutes
+      maxRunTimeMs: 180000 // 3 minutes
     },
     extended: {
       commandsCount: 2000000,

types/v1/driver.d.ts

@@ -45,7 +45,11 @@ declare interface Config {
   trust?: TrustStrategy;
   trustedCertificates?: string[];
   knownHosts?: string;
+  /**
+   * @deprecated use {@link maxConnectionPoolSize} instead.
+   */
   connectionPoolSize?: number;
+  maxConnectionPoolSize?: number;
   maxTransactionRetryTime?: number;
   loadBalancingStrategy?: LoadBalancingStrategy;
   maxConnectionLifetime?: number;