Skip to content

Type checks for string parameters #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/v1/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ={
Expand Down Expand Up @@ -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://") {
Expand Down
49 changes: 37 additions & 12 deletions src/v1/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
18 changes: 11 additions & 7 deletions src/v1/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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 "
Expand Down
3 changes: 3 additions & 0 deletions src/v1/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}

Expand Down
66 changes: 66 additions & 0 deletions test/internal/util.test.js
Original file line number Diff line number Diff line change
@@ -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);
}

});
16 changes: 14 additions & 2 deletions test/v1/driver.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
19 changes: 19 additions & 0 deletions test/v1/session.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});


25 changes: 25 additions & 0 deletions test/v1/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down