From 021e9a58a181b03d67c7d6e40258dcb97bf8532b Mon Sep 17 00:00:00 2001 From: lutovich Date: Sat, 3 Jun 2017 14:56:08 +0200 Subject: [PATCH 01/13] Move types test to ES6 --- test/v1/types.test.js | 172 +++++++++++++++++++++--------------------- 1 file changed, 88 insertions(+), 84 deletions(-) diff --git a/test/v1/types.test.js b/test/v1/types.test.js index 4d6461b57..fc523aa26 100644 --- a/test/v1/types.test.js +++ b/test/v1/types.test.js @@ -17,133 +17,137 @@ * limitations under the License. */ -var neo4j = require("../../lib/v1"); -var sharedNeo4j = require("../internal/shared-neo4j").default; - -describe('floating point values', function() { - it('should support float 1.0 ', testVal( 1 ) ); - it('should support float 0.0 ', testVal( 0.0 ) ); - it('should support pretty big float ', testVal( 3.4028235e+38 ) ); // Max 32-bit - it('should support really big float ', testVal( 1.7976931348623157e+308 ) ); // Max 64-bit - it('should support pretty small float ', testVal( 1.4e-45 ) ); // Min 32-bit - it('should support really small float ', testVal( 4.9e-324 ) ); // Min 64-bit +import neo4j from '../../src/v1'; +import sharedNeo4j from '../internal/shared-neo4j'; + +describe('floating point values', () => { + it('should support float 1.0 ', testValue(1)); + it('should support float 0.0 ', testValue(0.0)); + it('should support pretty big float ', testValue(3.4028235e+38)); // Max 32-bit + it('should support really big float ', testValue(1.7976931348623157e+308)); // Max 64-bit + it('should support pretty small float ', testValue(1.4e-45)); // Min 32-bit + it('should support really small float ', testValue(4.9e-324)); // Min 64-bit }); -describe('integer values', function() { - it('should support integer 1 ', testVal( neo4j.int(1) ) ); - it('should support integer 0 ', testVal( neo4j.int(0) ) ); - it('should support integer -1 ', testVal( neo4j.int(-1) ) ); - it('should support integer larger than JS Numbers can model', testVal( neo4j.int("0x7fffffffffffffff") ) ); - it('should support integer smaller than JS Numbers can model', testVal( neo4j.int("0x8000000000000000") ) ); +describe('integer values', () => { + it('should support integer 1 ', testValue(neo4j.int(1))); + it('should support integer 0 ', testValue(neo4j.int(0))); + it('should support integer -1 ', testValue(neo4j.int(-1))); + it('should support integer larger than JS Numbers can model', testValue(neo4j.int('0x7fffffffffffffff'))); + it('should support integer smaller than JS Numbers can model', testValue(neo4j.int('0x8000000000000000'))); }); -describe('boolean values', function() { - it('should support true ', testVal( true ) ); - it('should support false ', testVal( false ) ); +describe('boolean values', () => { + it('should support true ', testValue(true)); + it('should support false ', testValue(false)); }); -describe('string values', function() { - it('should support empty string ', testVal( "" ) ); - it('should support simple string ', testVal( "abcdefghijklmnopqrstuvwxyz" ) ); - it('should support awesome string ', testVal( "All makt åt Tengil, vår befriare." ) ); +describe('string values', () => { + it('should support empty string ', testValue('')); + it('should support simple string ', testValue('abcdefghijklmnopqrstuvwxyz')); + it('should support awesome string ', testValue('All makt åt Tengil, vår befriare.')); }); -describe('list values', function() { - it('should support empty lists ', testVal( [] ) ); - it('should support sparse lists ', testVal( [ undefined, 4 ], [ null, 4 ] ) ); - it('should support float lists ', testVal( [ 1,2,3 ] ) ); - it('should support boolean lists ', testVal( [ true, false ] ) ); - it('should support string lists ', testVal( [ "", "hello!" ] ) ); - it('should support list lists ', testVal( [ [], [1,2,3] ] ) ); - it('should support map lists ', testVal( [ {}, {a:12} ] ) ); +describe('list values', () => { + it('should support empty lists ', testValue([])); + it('should support sparse lists ', testValue([undefined, 4], [null, 4])); + it('should support float lists ', testValue([1, 2, 3])); + it('should support boolean lists ', testValue([true, false])); + it('should support string lists ', testValue(['', 'hello!'])); + it('should support list lists ', testValue([[], [1, 2, 3]])); + it('should support map lists ', testValue([{}, {a: 12}])); }); -describe('map values', function() { - it('should support empty maps ', testVal( {} ) ); - it('should support basic maps ', testVal( {a:1, b:{}, c:[], d:{e:1}} ) ); - it('should support sparse maps ', testVal( {foo: undefined, bar: null}, {bar: null} ) ); +describe('map values', () => { + it('should support empty maps ', testValue({})); + it('should support basic maps ', testValue({a: 1, b: {}, c: [], d: {e: 1}})); + it('should support sparse maps ', testValue({foo: undefined, bar: null}, {bar: null})); }); -describe('node values', function() { - it('should support returning nodes ', function(done) { +describe('node values', () => { + it('should support returning nodes ', done => { // Given - var driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken); - var session = driver.session(); + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const session = driver.session(); // When - session.run("CREATE (n:User {name:'Lisa'}) RETURN n, id(n)").then(function(result) { - var node = result.records[0].get('n'); + session.run('CREATE (n:User {name:\'Lisa\'}) RETURN n, id(n)').then(result => { + const node = result.records[0].get('n'); - expect( node.properties ).toEqual( { name:"Lisa" } ); - expect( node.labels ).toEqual( ["User"] ); - // expect( node.identity ).toEqual( rs[0]['id(n)'] ); // TODO - driver.close(); - done(); + expect(node.properties).toEqual({name: 'Lisa'}); + expect(node.labels).toEqual(['User']); + expect(node.identity).toEqual(result.records[0].get('id(n)')); + driver.close(); + done(); - }); + }); }); }); -describe('relationship values', function() { - it('should support returning relationships', function(done) { +describe('relationship values', () => { + it('should support returning relationships', done => { // Given - var driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken); - var session = driver.session(); + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const session = driver.session(); // When - session.run("CREATE ()-[r:User {name:'Lisa'}]->() RETURN r, id(r)").then(function(result) { - var rel = result.records[0].get('r'); + session.run('CREATE ()-[r:User {name:\'Lisa\'}]->() RETURN r, id(r)').then(result => { + const rel = result.records[0].get('r'); - expect( rel.properties ).toEqual( { name:"Lisa" } ); - expect( rel.type ).toEqual( "User" ); - // expect( rel.identity ).toEqual( rs[0]['id(r)'] ); // TODO - driver.close(); - done(); + expect(rel.properties).toEqual({name: 'Lisa'}); + expect(rel.type).toEqual('User'); + expect(rel.identity).toEqual(result.records[0].get('id(r)')); + driver.close(); + done(); - }); + }); }); }); -describe('path values', function() { - it('should support returning paths', function(done) { +describe('path values', () => { + it('should support returning paths', done => { // Given - var driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken); - var session = driver.session(); + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const session = driver.session(); // When - session.run("CREATE p=(:User { name:'Lisa' })<-[r:KNOWS {since:1234.0}]-() RETURN p") - .then(function(result) { - var path = result.records[0].get('p'); + session.run('CREATE p=(:User { name:\'Lisa\' })<-[r:KNOWS {since:1234.0}]-() RETURN p') + .then(result => { + const path = result.records[0].get('p'); - expect( path.start.properties ).toEqual( { name:"Lisa" } ); - expect( path.end.properties ).toEqual( { } ); + expect(path.start.properties).toEqual({name: 'Lisa'}); + expect(path.end.properties).toEqual({}); // Accessing path segments - expect( path.length ).toEqual( 1 ); - for (var i = 0; i < path.length; i++) { - var segment = path.segments[i]; + expect(path.length).toEqual(1); + for (let i = 0; i < path.length; i++) { + const segment = path.segments[i]; // The direction of the path segment goes from lisa to the blank node - expect( segment.start.properties ).toEqual( { name:"Lisa" } ); - expect( segment.end.properties ).toEqual( { } ); + expect(segment.start.properties).toEqual({name: 'Lisa'}); + expect(segment.end.properties).toEqual({}); // Which is the inverse of the relationship itself! - expect( segment.relationship.properties ).toEqual( { since: 1234 } ); + expect(segment.relationship.properties).toEqual({since: 1234}); } driver.close(); done(); - }).catch(function(err) { console.log(err); }); + }).catch(err => { + console.log(err); + }); }); }); -function testVal( val, expected ) { - return function( done ) { - var driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken); - var session = driver.session(); +function testValue(actual, expected) { + return done => { + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const session = driver.session(); - session.run("RETURN {val} as v", {val: val}) - .then( function( result ) { - expect( result.records[0].get('v') ).toEqual( expected || val ); + session.run('RETURN {val} as v', {val: actual}) + .then(result => { + expect(result.records[0].get('v')).toEqual(expected || actual); driver.close(); done(); - }).catch(function(err) { console.log(err); }); - } + }).catch(err => { + console.log(err); + }); + }; } From d650cbfbbb41a9e5d080283d566703604c4b03ab Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 14 Jun 2017 10:15:13 +0200 Subject: [PATCH 02/13] Byte arrays support This commit makes driver handle writing and reading of byte arrays. They are represented as instances of Int8Array class. Driver will fail-fast when server does not support byte arrays and will not even attempt to send the given array. --- src/v1/internal/connection-providers.js | 24 ++++- src/v1/internal/connector.js | 16 +++- src/v1/internal/packstream.js | 60 +++++++++++- test/internal/shared-neo4j.js | 2 +- test/v1/types.test.js | 121 ++++++++++++++++++++++-- 5 files changed, 201 insertions(+), 22 deletions(-) diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index b9b633011..b124c2892 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -54,8 +54,7 @@ export class DirectConnectionProvider extends ConnectionProvider { } acquireConnection(mode) { - const connection = this._connectionPool.acquire(this._address); - const connectionPromise = Promise.resolve(connection); + const connectionPromise = acquireConnectionFromPool(this._connectionPool, this._address); return this._withAdditionalOnErrorCallback(connectionPromise, this._driverOnErrorCallback); } } @@ -102,7 +101,7 @@ export class LoadBalancer extends ConnectionProvider { `Failed to obtain connection towards ${serverName} server. Known routing table is: ${this._routingTable}`, SESSION_EXPIRED)); } - return this._connectionPool.acquire(address); + return acquireConnectionFromPool(this._connectionPool, address); } _freshRoutingTable(accessMode) { @@ -199,10 +198,9 @@ export class LoadBalancer extends ConnectionProvider { } _createSessionForRediscovery(routerAddress) { - const connection = this._connectionPool.acquire(routerAddress); // initialized connection is required for routing procedure call // server version needs to be known to decide which routing procedure to use - const initializedConnectionPromise = connection.initializationCompleted(); + const initializedConnectionPromise = acquireConnectionFromPool(this._connectionPool, routerAddress); const connectionProvider = new SingleConnectionProvider(initializedConnectionPromise); return new Session(READ, connectionProvider); } @@ -263,3 +261,19 @@ export class SingleConnectionProvider extends ConnectionProvider { return connectionPromise; } } + +// todo: test that all connection providers return initialized connections + +/** + * Acquire an initialized connection from the given connection pool for the given address. Returned connection + * promise will be resolved by a connection which completed initialization, i.e. received a SUCCESS response + * for it's INIT message. + * @param {Pool} connectionPool the connection pool to acquire connection from. + * @param {string} address the server address. + * @return {Promise} the initialized connection. + */ +function acquireConnectionFromPool(connectionPool, address) { + const connection = connectionPool.acquire(address); + // initialized connection is required to be able to perform subsequent server version checks + return connection.initializationCompleted(); +} diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 083c8947b..2e5848ad0 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -27,6 +27,7 @@ import {newError} from './../error'; import ChannelConfig from './ch-config'; import {parseHost, parsePort} from './util'; import StreamObserver from './stream-observer'; +import {ServerVersion, VERSION_3_2_0} from './server-version'; let Channel; if( NodeChannel.available ) { @@ -472,8 +473,17 @@ class Connection { return this._packer.packable(value, (err) => this._handleFatalError(err)); } - setServerVersion(version) { - this.server.version = version; + /** + * @protected + */ + _markInitialized(metadata) { + const serverVersion = metadata.server; + if (!this.server.version) { + this.server.version = serverVersion; + if (ServerVersion.fromString(serverVersion).compareTo(VERSION_3_2_0) < 0) { + this._packer.disableByteArrays(); + } + } } } @@ -524,7 +534,7 @@ class ConnectionState { }, onCompleted: metaData => { if (metaData && metaData.server) { - this._connection.setServerVersion(metaData.server); + this._connection._markInitialized(metaData); } this._initialized = true; if (this._resolvePromise) { diff --git a/src/v1/internal/packstream.js b/src/v1/internal/packstream.js index 7e3f0f3bb..fed7c9438 100644 --- a/src/v1/internal/packstream.js +++ b/src/v1/internal/packstream.js @@ -16,9 +16,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import utf8 from "./utf8"; -import Integer, {int, isInt} from "../integer"; -import {newError} from "./../error"; +import utf8 from './utf8'; +import Integer, {int, isInt} from '../integer'; +import {newError} from './../error'; const TINY_STRING = 0x80; const TINY_LIST = 0x90; @@ -38,6 +38,9 @@ const STRING_32 = 0xD2; const LIST_8 = 0xD4; const LIST_16 = 0xD5; const LIST_32 = 0xD6; +const BYTES_8 = 0xCC; +const BYTES_16 = 0xCD; +const BYTES_32 = 0xCE; const MAP_8 = 0xD8; const MAP_16 = 0xD9; const MAP_32 = 0xDA; @@ -74,6 +77,7 @@ class Structure { class Packer { constructor (channel) { this._ch = channel; + this._byteArraysSupported = true; } /** @@ -95,6 +99,8 @@ class Packer { return () => this.packString(x, onError); } else if (isInt(x)) { return () => this.packInteger( x ); + } else if (x instanceof Int8Array) { + return () => this.packBytes(x, onError); } else if (x instanceof Array) { return () => { this.packListHeader(x.length, onError); @@ -225,6 +231,36 @@ class Packer { } } + packBytes(array, onError) { + if(this._byteArraysSupported) { + this.packBytesHeader(array.length, onError); + for (let i = 0; i < array.length; i++) { + this._ch.writeInt8(array[i]); + } + }else { + onError(newError("Byte arrays are not supported by the database this driver is connected to")); + } + } + + packBytesHeader(size, onError) { + if (size < 0x100) { + this._ch.writeUInt8(BYTES_8); + this._ch.writeUInt8(size); + } else if (size < 0x10000) { + this._ch.writeUInt8(BYTES_16); + this._ch.writeUInt8((size / 256 >> 0) % 256); + this._ch.writeUInt8(size % 256); + } else if (size < 0x100000000) { + this._ch.writeUInt8(BYTES_32); + this._ch.writeUInt8((size / 16777216 >> 0) % 256); + this._ch.writeUInt8((size / 65536 >> 0) % 256); + this._ch.writeUInt8((size / 256 >> 0) % 256); + this._ch.writeUInt8(size % 256); + } else { + onError(newError('Byte arrays of size ' + size + ' are not supported')); + } + } + packMapHeader (size, onError) { if (size < 0x10) { this._ch.writeUInt8(TINY_MAP | size); @@ -262,6 +298,10 @@ class Packer { onError(newError("Structures of size " + size + " are not supported")); } } + + disableByteArrays() { + this._byteArraysSupported = false; + } } /** @@ -284,6 +324,14 @@ class Unpacker { return value; } + unpackBytes(size, buffer) { + const value = new Int8Array(size); + for (let i = 0; i < size; i++) { + value[i] = buffer.readInt8(); + } + return value; + } + unpackMap (size, buffer) { let value = {}; for(let i = 0; i < size; i++) { @@ -344,6 +392,12 @@ class Unpacker { return this.unpackList(buffer.readUInt16(), buffer); } else if (marker == LIST_32) { return this.unpackList(buffer.readUInt32(), buffer); + } else if (marker == BYTES_8) { + return this.unpackBytes(buffer.readUInt8(), buffer); + } else if (marker == BYTES_16) { + return this.unpackBytes(buffer.readUInt16(), buffer); + } else if (marker == BYTES_32) { + return this.unpackBytes(buffer.readUInt32(), buffer); } else if (marker == MAP_8) { return this.unpackMap(buffer.readUInt8(), buffer); } else if (marker == MAP_16) { diff --git a/test/internal/shared-neo4j.js b/test/internal/shared-neo4j.js index c3e23d95f..17148d7ae 100644 --- a/test/internal/shared-neo4j.js +++ b/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.1.3'; +const defaultNeo4jVersion = '3.2.0'; const defaultNeoCtrlArgs = `${neoCtrlVersionParam} ${defaultNeo4jVersion}`; function neo4jCertPath(dir) { diff --git a/test/v1/types.test.js b/test/v1/types.test.js index fc523aa26..b33d038ba 100644 --- a/test/v1/types.test.js +++ b/test/v1/types.test.js @@ -19,6 +19,8 @@ import neo4j from '../../src/v1'; import sharedNeo4j from '../internal/shared-neo4j'; +import _ from 'lodash'; +import {ServerVersion, VERSION_3_2_0} from '../../src/v1/internal/server-version'; describe('floating point values', () => { it('should support float 1.0 ', testValue(1)); @@ -136,18 +138,117 @@ describe('path values', () => { }); }); -function testValue(actual, expected) { - return done => { +describe('byte arrays', () => { + + let originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + let serverSupportsByteArrays = false; + + beforeEach(done => { + jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000; + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); const session = driver.session(); + session.run('RETURN 1').then(result => { + driver.close(); + const serverVersion = ServerVersion.fromString(result.summary.server.version); + serverSupportsByteArrays = serverVersion.compareTo(VERSION_3_2_0) >= 0; + done(); + }); + }); - session.run('RETURN {val} as v', {val: actual}) - .then(result => { - expect(result.records[0].get('v')).toEqual(expected || actual); - driver.close(); - done(); - }).catch(err => { - console.log(err); + afterEach(() => { + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + it('should support returning empty byte array', done => { + if(!serverSupportsByteArrays) { + done(); + return; + } + + testValue(new Int8Array(0))(done); + }); + + it('should support returning empty byte array', conditionalTestValues(serverSupportsByteArrays, new Int8Array(0))); + + it('should support returning short byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(100, 1, 255))); + + it('should support returning medium byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(50, 256, 65535))); + + it('should support returning long byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(10, 65536, 2 * 65536))); + + it('should fail to return byte array', done => { + if (serverSupportsByteArrays) { + done(); + return; + } + + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const session = driver.session(); + session.run('RETURN {array}', {array: randomByteArray(42)}).catch(error => { + driver.close(); + expect(error.message).toEqual('Byte arrays are not supported by the database this driver is connected to'); + done(); + }); + }); +}); + +function conditionalTestValues(condition, values) { + if (!condition) { + return done => done(); + } + + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const queriesPromise = values.reduce((acc, value) => + acc.then(() => runReturnQuery(driver, value)), Promise.resolve()); + return asTestFunction(queriesPromise, driver); +} + +function testValue(actual, expected) { + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const queryPromise = runReturnQuery(driver, actual, expected); + return asTestFunction(queryPromise, driver); +} + +function testValues(values) { + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const queriesPromise = values.reduce((acc, value) => + acc.then(() => runReturnQuery(driver, value)), Promise.resolve()); + return asTestFunction(queriesPromise, driver); +} + +function runReturnQuery(driver, actual, expected) { + const session = driver.session(); + return new Promise((resolve, reject) => { + session.run('RETURN {val} as v', {val: actual}).then(result => { + expect(result.records[0].get('v')).toEqual(expected || actual); + session.close(); + resolve(); + }).catch(error => { + reject(error); }); - }; + }); +} + +function asTestFunction(promise, driver) { + return done => + promise.then(() => { + driver.close(); + done(); + }).catch(error => { + driver.close(); + console.log(error); + }); +} + +function randomByteArrays(count, minLength, maxLength) { + return _.range(count).map(() => { + const length = _.random(minLength, maxLength); + return randomByteArray(length); + }); +} + +function randomByteArray(length) { + const array = _.range(length).map(() => _.random(-128, 127)); + return new Int8Array(array); } From 384185e976ef7978bed38e39f84a15b18ae23cf6 Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 15 Jun 2017 00:31:45 +0200 Subject: [PATCH 03/13] Always close driver in tests --- test/internal/tls.test.js | 6 ++++++ test/v1/examples.test.js | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/test/internal/tls.test.js b/test/internal/tls.test.js index b259b7f80..855c9ed3c 100644 --- a/test/internal/tls.test.js +++ b/test/internal/tls.test.js @@ -197,6 +197,12 @@ describe('trust-system-ca-signed-certificates', function() { done(); }); }); + + afterEach(function () { + if (driver) { + driver.close(); + } + }); }); describe('trust-on-first-use', function() { diff --git a/test/v1/examples.test.js b/test/v1/examples.test.js index cd8066b0d..91554e08d 100644 --- a/test/v1/examples.test.js +++ b/test/v1/examples.test.js @@ -108,6 +108,7 @@ describe('examples', () => { const session = driver.session(); session.run('RETURN 1').then(() => { session.close(); + driver.close(); }); }); @@ -128,6 +129,7 @@ describe('examples', () => { const session = driver.session(); session.run('RETURN 1').then(() => { session.close(); + driver.close(); }); }); @@ -152,6 +154,7 @@ describe('examples', () => { const session = driver.session(); session.run('RETURN 1').then(() => { session.close(); + driver.close(); }).catch(error => { }); }); @@ -172,6 +175,7 @@ describe('examples', () => { const session = driver.session(); session.run('RETURN 1').then(() => { session.close(); + driver.close(); }); }); @@ -193,6 +197,7 @@ describe('examples', () => { const session = driver.session(); session.run('RETURN 1').then(() => { session.close(); + driver.close(); }); }); @@ -202,6 +207,8 @@ describe('examples', () => { // tag::kerberos-auth[] const driver = neo4j.driver(uri, neo4j.auth.kerberos(ticket)); // end::kerberos-auth[] + + driver.close(); }); it('cypher error example', done => { @@ -427,6 +434,7 @@ describe('examples', () => { }); testResultPromise.then(loggedMsg => { + driver.close(); expect(loggedMsg).toEqual('Created 2 employees'); done(); }); @@ -450,6 +458,7 @@ describe('examples', () => { // end::service-unavailable[] testResultPromise.then(loggedMsg => { + driver.close(); expect(loggedMsg).toBe('Unable to create node: ' + neo4j.error.SERVICE_UNAVAILABLE); done(); }); From f477ee4468a65178b1d7139b130792e1bbce9c6e Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 15 Jun 2017 19:59:29 +0200 Subject: [PATCH 04/13] Tiny simplification of connection initialization --- src/v1/internal/connector.js | 10 +++++----- test/v1/types.test.js | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 2e5848ad0..9377576b6 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -477,10 +477,12 @@ class Connection { * @protected */ _markInitialized(metadata) { - const serverVersion = metadata.server; + const serverVersion = metadata ? metadata.server : null; if (!this.server.version) { this.server.version = serverVersion; - if (ServerVersion.fromString(serverVersion).compareTo(VERSION_3_2_0) < 0) { + + const version = ServerVersion.fromString(serverVersion); + if (version.compareTo(VERSION_3_2_0) < 0) { this._packer.disableByteArrays(); } } @@ -533,9 +535,7 @@ class ConnectionState { } }, onCompleted: metaData => { - if (metaData && metaData.server) { - this._connection._markInitialized(metaData); - } + this._connection._markInitialized(metaData); this._initialized = true; if (this._resolvePromise) { this._resolvePromise(this._connection); diff --git a/test/v1/types.test.js b/test/v1/types.test.js index b33d038ba..dda8560e7 100644 --- a/test/v1/types.test.js +++ b/test/v1/types.test.js @@ -143,7 +143,7 @@ describe('byte arrays', () => { let originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; let serverSupportsByteArrays = false; - beforeEach(done => { + beforeAll(done => { jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000; const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); @@ -156,7 +156,7 @@ describe('byte arrays', () => { }); }); - afterEach(() => { + afterAll(() => { jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; }); From df7702a9c4f778425459e6bc64a1a5e9b046eea5 Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 17:28:33 +0200 Subject: [PATCH 05/13] Fix socket leak in types test --- test/v1/types.test.js | 96 +++++++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 36 deletions(-) diff --git a/test/v1/types.test.js b/test/v1/types.test.js index dda8560e7..152a2c053 100644 --- a/test/v1/types.test.js +++ b/test/v1/types.test.js @@ -160,8 +160,8 @@ describe('byte arrays', () => { jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; }); - it('should support returning empty byte array', done => { - if(!serverSupportsByteArrays) { + it('should support returning empty byte array if server supports byte arrays', done => { + if (!serverSupportsByteArrays) { done(); return; } @@ -169,15 +169,43 @@ describe('byte arrays', () => { testValue(new Int8Array(0))(done); }); - it('should support returning empty byte array', conditionalTestValues(serverSupportsByteArrays, new Int8Array(0))); + it('should support returning empty byte array if server supports byte arrays', done => { + if (!serverSupportsByteArrays) { + done(); + return; + } + + testValues([new Int8Array(0)])(done); + }); + + it('should support returning short byte arrays if server supports byte arrays', done => { + if (!serverSupportsByteArrays) { + done(); + return; + } + + testValues(randomByteArrays(100, 1, 255))(done); + }); + + it('should support returning medium byte arrays if server supports byte arrays', done => { + if (!serverSupportsByteArrays) { + done(); + return; + } - it('should support returning short byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(100, 1, 255))); + testValues(randomByteArrays(50, 256, 65535))(done); + }); - it('should support returning medium byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(50, 256, 65535))); + it('should support returning long byte arrays if server supports byte arrays', done => { + if (!serverSupportsByteArrays) { + done(); + return; + } - it('should support returning long byte arrays', conditionalTestValues(serverSupportsByteArrays, randomByteArrays(10, 65536, 2 * 65536))); + testValues(randomByteArrays(10, 65536, 2 * 65536))(done); + }); - it('should fail to return byte array', done => { + it('should fail to return byte array if server does not support byte arrays', done => { if (serverSupportsByteArrays) { done(); return; @@ -193,28 +221,35 @@ describe('byte arrays', () => { }); }); -function conditionalTestValues(condition, values) { - if (!condition) { - return done => done(); - } - - const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); - const queriesPromise = values.reduce((acc, value) => - acc.then(() => runReturnQuery(driver, value)), Promise.resolve()); - return asTestFunction(queriesPromise, driver); -} - function testValue(actual, expected) { - const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); - const queryPromise = runReturnQuery(driver, actual, expected); - return asTestFunction(queryPromise, driver); + return done => { + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const queryPromise = runReturnQuery(driver, actual, expected); + + queryPromise.then(() => { + driver.close(); + done(); + }).catch(error => { + driver.close(); + console.log(error); + }); + }; } function testValues(values) { - const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); - const queriesPromise = values.reduce((acc, value) => - acc.then(() => runReturnQuery(driver, value)), Promise.resolve()); - return asTestFunction(queriesPromise, driver); + return done => { + const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + const queriesPromise = values.reduce((acc, value) => + acc.then(() => runReturnQuery(driver, value)), Promise.resolve()); + + queriesPromise.then(() => { + driver.close(); + done(); + }).catch(error => { + driver.close(); + console.log(error); + }); + }; } function runReturnQuery(driver, actual, expected) { @@ -230,17 +265,6 @@ function runReturnQuery(driver, actual, expected) { }); } -function asTestFunction(promise, driver) { - return done => - promise.then(() => { - driver.close(); - done(); - }).catch(error => { - driver.close(); - console.log(error); - }); -} - function randomByteArrays(count, minLength, maxLength) { return _.range(count).map(() => { const length = _.random(minLength, maxLength); From cb22768ac95b36b28e9cbc9ae52745aa40d8d932 Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 17:29:24 +0200 Subject: [PATCH 06/13] Always close drivers in TCK tests --- test/v1/tck/steps/authsteps.js | 26 ++++++++++++++++++------- test/v1/tck/steps/erroreportingsteps.js | 9 ++++++++- test/v1/tck/steps/tlssteps.js | 23 +++++++++++++++++++++- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/test/v1/tck/steps/authsteps.js b/test/v1/tck/steps/authsteps.js index 3f7e0aa61..a3d8c242a 100644 --- a/test/v1/tck/steps/authsteps.js +++ b/test/v1/tck/steps/authsteps.js @@ -31,16 +31,19 @@ module.exports = function () { }); this.Then(/^reading and writing to the database should be possible$/, function (callback) { - var session = this.driver.session(); - session.run("CREATE (:label1)").then( function( ) { - callback(); - }).catch(function(err) {callback(new Error("Rejected Promise: " + err))}); + var driver = this.driver; + var session = driver.session(); + session.run('CREATE (:label1)').then(function () { + closeDriver(driver); + callback(); + }).catch(function (err) { + closeDriver(driver); + callback(new Error('Rejected Promise: ' + err)); + }); }); this.Given(/^a driver is configured with auth enabled and the wrong password is provided$/, function () { - if (this.driver) { - this.driver.close(); - } + closeDriver(this.driver); this.driver = neo4j.driver("bolt://localhost", neo4j.auth.basic(sharedNeo4j.username, "wrong")); this.driver.session(); }); @@ -49,14 +52,17 @@ module.exports = function () { var self = this; this.driver.onError = function (err) { + closeDriver(self.driver); self.err = err; callback(); }; var session = this.driver.session(); session.run("CREATE (:label1)").then( function( ) { + closeDriver(self.driver); callback(new Error("Should not be able to run session!")); }).catch( function(err) { + closeDriver(self.driver); callback(); }); }); @@ -76,4 +82,10 @@ module.exports = function () { throw new Error("Wrong error code. Expected: '" + expectedCode + "'. Got: '" + code + "'"); } }); + + function closeDriver(driver) { + if (driver) { + driver.close(); + } + } }; diff --git a/test/v1/tck/steps/erroreportingsteps.js b/test/v1/tck/steps/erroreportingsteps.js index 46136c474..94cac84ed 100644 --- a/test/v1/tck/steps/erroreportingsteps.js +++ b/test/v1/tck/steps/erroreportingsteps.js @@ -65,7 +65,14 @@ module.exports = function () { this.When(/^I set up a driver to an incorrect port$/, function (callback) { var self = this; var driver = neo4j.driver("bolt://localhost:7777", neo4j.auth.basic(sharedNeo4j.username, sharedNeo4j.password)); - driver.onError = function (error) { self.error = error; callback()}; + driver.onSuccess = function () { + driver.close(); + }; + driver.onError = function (error) { + driver.close(); + self.error = error; + callback(); + }; driver.session().beginTransaction(); setTimeout(callback, 1000); }); diff --git a/test/v1/tck/steps/tlssteps.js b/test/v1/tck/steps/tlssteps.js index 38152a175..2eaee3afa 100644 --- a/test/v1/tck/steps/tlssteps.js +++ b/test/v1/tck/steps/tlssteps.js @@ -20,11 +20,14 @@ module.exports = function () { }); this.Then(/^sessions should simply work$/, {timeout: CALLBACK_TIMEOUT}, function (callback) { - var session = this.driver1.session(); + var self = this; + var session = self.driver1.session(); session.run("RETURN 1").then(function (result) { session.close(); + _closeDrivers(self.driver1, self.driver2); callback(); }).catch(function (error) { + _closeDrivers(self.driver1, self.driver2); console.log(error); }); }); @@ -57,11 +60,13 @@ module.exports = function () { var self = this; session.run("RETURN 1") .then(function(res) { + _closeDrivers(self.driver1, self.driver2); console.log(res); }) .catch(function (error) { self.error = error; session.close(); + _closeDrivers(self.driver1, self.driver2); callback(); }); }); @@ -76,6 +81,9 @@ module.exports = function () { "and the driver will update the file with the new certificate. You can configure which file the driver should use " + "to store this information by setting `knownHosts` to another path in your driver configuration - " + "and you can disable encryption there as well using `encrypted:\"ENCRYPTION_OFF\"`."; + + _closeDrivers(this.driver1, this.driver2); + if (this.error.message !== expected) { callback(new Error("Given and expected results does not match: " + this.error.message + " Expected " + expected)); } else { @@ -101,6 +109,7 @@ module.exports = function () { var session2 = self.driver2.session(); session2.run("RETURN 1").then(function (result) { session2.close(); + _closeDrivers(self.driver1, self.driver2); callback(); }); }); @@ -157,6 +166,9 @@ module.exports = function () { "`neo4j.v1.driver(.., { trustedCertificates:['path/to/certificate.crt']}). This is a security measure to protect " + "against man-in-the-middle attacks. If you are just trying Neo4j out and are not concerned about encryption, " + "simply disable it using `encrypted=\"ENCRYPTION_OFF\"` in the driver options. Socket responded with: DEPTH_ZERO_SELF_SIGNED_CERT"; + + _closeDrivers(this.driver1, this.driver2); + if (this.error.message !== expected) { callback(new Error("Given and expected results does not match: " + this.error.message + " Expected " + expected)); } else { @@ -171,4 +183,13 @@ module.exports = function () { encrypted: "ENCRYPTION_ON" }); } + + function _closeDrivers() { + for (var i = 0; i < arguments.length; i++) { + var driver = arguments[i]; + if (driver) { + driver.close(); + } + } + } }; From 5ccf9b0574eda39501a5596c59ec982711afe22b Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 17:29:55 +0200 Subject: [PATCH 07/13] Always close connections in connector tests --- src/v1/internal/ch-dummy.js | 8 +++++ test/internal/connector.test.js | 57 +++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/v1/internal/ch-dummy.js b/src/v1/internal/ch-dummy.js index 433799336..4d4e0044f 100644 --- a/src/v1/internal/ch-dummy.js +++ b/src/v1/internal/ch-dummy.js @@ -18,6 +18,7 @@ */ import {CombinedBuffer} from './buf'; + const observer = { instance: null, updateInstance: (instance) => { @@ -55,6 +56,13 @@ class DummyChannel { toBuffer () { return new CombinedBuffer( this.written ); } + + close(cb) { + this.written = []; + if (cb) { + return cb(); + } + } } const channel = DummyChannel; diff --git a/test/internal/connector.test.js b/test/internal/connector.test.js index 02d12ace3..c7852c999 100644 --- a/test/internal/connector.test.js +++ b/test/internal/connector.test.js @@ -28,63 +28,72 @@ import {ServerVersion} from '../../src/v1/internal/server-version'; describe('connector', () => { + let connection; + + afterEach(done => { + const usedConnection = connection; + connection = null; + if (usedConnection) { + usedConnection.close(); + } + done(); + }); + it('should read/write basic messages', done => { // Given - const conn = connect("bolt://localhost"); + connection = connect("bolt://localhost"); // When - conn.initialize("mydriver/0.0.0", basicAuthToken(), { + connection.initialize("mydriver/0.0.0", basicAuthToken(), { onCompleted: msg => { expect(msg).not.toBeNull(); - conn.close(); done(); }, onError: console.log }); - conn.sync(); + connection.sync(); }); it('should retrieve stream', done => { // Given - const conn = connect("bolt://localhost"); + connection = connect("bolt://localhost"); // When const records = []; - conn.initialize("mydriver/0.0.0", basicAuthToken()); - conn.run("RETURN 1.0", {}); - conn.pullAll({ + connection.initialize("mydriver/0.0.0", basicAuthToken()); + connection.run("RETURN 1.0", {}); + connection.pullAll({ onNext: record => { records.push(record); }, onCompleted: () => { expect(records[0][0]).toBe(1); - conn.close(); done(); } }); - conn.sync(); + connection.sync(); }); it('should use DummyChannel to read what gets written', done => { // Given const observer = DummyChannel.observer; - const conn = connect("bolt://localhost", {channel: DummyChannel.channel}); + connection = connect("bolt://localhost", {channel: DummyChannel.channel}); // When - conn.initialize("mydriver/0.0.0", basicAuthToken()); - conn.run("RETURN 1", {}); - conn.sync(); + connection.initialize("mydriver/0.0.0", basicAuthToken()); + connection.run("RETURN 1", {}); + connection.sync(); expect(observer.instance.toHex()).toBe('60 60 b0 17 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 44 b2 01 8e 6d 79 64 72 69 76 65 72 2f 30 2e 30 2e 30 a3 86 73 63 68 65 6d 65 85 62 61 73 69 63 89 70 72 69 6e 63 69 70 61 6c 85 6e 65 6f 34 6a 8b 63 72 65 64 65 6e 74 69 61 6c 73 88 70 61 73 73 77 6f 72 64 00 00 00 0c b2 10 88 52 45 54 55 52 4e 20 31 a0 00 00 '); done(); }); it('should provide error message when connecting to http-port', done => { // Given - const conn = connect("bolt://localhost:7474", {encrypted: false}); + connection = connect("bolt://localhost:7474", {encrypted: false}); // When - conn.initialize("mydriver/0.0.0", basicAuthToken(), { + connection.initialize("mydriver/0.0.0", basicAuthToken(), { onCompleted: msg => { }, onError: err => { @@ -96,13 +105,13 @@ describe('connector', () => { done(); } }); - conn.sync(); + connection.sync(); }); it('should convert failure messages to errors', done => { const channel = new DummyChannel.channel; - const connection = new Connection(channel, 'bolt://localhost'); + connection = new Connection(channel, 'bolt://localhost'); const errorCode = 'Neo.ClientError.Schema.ConstraintValidationFailed'; const errorMessage = 'Node 0 already exists with label User and property "email"=[john@doe.com]'; @@ -119,7 +128,7 @@ describe('connector', () => { }); it('should notify when connection initialization completes', done => { - const connection = connect('bolt://localhost'); + connection = connect('bolt://localhost'); connection.initializationCompleted().then(initializedConnection => { expect(initializedConnection).toBe(connection); @@ -130,7 +139,7 @@ describe('connector', () => { }); it('should notify when connection initialization fails', done => { - const connection = connect('bolt://localhost:7474'); // wrong port + connection = connect('bolt://localhost:7474'); // wrong port connection.initializationCompleted().catch(error => { expect(error).toBeDefined(); @@ -141,7 +150,7 @@ describe('connector', () => { }); it('should notify provided observer when connection initialization completes', done => { - const connection = connect('bolt://localhost'); + connection = connect('bolt://localhost'); connection.initialize('mydriver/0.0.0', basicAuthToken(), { onCompleted: metaData => { @@ -153,7 +162,7 @@ describe('connector', () => { }); it('should notify provided observer when connection initialization fails', done => { - const connection = connect('bolt://localhost:7474'); // wrong port + connection = connect('bolt://localhost:7474'); // wrong port connection.initialize('mydriver/0.0.0', basicAuthToken(), { onError: error => { @@ -165,7 +174,7 @@ describe('connector', () => { }); it('should have server version after connection initialization completed', done => { - const connection = connect('bolt://localhost'); + connection = connect('bolt://localhost'); connection.initializationCompleted().then(initializedConnection => { const serverVersion = ServerVersion.fromString(initializedConnection.server.version); @@ -177,7 +186,7 @@ describe('connector', () => { }); it('should fail all new observers after initialization error', done => { - const connection = connect('bolt://localhost:7474'); // wrong port + connection = connect('bolt://localhost:7474'); // wrong port connection.initialize('mydriver/0.0.0', basicAuthToken(), { onError: initialError => { From cc646df8a874ac09f16f9355e79c4ca6f9d7331a Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 18:13:07 +0200 Subject: [PATCH 08/13] Optional logging of active Node handles in gulp This commit adds optional logging of open sockets, file handles, etc. to the gulp script. Logging happens after various NodeJS test task that can theoretically leave unclosed resources after execution. Logging is turned off by default. It happened to be very useful when NodeJS process does not exit after tests because of open `TLSSocket` objects. --- gulpfile.babel.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/gulpfile.babel.js b/gulpfile.babel.js index e6d10fefd..cdf88323a 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -48,6 +48,11 @@ var file = require('gulp-file'); var semver = require('semver'); var sharedNeo4j = require('./test/internal/shared-neo4j').default; +/** + * Useful to investigate resource leaks in tests. Enable to see active sockets and file handles after the 'test' task. + */ +var enableActiveNodeHandlesLogging = false; + gulp.task('default', ["test"]); gulp.task('browser', function(cb){ @@ -165,7 +170,7 @@ gulp.task('test-nodejs', ['install-driver-into-sandbox'], function () { .pipe(jasmine({ includeStackTrace: true, verbose: true - })); + })).on('end', logActiveNodeHandles); }); gulp.task('test-boltkit', ['nodejs'], function () { @@ -173,7 +178,7 @@ gulp.task('test-boltkit', ['nodejs'], function () { .pipe(jasmine({ includeStackTrace: true, verbose: true - })); + })).on('end', logActiveNodeHandles); }); gulp.task('test-browser', function (cb) { @@ -210,7 +215,7 @@ gulp.task('run-tck', ['download-tck', 'nodejs'], function() { 'steps': 'test/v1/tck/steps/*.js', 'format': 'progress', 'tags' : ['~@fixed_session_pool', '~@db', '~@equality', '~@streaming_and_cursor_navigation'] - })); + })).on('end', logActiveNodeHandles); }); /** Set the project version, controls package.json and version.js */ @@ -248,5 +253,11 @@ gulp.task('run-stress-tests', function () { .pipe(jasmine({ includeStackTrace: true, verbose: true - })); + })).on('end', logActiveNodeHandles); }); + +function logActiveNodeHandles() { + if (enableActiveNodeHandlesLogging) { + console.log('-- Active NodeJS handles START\n', process._getActiveHandles(), '\n-- Active NodeJS handles END'); + } +} From 79585223b4fa1e30065c204788cce3eee6f6fe43 Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 19:01:31 +0200 Subject: [PATCH 09/13] Couple unit tests for connection providers To assert that both `DirectConnectionProvider` and `LoadBalancer` return promises with initialized connections. --- test/internal/connection-providers.test.js | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/internal/connection-providers.test.js b/test/internal/connection-providers.test.js index 14e291f17..e70299246 100644 --- a/test/internal/connection-providers.test.js +++ b/test/internal/connection-providers.test.js @@ -44,6 +44,16 @@ describe('DirectConnectionProvider', () => { }); }); + it('returns an initialized connection', done => { + const pool = newPool(); + const connectionProvider = newDirectConnectionProvider('localhost:123', pool); + + connectionProvider.acquireConnection(READ).then(connection => { + expect(connection.initialized).toBeTruthy(); + done(); + }); + }); + }); describe('LoadBalancer', () => { @@ -1049,6 +1059,26 @@ describe('LoadBalancer', () => { }); }); + it('returns an initialized connection', done => { + const pool = newPool(); + const loadBalancer = newLoadBalancer( + ['server-1', 'server-2'], + ['server-3', 'server-4'], + ['server-5', 'server-6'], + pool + ); + + loadBalancer.acquireConnection(READ).then(connection => { + expect(connection.initialized).toBeTruthy(); + + loadBalancer.acquireConnection(WRITE).then(connection => { + expect(connection.initialized).toBeTruthy(); + + done(); + }); + }); + }); + }); function newDirectConnectionProvider(address, pool) { @@ -1126,6 +1156,7 @@ class FakeConnection { constructor(address, release) { this.address = address; this.release = release; + this.initialized = false; } static create(address, release) { @@ -1133,6 +1164,7 @@ class FakeConnection { } initializationCompleted() { + this.initialized = true; return Promise.resolve(this); } } From 83e35fc82d3361a97a66f1f2a59f68a18fbaf04c Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 19:02:02 +0200 Subject: [PATCH 10/13] Make stress test not care about label and property ordering --- test/v1/stress.test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/v1/stress.test.js b/test/v1/stress.test.js index dd2581779..993f5159f 100644 --- a/test/v1/stress.test.js +++ b/test/v1/stress.test.js @@ -217,12 +217,12 @@ describe('stress tests', () => { function verifyRecord(record) { const node = record.get(0); - if (!_.isEqual(['Person', 'Employee'], node.labels)) { + if (!arraysEqual(['Person', 'Employee'], node.labels)) { return new Error(`Unexpected labels in node: ${JSON.stringify(node)}`); } const propertyKeys = _.keys(node.properties); - if (!_.isEmpty(propertyKeys) && !_.isEqual(['name', 'salary'], propertyKeys)) { + if (!_.isEmpty(propertyKeys) && !arraysEqual(['name', 'salary'], propertyKeys)) { return new Error(`Unexpected property keys in node: ${JSON.stringify(node)}`); } @@ -298,6 +298,10 @@ describe('stress tests', () => { }); } + function arraysEqual(array1, array2) { + return _.difference(array1, array2).length === 0; + } + class Context { constructor(driver, loggingEnabled) { From 3c87a27d9746ea8df482d6bb629dc68281a0238b Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 16 Jun 2017 19:33:40 +0200 Subject: [PATCH 11/13] Fix couple boltkit tests They were failing because of incorrect artificial Neo4j version. Also fixes JS doc visibility modifier in driver. --- src/v1/driver.js | 2 +- test/resources/boltkit/read_server_with_version.script | 2 +- test/resources/boltkit/write_server_with_version.script | 2 +- test/v1/routing.driver.boltkit.it.js | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/v1/driver.js b/src/v1/driver.js index 882531482..4dd03c593 100644 --- a/src/v1/driver.js +++ b/src/v1/driver.js @@ -62,7 +62,7 @@ class Driver { /** * Reference to the connection provider. Initialized lazily by {@link _getOrCreateConnectionProvider}. * @type {ConnectionProvider} - * @private + * @protected */ this._connectionProvider = null; } diff --git a/test/resources/boltkit/read_server_with_version.script b/test/resources/boltkit/read_server_with_version.script index 016b3c6d0..6adc18c52 100644 --- a/test/resources/boltkit/read_server_with_version.script +++ b/test/resources/boltkit/read_server_with_version.script @@ -2,7 +2,7 @@ !: AUTO PULL_ALL C: INIT "neo4j-javascript/0.0.0-dev" {"credentials": "neo4j", "scheme": "basic", "principal": "neo4j"} -S: SUCCESS {"server": "TheReadServerV1"} +S: SUCCESS {"server": "Neo4j/8.8.8"} C: RUN "MATCH (n) RETURN n.name" {} PULL_ALL S: SUCCESS {"fields": ["n.name"]} diff --git a/test/resources/boltkit/write_server_with_version.script b/test/resources/boltkit/write_server_with_version.script index 3861d27c0..294fb774a 100644 --- a/test/resources/boltkit/write_server_with_version.script +++ b/test/resources/boltkit/write_server_with_version.script @@ -2,7 +2,7 @@ !: AUTO PULL_ALL C: INIT "neo4j-javascript/0.0.0-dev" {"credentials": "neo4j", "scheme": "basic", "principal": "neo4j"} -S: SUCCESS {"server": "TheWriteServerV1"} +S: SUCCESS {"server": "Neo4j/9.9.9"} C: RUN "CREATE (n {name:'Bob'})" {} PULL_ALL S: SUCCESS {} diff --git a/test/v1/routing.driver.boltkit.it.js b/test/v1/routing.driver.boltkit.it.js index deac70b4a..ebc97eb15 100644 --- a/test/v1/routing.driver.boltkit.it.js +++ b/test/v1/routing.driver.boltkit.it.js @@ -796,10 +796,10 @@ describe('routing driver', () => { readServer.exit(readServerExitCode => { expect(readServerInfo.address).toBe('127.0.0.1:9005'); - expect(readServerInfo.version).toBe('TheReadServerV1'); + expect(readServerInfo.version).toBe('Neo4j/8.8.8'); expect(writeServerInfo.address).toBe('127.0.0.1:9007'); - expect(writeServerInfo.version).toBe('TheWriteServerV1'); + expect(writeServerInfo.version).toBe('Neo4j/9.9.9'); expect(routingServerExitCode).toEqual(0); expect(writeServerExitCode).toEqual(0); @@ -852,10 +852,10 @@ describe('routing driver', () => { readServer.exit(readServerExitCode => { expect(readSummary.server.address).toBe('127.0.0.1:9005'); - expect(readSummary.server.version).toBe('TheReadServerV1'); + expect(readSummary.server.version).toBe('Neo4j/8.8.8'); expect(writeSummary.server.address).toBe('127.0.0.1:9007'); - expect(writeSummary.server.version).toBe('TheWriteServerV1'); + expect(writeSummary.server.version).toBe('Neo4j/9.9.9'); expect(routingServerExitCode).toEqual(0); expect(writeServerExitCode).toEqual(0); From 4e33fc924bf6216b67c2f8a511c1d8d8f8a9c508 Mon Sep 17 00:00:00 2001 From: lutovich Date: Sat, 17 Jun 2017 01:23:30 +0200 Subject: [PATCH 12/13] Fix purging of connections on INIT failure Commit moves waiting for connection initialization promise higher in the stack - from `ConnectionProvider` to `ConnectionHolder`. This is needed to be able to notify stream observer about used connection even if initialization fails. It is later possible to purge connections for the given server. Previously stream observer did not get to know the connection and pool was not purged on connectivity errors. --- src/v1/internal/connection-holder.js | 10 ++-- src/v1/internal/connection-providers.js | 24 ++-------- src/v1/internal/connector.js | 56 +++++++++++++--------- src/v1/session.js | 3 +- src/v1/transaction.js | 18 +++---- test/internal/connection-holder.test.js | 52 ++++++++++++++++++-- test/internal/connection-providers.test.js | 32 ------------- test/internal/fake-connection.js | 11 ++++- 8 files changed, 112 insertions(+), 94 deletions(-) diff --git a/src/v1/internal/connection-holder.js b/src/v1/internal/connection-holder.js index bae63cacc..a31dc94aa 100644 --- a/src/v1/internal/connection-holder.js +++ b/src/v1/internal/connection-holder.js @@ -49,10 +49,14 @@ export default class ConnectionHolder { /** * Get the current connection promise. + * @param {StreamObserver} streamObserver an observer for this connection. * @return {Promise} promise resolved with the current connection. */ - getConnection() { - return this._connectionPromise; + getConnection(streamObserver) { + return this._connectionPromise.then(connection => { + streamObserver.resolveConnection(connection); + return connection.initializationCompleted(); + }); } /** @@ -117,7 +121,7 @@ class EmptyConnectionHolder extends ConnectionHolder { // nothing to initialize } - getConnection() { + getConnection(streamObserver) { return Promise.reject(newError('This connection holder does not serve connections')); } diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index b124c2892..b9b633011 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -54,7 +54,8 @@ export class DirectConnectionProvider extends ConnectionProvider { } acquireConnection(mode) { - const connectionPromise = acquireConnectionFromPool(this._connectionPool, this._address); + const connection = this._connectionPool.acquire(this._address); + const connectionPromise = Promise.resolve(connection); return this._withAdditionalOnErrorCallback(connectionPromise, this._driverOnErrorCallback); } } @@ -101,7 +102,7 @@ export class LoadBalancer extends ConnectionProvider { `Failed to obtain connection towards ${serverName} server. Known routing table is: ${this._routingTable}`, SESSION_EXPIRED)); } - return acquireConnectionFromPool(this._connectionPool, address); + return this._connectionPool.acquire(address); } _freshRoutingTable(accessMode) { @@ -198,9 +199,10 @@ export class LoadBalancer extends ConnectionProvider { } _createSessionForRediscovery(routerAddress) { + const connection = this._connectionPool.acquire(routerAddress); // initialized connection is required for routing procedure call // server version needs to be known to decide which routing procedure to use - const initializedConnectionPromise = acquireConnectionFromPool(this._connectionPool, routerAddress); + const initializedConnectionPromise = connection.initializationCompleted(); const connectionProvider = new SingleConnectionProvider(initializedConnectionPromise); return new Session(READ, connectionProvider); } @@ -261,19 +263,3 @@ export class SingleConnectionProvider extends ConnectionProvider { return connectionPromise; } } - -// todo: test that all connection providers return initialized connections - -/** - * Acquire an initialized connection from the given connection pool for the given address. Returned connection - * promise will be resolved by a connection which completed initialization, i.e. received a SUCCESS response - * for it's INIT message. - * @param {Pool} connectionPool the connection pool to acquire connection from. - * @param {string} address the server address. - * @return {Promise} the initialized connection. - */ -function acquireConnectionFromPool(connectionPool, address) { - const connection = connectionPool.acquire(address); - // initialized connection is required to be able to perform subsequent server version checks - return connection.initializationCompleted(); -} diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 9377576b6..54bf35420 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -498,11 +498,15 @@ class ConnectionState { constructor(connection) { this._connection = connection; - this._initialized = false; - this._initializationError = null; - - this._resolvePromise = null; - this._rejectPromise = null; + this._initRequested = false; + this._initError = null; + + this._resolveInitPromise = null; + this._rejectInitPromise = null; + this._initPromise = new Promise((resolve, reject) => { + this._resolveInitPromise = resolve; + this._rejectInitPromise = reject; + }); } /** @@ -519,11 +523,7 @@ class ConnectionState { } }, onError: error => { - this._initializationError = error; - if (this._rejectPromise) { - this._rejectPromise(error); - this._rejectPromise = null; - } + this._processFailure(error); this._connection._updateCurrentObserver(); // make sure this same observer will not be called again try { @@ -536,11 +536,8 @@ class ConnectionState { }, onCompleted: metaData => { this._connection._markInitialized(metaData); - this._initialized = true; - if (this._resolvePromise) { - this._resolvePromise(this._connection); - this._resolvePromise = null; - } + this._resolveInitPromise(this._connection); + if (observer && observer.onCompleted) { observer.onCompleted(metaData); } @@ -553,15 +550,28 @@ class ConnectionState { * @return {Promise} the result of connection initialization. */ initializationCompleted() { - if (this._initialized) { - return Promise.resolve(this._connection); - } else if (this._initializationError) { - return Promise.reject(this._initializationError); + this._initRequested = true; + + if (this._initError) { + const error = this._initError; + this._initError = null; // to reject initPromise only once + this._rejectInitPromise(error); + } + + return this._initPromise; + } + + /** + * @private + */ + _processFailure(error) { + if (this._initRequested) { + // someone is waiting for initialization to complete, reject the promise + this._rejectInitPromise(error); } else { - return new Promise((resolve, reject) => { - this._resolvePromise = resolve; - this._rejectPromise = reject; - }); + // no one is waiting for initialization, memorize the error but do not reject the promise + // to avoid unnecessary unhandled promise rejection warnings + this._initError = error; } } } diff --git a/src/v1/session.js b/src/v1/session.js index 15c491fd6..74770a936 100644 --- a/src/v1/session.js +++ b/src/v1/session.js @@ -75,8 +75,7 @@ class Session { const connectionHolder = this._connectionHolderWithMode(this._mode); if (!this._hasTx) { connectionHolder.initializeConnection(); - connectionHolder.getConnection().then(connection => { - streamObserver.resolveConnection(connection); + connectionHolder.getConnection(streamObserver).then(connection => { statementRunner(connection, streamObserver); connection.pullAll(streamObserver); connection.sync(); diff --git a/src/v1/transaction.js b/src/v1/transaction.js index 37b6ba6a1..c4c0fa7e4 100644 --- a/src/v1/transaction.js +++ b/src/v1/transaction.js @@ -43,8 +43,7 @@ class Transaction { params = {bookmark: bookmark}; } - this._connectionHolder.getConnection().then(conn => { - streamObserver.resolveConnection(conn); + this._connectionHolder.getConnection(streamObserver).then(conn => { conn.run('BEGIN', params, streamObserver); conn.pullAll(streamObserver); }).catch(error => streamObserver.onError(error)); @@ -167,8 +166,7 @@ let _states = { return {result: _runPullAll("ROLLBACK", connectionHolder, observer), state: _states.ROLLED_BACK}; }, run: (connectionHolder, observer, statement, parameters) => { - connectionHolder.getConnection().then(conn => { - observer.resolveConnection(conn); + connectionHolder.getConnection(observer).then(conn => { conn.run(statement, parameters || {}, observer); conn.pullAll(observer); conn.sync(); @@ -246,13 +244,11 @@ let _states = { }; function _runPullAll(msg, connectionHolder, observer) { - connectionHolder.getConnection().then( - conn => { - observer.resolveConnection(conn); - conn.run(msg, {}, observer); - conn.pullAll(observer); - conn.sync(); - }).catch(error => observer.onError(error)); + connectionHolder.getConnection(observer).then(conn => { + conn.run(msg, {}, observer); + conn.pullAll(observer); + conn.sync(); + }).catch(error => observer.onError(error)); // for commit & rollback we need result that uses real connection holder and notifies it when // connection is not needed and can be safely released to the pool diff --git a/test/internal/connection-holder.test.js b/test/internal/connection-holder.test.js index 8c5d1a854..0cc6a5014 100644 --- a/test/internal/connection-holder.test.js +++ b/test/internal/connection-holder.test.js @@ -21,11 +21,12 @@ import ConnectionHolder, {EMPTY_CONNECTION_HOLDER} from '../../src/v1/internal/c import {SingleConnectionProvider} from '../../src/v1/internal/connection-providers'; import {READ} from '../../src/v1/driver'; import FakeConnection from './fake-connection'; +import StreamObserver from '../../src/v1/internal/stream-observer'; describe('EmptyConnectionHolder', () => { it('should return rejected promise instead of connection', done => { - EMPTY_CONNECTION_HOLDER.getConnection().catch(() => { + EMPTY_CONNECTION_HOLDER.getConnection(new StreamObserver()).catch(() => { done(); }); }); @@ -62,8 +63,40 @@ describe('ConnectionHolder', () => { connectionHolder.initializeConnection(); - connectionHolder.getConnection().then(connection => { - expect(connection).toBe(connection); + connectionHolder.getConnection(new StreamObserver()).then(conn => { + expect(conn).toBe(connection); + verifyConnectionInitialized(conn); + done(); + }); + }); + + it('should make stream observer aware about connection when initialization successful', done => { + const connection = new FakeConnection().withServerVersion('Neo4j/9.9.9'); + const connectionProvider = newSingleConnectionProvider(connection); + const connectionHolder = new ConnectionHolder(READ, connectionProvider); + const streamObserver = new StreamObserver(); + + connectionHolder.initializeConnection(); + + connectionHolder.getConnection(streamObserver).then(conn => { + verifyConnectionInitialized(conn); + verifyConnection(streamObserver, 'Neo4j/9.9.9'); + done(); + }); + }); + + it('should make stream observer aware about connection when initialization fails', done => { + const connection = new FakeConnection().withServerVersion('Neo4j/7.7.7').withFailedInitialization(new Error('Oh!')); + const connectionProvider = newSingleConnectionProvider(connection); + const connectionHolder = new ConnectionHolder(READ, connectionProvider); + const streamObserver = new StreamObserver(); + + connectionHolder.initializeConnection(); + + connectionHolder.getConnection(streamObserver).catch(error => { + expect(error.message).toEqual('Oh!'); + verifyConnectionInitialized(connection); + verifyConnection(streamObserver, 'Neo4j/7.7.7'); done(); }); }); @@ -195,3 +228,16 @@ class RecordingConnectionProvider extends SingleConnectionProvider { function newSingleConnectionProvider(connection) { return new SingleConnectionProvider(Promise.resolve(connection)); } + +function verifyConnectionInitialized(connection) { + expect(connection.initializationInvoked).toEqual(1); +} + +function verifyConnection(streamObserver, expectedServerVersion) { + expect(streamObserver._conn).toBeDefined(); + expect(streamObserver._conn).not.toBeNull(); + + // server version is taken from connection, verify it as well + const metadata = streamObserver.serverMetadata(); + expect(metadata.server.version).toEqual(expectedServerVersion); +} diff --git a/test/internal/connection-providers.test.js b/test/internal/connection-providers.test.js index e70299246..14e291f17 100644 --- a/test/internal/connection-providers.test.js +++ b/test/internal/connection-providers.test.js @@ -44,16 +44,6 @@ describe('DirectConnectionProvider', () => { }); }); - it('returns an initialized connection', done => { - const pool = newPool(); - const connectionProvider = newDirectConnectionProvider('localhost:123', pool); - - connectionProvider.acquireConnection(READ).then(connection => { - expect(connection.initialized).toBeTruthy(); - done(); - }); - }); - }); describe('LoadBalancer', () => { @@ -1059,26 +1049,6 @@ describe('LoadBalancer', () => { }); }); - it('returns an initialized connection', done => { - const pool = newPool(); - const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], - pool - ); - - loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.initialized).toBeTruthy(); - - loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.initialized).toBeTruthy(); - - done(); - }); - }); - }); - }); function newDirectConnectionProvider(address, pool) { @@ -1156,7 +1126,6 @@ class FakeConnection { constructor(address, release) { this.address = address; this.release = release; - this.initialized = false; } static create(address, release) { @@ -1164,7 +1133,6 @@ class FakeConnection { } initializationCompleted() { - this.initialized = true; return Promise.resolve(this); } } diff --git a/test/internal/fake-connection.js b/test/internal/fake-connection.js index c2b773d1d..e8ccda737 100644 --- a/test/internal/fake-connection.js +++ b/test/internal/fake-connection.js @@ -31,9 +31,12 @@ export default class FakeConnection { this.resetAsyncInvoked = 0; this.syncInvoked = 0; this.releaseInvoked = 0; + this.initializationInvoked = 0; this.seenStatements = []; this.seenParameters = []; this.server = {}; + + this._initializationPromise = Promise.resolve(this); } run(statement, parameters) { @@ -61,7 +64,8 @@ export default class FakeConnection { } initializationCompleted() { - return Promise.resolve(this); + this.initializationInvoked++; + return this._initializationPromise; } isReleasedOnceOnSessionClose() { @@ -94,4 +98,9 @@ export default class FakeConnection { this.server.version = version; return this; } + + withFailedInitialization(error) { + this._initializationPromise = Promise.reject(error); + return this; + } }; From b44296cefdfa83a7a75ce6d8a903d34d701db09e Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 21 Jun 2017 15:16:26 +0200 Subject: [PATCH 13/13] refactored unpack method --- src/v1/internal/connector.js | 1 - src/v1/internal/packstream.js | 212 +++++++++++++++++++++++----------- 2 files changed, 145 insertions(+), 68 deletions(-) diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 54bf35420..10755d9db 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -480,7 +480,6 @@ class Connection { const serverVersion = metadata ? metadata.server : null; if (!this.server.version) { this.server.version = serverVersion; - const version = ServerVersion.fromString(serverVersion); if (version.compareTo(VERSION_3_2_0) < 0) { this._packer.disableByteArrays(); diff --git a/src/v1/internal/packstream.js b/src/v1/internal/packstream.js index fed7c9438..d00df29c0 100644 --- a/src/v1/internal/packstream.js +++ b/src/v1/internal/packstream.js @@ -316,54 +316,65 @@ class Unpacker { this.structMappers = {}; } - unpackList (size, buffer) { - let value = []; - for(let i = 0; i < size; i++) { - value.push( this.unpack( buffer ) ); - } - return value; - } + unpack(buffer) { + const marker = buffer.readUInt8(); + const markerHigh = marker & 0xF0; + const markerLow = marker & 0x0F; - unpackBytes(size, buffer) { - const value = new Int8Array(size); - for (let i = 0; i < size; i++) { - value[i] = buffer.readInt8(); + if (marker == NULL) { + return null; } - return value; - } - unpackMap (size, buffer) { - let value = {}; - for(let i = 0; i < size; i++) { - let key = this.unpack(buffer); - value[key] = this.unpack(buffer); + const boolean = this._unpackBoolean(marker); + if (boolean !== null) { + return boolean; } - return value; - } - unpackStruct (size, buffer) { - let signature = buffer.readUInt8(); - let mapper = this.structMappers[signature]; - if( mapper ) { - return mapper( this, buffer ); - } else { - let value = new Structure(signature, []); - for(let i = 0; i < size; i++) { - value.fields.push(this.unpack(buffer)); - } - return value; + const number = this._unpackNumber(marker, buffer); + if (number !== null) { + return number; + } + + const string = this._unpackString(marker, markerHigh, markerLow, buffer); + if (string !== null) { + return string; + } + + const list = this._unpackList(marker, markerHigh, markerLow, buffer); + if (list !== null) { + return list; } + + const byteArray = this._unpackByteArray(marker, buffer); + if (byteArray !== null) { + return byteArray; + } + + const map = this._unpackMap(marker, markerHigh, markerLow, buffer); + if (map !== null) { + return map; + } + + const struct = this._unpackStruct(marker, markerHigh, markerLow, buffer); + if (struct !== null) { + return struct; + } + + throw newError('Unknown packed value with marker ' + marker.toString(16)); } - unpack ( buffer ) { - let marker = buffer.readUInt8(); - if (marker == NULL) { - return null; - } else if (marker == TRUE) { + _unpackBoolean(marker) { + if (marker == TRUE) { return true; } else if (marker == FALSE) { return false; - } else if (marker == FLOAT_64) { + } else { + return null; + } + } + + _unpackNumber(marker, buffer) { + if (marker == FLOAT_64) { return buffer.readFloat64(); } else if (marker >= 0 && marker < 128) { return int(marker); @@ -378,55 +389,122 @@ class Unpacker { return int(b); } else if (marker == INT_64) { let high = buffer.readInt32(); - let low = buffer.readInt32(); - return new Integer( low, high ); + let low = buffer.readInt32(); + return new Integer(low, high); + } else { + return null; + } + } + + _unpackString(marker, markerHigh, markerLow, buffer) { + if (markerHigh == TINY_STRING) { + return utf8.decode(buffer, markerLow); } else if (marker == STRING_8) { - return utf8.decode( buffer, buffer.readUInt8()); + return utf8.decode(buffer, buffer.readUInt8()); } else if (marker == STRING_16) { - return utf8.decode( buffer, buffer.readUInt16() ); + return utf8.decode(buffer, buffer.readUInt16()); } else if (marker == STRING_32) { - return utf8.decode( buffer, buffer.readUInt32() ); + return utf8.decode(buffer, buffer.readUInt32()); + } else { + return null; + } + } + + _unpackList(marker, markerHigh, markerLow, buffer) { + if (markerHigh == TINY_LIST) { + return this._unpackListWithSize(markerLow, buffer); } else if (marker == LIST_8) { - return this.unpackList(buffer.readUInt8(), buffer); + return this._unpackListWithSize(buffer.readUInt8(), buffer); } else if (marker == LIST_16) { - return this.unpackList(buffer.readUInt16(), buffer); + return this._unpackListWithSize(buffer.readUInt16(), buffer); } else if (marker == LIST_32) { - return this.unpackList(buffer.readUInt32(), buffer); - } else if (marker == BYTES_8) { - return this.unpackBytes(buffer.readUInt8(), buffer); + return this._unpackListWithSize(buffer.readUInt32(), buffer); + } else { + return null; + } + } + + _unpackListWithSize(size, buffer) { + let value = []; + for (let i = 0; i < size; i++) { + value.push(this.unpack(buffer)); + } + return value; + } + + _unpackByteArray(marker, buffer) { + if (marker == BYTES_8) { + return this._unpackByteArrayWithSize(buffer.readUInt8(), buffer); } else if (marker == BYTES_16) { - return this.unpackBytes(buffer.readUInt16(), buffer); + return this._unpackByteArrayWithSize(buffer.readUInt16(), buffer); } else if (marker == BYTES_32) { - return this.unpackBytes(buffer.readUInt32(), buffer); + return this._unpackByteArrayWithSize(buffer.readUInt32(), buffer); + } else { + return null; + } + } + + _unpackByteArrayWithSize(size, buffer) { + const value = new Int8Array(size); + for (let i = 0; i < size; i++) { + value[i] = buffer.readInt8(); + } + return value; + } + + _unpackMap(marker, markerHigh, markerLow, buffer) { + if (markerHigh == TINY_MAP) { + return this._unpackMapWithSize(markerLow, buffer); } else if (marker == MAP_8) { - return this.unpackMap(buffer.readUInt8(), buffer); + return this._unpackMapWithSize(buffer.readUInt8(), buffer); } else if (marker == MAP_16) { - return this.unpackMap(buffer.readUInt16(), buffer); + return this._unpackMapWithSize(buffer.readUInt16(), buffer); } else if (marker == MAP_32) { - return this.unpackMap(buffer.readUInt32(), buffer); + return this._unpackMapWithSize(buffer.readUInt32(), buffer); + } else { + return null; + } + } + + _unpackMapWithSize(size, buffer) { + let value = {}; + for (let i = 0; i < size; i++) { + let key = this.unpack(buffer); + value[key] = this.unpack(buffer); + } + return value; + } + + _unpackStruct(marker, markerHigh, markerLow, buffer) { + if (markerHigh == TINY_STRUCT) { + return this._unpackStructWithSize(markerLow, buffer); } else if (marker == STRUCT_8) { - return this.unpackStruct(buffer.readUInt8(), buffer); + return this._unpackStructWithSize(buffer.readUInt8(), buffer); } else if (marker == STRUCT_16) { - return this.unpackStruct(buffer.readUInt16(), buffer); - } - let markerHigh = marker & 0xF0; - let markerLow = marker & 0x0F; - if (markerHigh == 0x80) { - return utf8.decode( buffer, markerLow ); - } else if (markerHigh == 0x90) { - return this.unpackList(markerLow, buffer); - } else if (markerHigh == 0xA0) { - return this.unpackMap(markerLow, buffer); - } else if (markerHigh == 0xB0) { - return this.unpackStruct(markerLow, buffer); + return this._unpackStructWithSize(buffer.readUInt16(), buffer); + } else { + return null; + } + } + + _unpackStructWithSize(size, buffer) { + let signature = buffer.readUInt8(); + let mapper = this.structMappers[signature]; + if (mapper) { + return mapper(this, buffer); } else { - throw newError("Unknown packed value with marker " + marker.toString(16)); + let value = new Structure(signature, []); + for (let i = 0; i < size; i++) { + value.fields.push(this.unpack(buffer)); + } + return value; } } + } export { Packer, Unpacker, Structure -} +};