diff --git a/src/v1/index.js b/src/v1/index.js index 8d0f919b0..07ba41bdd 100644 --- a/src/v1/index.js +++ b/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://") { diff --git a/src/v1/internal/util.js b/src/v1/internal/util.js index d68360372..0e7efdc89 100644 --- a/src/v1/internal/util.js +++ b/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 } diff --git a/src/v1/session.js b/src/v1/session.js index 747ebd93f..7c89d4e58 100644 --- a/src/v1/session.js +++ b/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 " diff --git a/src/v1/transaction.js b/src/v1/transaction.js index caacc2ca3..b29e534fe 100644 --- a/src/v1/transaction.js +++ b/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); } diff --git a/test/internal/util.test.js b/test/internal/util.test.js new file mode 100644 index 000000000..bc557e107 --- /dev/null +++ b/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); + } + +}); diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js index b2c07b9cc..a1167a61d 100644 --- a/test/v1/driver.test.js +++ b/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) { diff --git a/test/v1/session.test.js b/test/v1/session.test.js index 447fc7003..bb194882d 100644 --- a/test/v1/session.test.js +++ b/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); + }); }); diff --git a/test/v1/transaction.test.js b/test/v1/transaction.test.js index 6bf2eeb60..6d901e207 100644 --- a/test/v1/transaction.test.js +++ b/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');