Merge pull request #196 from lutovich/1.1-str-type-checks

Type checks for string parameters

Author: lutovich

src/v1/index.js

@@ -27,6 +27,7 @@ import {Driver, READ, WRITE} from './driver';
 import RoutingDriver from './routing-driver';
 import VERSION from '../version';
 import {parseScheme, parseUrl} from "./internal/connector";
+import {assertString} from "./internal/util";
 
 
 const auth ={
@@ -107,7 +108,8 @@ let USER_AGENT = "neo4j-javascript/" + VERSION;
  * @returns {Driver}
  */
 function driver(url, authToken, config = {}) {
-  let scheme = parseScheme(url);
+  assertString(url, 'Bolt URL');
+  const scheme = parseScheme(url);
   if (scheme === "bolt+routing://") {
     return new RoutingDriver(parseUrl(url), USER_AGENT, authToken, config);
   } else if (scheme === "bolt://") {

src/v1/internal/util.js

@@ -20,22 +20,47 @@
 const ENCRYPTION_ON = "ENCRYPTION_ON";
 const ENCRYPTION_OFF = "ENCRYPTION_OFF";
 
-function isEmptyObjectOrNull(object) {
-    if (!object) {
-        return true;
-    }
+function isEmptyObjectOrNull(obj) {
+  if (isNull(obj)) {
+    return true;
+  }
+
+  if (!isObject(obj)) {
+    return false;
+  }
 
-    for (let prop in object) {
-        if (object.hasOwnProperty(prop)) {
-            return false;
-        }
+  for (let prop in obj) {
+    if (obj.hasOwnProperty(prop)) {
+      return false;
     }
+  }
 
-    return true;
+  return true;
+}
+
+function isNull(obj) {
+  return obj === null;
+}
+
+function isObject(obj) {
+  const type = typeof obj;
+  return type === 'function' || type === 'object' && Boolean(obj);
+}
+
+function assertString(obj, objName) {
+  if (!isString(obj)) {
+    throw new TypeError(objName + ' expected to be string but was: ' + JSON.stringify(obj));
+  }
+  return obj;
+}
+
+function isString(str) {
+  return Object.prototype.toString.call(str) === '[object String]';
 }
 
 export {
-    isEmptyObjectOrNull,
-    ENCRYPTION_ON,
-    ENCRYPTION_OFF
+  isEmptyObjectOrNull,
+  assertString,
+  ENCRYPTION_ON,
+  ENCRYPTION_OFF
 }

src/v1/session.js

@@ -16,13 +16,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-import StreamObserver from './internal/stream-observer';
-import Result from './result';
-import Transaction from './transaction';
-import Integer from "./integer";
-import {int} from "./integer";
+import StreamObserver from "./internal/stream-observer";
+import Result from "./result";
+import Transaction from "./transaction";
 import {newError} from "./error";
+import {assertString} from "./internal/util";
 
 /**
   * A Session instance is used for handling the connection and
@@ -55,7 +53,9 @@ class Session {
       parameters = statement.parameters || {};
       statement = statement.text;
     }
-    let streamObserver = new _RunObserver(this._onRunFailure());
+    assertString(statement, "Cypher statement");
+
+    const streamObserver = new _RunObserver(this._onRunFailure());
     if (!this._hasTx) {
       this._connectionPromise.then((conn) => {
         streamObserver.resolveConnection(conn);
@@ -80,6 +80,10 @@ class Session {
    * @returns {Transaction} - New Transaction
    */
   beginTransaction(bookmark) {
+    if (bookmark) {
+      assertString(bookmark, "Bookmark");
+    }
+
     if (this._hasTx) {
       throw newError("You cannot begin a transaction on a session with an "
       + "open transaction; either run from within the transaction or use a "

src/v1/transaction.js

@@ -18,6 +18,7 @@
  */
 import StreamObserver from './internal/stream-observer';
 import Result from './result';
+import {assertString} from './internal/util';
 
 /**
  * Represents a transaction in the Neo4j database.
@@ -65,6 +66,8 @@ class Transaction {
       parameters = statement.parameters || {};
       statement = statement.text;
     }
+    assertString(statement, "Cypher statement");
+
     return this._state.run(this._connectionPromise,  new _TransactionStreamObserver(this), statement, parameters);
   }
 

test/internal/util.test.js

@@ -0,0 +1,66 @@
+/**
+ * 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 util = require('../../lib/v1/internal/util.js');
+
+describe('util', () => {
+
+  it('should check empty objects', () => {
+    expect(util.isEmptyObjectOrNull(null)).toBeTruthy();
+    expect(util.isEmptyObjectOrNull({})).toBeTruthy();
+    expect(util.isEmptyObjectOrNull([])).toBeTruthy();
+
+    const func = () => {
+      return 42;
+    };
+    expect(util.isEmptyObjectOrNull(func)).toBeTruthy();
+    func.foo = 'bar';
+    expect(util.isEmptyObjectOrNull(func)).toBeFalsy();
+
+    expect(util.isEmptyObjectOrNull()).toBeFalsy();
+    expect(util.isEmptyObjectOrNull(undefined)).toBeFalsy();
+    expect(util.isEmptyObjectOrNull(0)).toBeFalsy();
+    expect(util.isEmptyObjectOrNull('')).toBeFalsy();
+    expect(util.isEmptyObjectOrNull('abc')).toBeFalsy();
+    expect(util.isEmptyObjectOrNull({foo: 'bar'})).toBeFalsy();
+  });
+
+  it('should check strings', () => {
+    verifyValidString('');
+    verifyValidString(new String('foo'));
+    verifyValidString(String('foo'));
+    verifyValidString("hi!");
+
+    verifyInvalidString({});
+    verifyInvalidString({foo: 1});
+    verifyInvalidString([]);
+    verifyInvalidString(['1']);
+    verifyInvalidString([1, '2']);
+    verifyInvalidString(console.log);
+  });
+
+  function verifyValidString(str) {
+    expect(util.assertString(str, 'Test string')).toBe(str);
+  }
+
+  function verifyInvalidString(str) {
+    expect(() => util.assertString(str, 'Test string')).toThrowError(TypeError);
+  }
+
+});

test/v1/driver.test.js

@@ -56,8 +56,20 @@ describe('driver', function() {
     driver.session();
   });
 
-  it('should handle wrong scheme ', function() {
-    expect(function(){neo4j.driver("tank://localhost", neo4j.auth.basic("neo4j", "neo4j"))}).toThrow(new Error("Unknown scheme: tank://"));
+  it('should handle wrong scheme', () => {
+    expect(() => neo4j.driver("tank://localhost", neo4j.auth.basic("neo4j", "neo4j")))
+      .toThrow(new Error("Unknown scheme: tank://"));
+  });
+
+  it('should handle URL parameter string', () => {
+    expect(() => neo4j.driver({uri: 'bolt://localhost'})).toThrowError(TypeError);
+
+    expect(() => neo4j.driver(['bolt:localhost'])).toThrowError(TypeError);
+
+    expect(() => {
+      const driver = neo4j.driver(String('bolt://localhost', neo4j.auth.basic("neo4j", "neo4j")));
+      return driver.session();
+    }).toBeDefined();
   });
 
   it('should fail early on wrong credentials', function(done) {

test/v1/session.test.js

@@ -356,6 +356,25 @@ describe('session', function () {
         done();
       })
   });
+
+  it('should fail nicely for illegal statement', () => {
+    expect(() => session.run()).toThrowError(TypeError);
+    expect(() => session.run(null)).toThrowError(TypeError);
+    expect(() => session.run({})).toThrowError(TypeError);
+    expect(() => session.run(42)).toThrowError(TypeError);
+    expect(() => session.run([])).toThrowError(TypeError);
+    expect(() => session.run(['CREATE ()'])).toThrowError(TypeError);
+
+    expect(() => session.run({statement: 'CREATE ()'})).toThrowError(TypeError);
+    expect(() => session.run({cypher: 'CREATE ()'})).toThrowError(TypeError);
+  });
+
+  it('should fail nicely for illegal bookmark', () => {
+    expect(() => session.beginTransaction({})).toThrowError(TypeError);
+    expect(() => session.beginTransaction(42)).toThrowError(TypeError);
+    expect(() => session.beginTransaction([])).toThrowError(TypeError);
+    expect(() => session.beginTransaction(['bookmark'])).toThrowError(TypeError);
+  });
 });
 
 

test/v1/transaction.test.js

@@ -374,6 +374,31 @@ describe('transaction', () => {
       });
   });
 
+  it('should fail nicely for illegal statement', () => {
+    const tx = session.beginTransaction();
+
+    expect(() => tx.run()).toThrowError(TypeError);
+    expect(() => tx.run(null)).toThrowError(TypeError);
+    expect(() => tx.run({})).toThrowError(TypeError);
+    expect(() => tx.run(42)).toThrowError(TypeError);
+    expect(() => tx.run([])).toThrowError(TypeError);
+    expect(() => tx.run(['CREATE ()'])).toThrowError(TypeError);
+
+    expect(() => tx.run({statement: 'CREATE ()'})).toThrowError(TypeError);
+    expect(() => tx.run({cypher: 'CREATE ()'})).toThrowError(TypeError);
+  });
+
+  it('should accept a statement object ', done => {
+    const tx = session.beginTransaction();
+    const statement = {text: "RETURN 1 AS a"};
+
+    tx.run(statement).then(result => {
+      expect(result.records.length).toBe(1);
+      expect(result.records[0].get('a').toInt()).toBe(1);
+      done();
+    }).catch(console.log);
+  });
+
   function expectSyntaxError(error) {
     const code = error.fields[0].code;
     expect(code).toBe('Neo.ClientError.Statement.SyntaxError');