From 0ca8b3705f2dd86b00843e3bac6fefec1ec22c55 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Mon, 28 Oct 2019 13:47:23 +0100 Subject: [PATCH 1/3] Added missing system updates Changed updates to a dictionary. --- src/result-summary.js | 84 ++++---------------- test/rx/summary.test.js | 125 +++++++++++++++++++++++++----- test/session.test.js | 8 +- test/summary.test.js | 26 ++++--- test/transaction.test.js | 2 +- test/types/result-summary.test.ts | 14 +--- types/result-summary.d.ts | 22 +----- 7 files changed, 145 insertions(+), 136 deletions(-) diff --git a/src/result-summary.js b/src/result-summary.js index 2b7b03724..41ebdd371 100644 --- a/src/result-summary.js +++ b/src/result-summary.js @@ -206,11 +206,13 @@ class StatementStatistics { constraintsAdded: 0, constraintsRemoved: 0 } + this._systemUpdates = 0 Object.keys(statistics).forEach(index => { // To camelCase - this._stats[index.replace(/(-\w)/g, m => m[1].toUpperCase())] = intValue( - statistics[index] - ) + const camelCaseIndex = index.replace(/(-\w)/g, m => m[1].toUpperCase()) + if (camelCaseIndex in this._stats) { this._stats[camelCaseIndex] = intValue(statistics[index]) } else if (camelCaseIndex === 'systemUpdates') { + this._systemUpdates = intValue(statistics[index]) + } }) } @@ -227,80 +229,26 @@ class StatementStatistics { } /** - * @return {Number} - Number of nodes created. + * Returns the statement statistics updates in a dictionary. + * @returns {*} */ - nodesCreated () { - return this._stats.nodesCreated + updates () { + return this._stats } /** - * @return {Number} - Number of nodes deleted. + * Return true if the system database get updated, otherwise false + * @returns {boolean} - If the system database get updated or not. */ - nodesDeleted () { - return this._stats.nodesDeleted + containsSystemUpdates () { + return this._systemUpdates > 0 } /** - * @return {Number} - Number of relationships created. + * @returns {number} - Number of system updates */ - relationshipsCreated () { - return this._stats.relationshipsCreated - } - - /** - * @return {Number} - Number of nodes deleted. - */ - relationshipsDeleted () { - return this._stats.relationshipsDeleted - } - - /** - * @return {Number} - Number of properties set. - */ - propertiesSet () { - return this._stats.propertiesSet - } - - /** - * @return {Number} - Number of labels added. - */ - labelsAdded () { - return this._stats.labelsAdded - } - - /** - * @return {Number} - Number of labels removed. - */ - labelsRemoved () { - return this._stats.labelsRemoved - } - - /** - * @return {Number} - Number of indexes added. - */ - indexesAdded () { - return this._stats.indexesAdded - } - - /** - * @return {Number} - Number of indexes removed. - */ - indexesRemoved () { - return this._stats.indexesRemoved - } - - /** - * @return {Number} - Number of constraints added. - */ - constraintsAdded () { - return this._stats.constraintsAdded - } - - /** - * @return {Number} - Number of constraints removed. - */ - constraintsRemoved () { - return this._stats.constraintsRemoved + systemUpdates () { + return this._systemUpdates } } diff --git a/test/rx/summary.test.js b/test/rx/summary.test.js index c1ea1b88d..782801996 100644 --- a/test/rx/summary.test.js +++ b/test/rx/summary.test.js @@ -201,6 +201,43 @@ describe('#integration-rx summary', () => { shouldReturnNotification(serverVersion, txc)) }) + describe('system', () => { + let driver + /** @type {RxSession} */ + let session + /** @type {ServerVersion} */ + let serverVersion + + beforeEach(async () => { + driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken) + session = driver.rxSession({ database: 'system' }) + // + + const normalSession = driver.session() + try { + const result = await normalSession.run('MATCH (n) DETACH DELETE n') + serverVersion = ServerVersion.fromString(result.summary.server.version) + } finally { + await normalSession.close() + } + + await dropConstraintsAndIndices(driver) + }) + + afterEach(async () => { + if (session) { + await session.close().toPromise() + } + await driver.close() + }) + + it('session should return summary with correct system updates for create', () => + shouldReturnSummaryWithSystemUpdates(serverVersion, session)) + + it('transaction should return summary with correct system updates for create', () => + shouldReturnSummaryWithSystemUpdates(serverVersion, session, true)) + }) + /** * @param {ServerVersion} version * @param {RxSession|RxTransaction} runnable @@ -269,6 +306,37 @@ describe('#integration-rx summary', () => { await verifyStatementType(runnable, 'CREATE (n) RETURN n', 'rw') } + /** + * @param {ServerVersion} version + * @param {RxSession} session + * @param {boolean} useTransaction + */ + async function shouldReturnSummaryWithSystemUpdates ( + version, + session, + useTransaction = false + ) { + if (version.compareTo(VERSION_4_0_0) < 0) { + return + } + + let runnable = session + if (useTransaction) { + runnable = await session.beginTransaction().toPromise() + } + + try { + await verifySystemUpdates( + runnable, + "CREATE USER foo SET PASSWORD 'bar'", + {}, + 1 + ) + } finally { + await verifySystemUpdates(runnable, 'DROP USER foo', {}, 1) + } + } + /** * @param {ServerVersion} version * @param {RxSession|RxTransaction} runnable @@ -281,7 +349,7 @@ describe('#integration-rx summary', () => { return } - await verifyCounters( + await verifyUpdates( runnable, 'CREATE (n:Label1 {id: $id1})-[:KNOWS]->(m:Label2 {id: $id2}) RETURN n, m', { id1: 10, id2: 20 }, @@ -316,7 +384,7 @@ describe('#integration-rx summary', () => { // first create the to-be-deleted nodes await shouldReturnSummaryWithUpdateStatisticsForCreate(version, runnable) - await verifyCounters( + await verifyUpdates( runnable, 'MATCH (n:Label1)-[r:KNOWS]->(m:Label2) DELETE n, r', null, @@ -348,7 +416,7 @@ describe('#integration-rx summary', () => { return } - await verifyCounters(runnable, 'CREATE INDEX on :Label(prop)', null, { + await verifyUpdates(runnable, 'CREATE INDEX on :Label(prop)', null, { nodesCreated: 0, nodesDeleted: 0, relationshipsCreated: 0, @@ -384,7 +452,7 @@ describe('#integration-rx summary', () => { await session.close() } - await verifyCounters(runnable, 'DROP INDEX on :Label(prop)', null, { + await verifyUpdates(runnable, 'DROP INDEX on :Label(prop)', null, { nodesCreated: 0, nodesDeleted: 0, relationshipsCreated: 0, @@ -411,7 +479,7 @@ describe('#integration-rx summary', () => { return } - await verifyCounters( + await verifyUpdates( runnable, 'CREATE CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE', null, @@ -454,7 +522,7 @@ describe('#integration-rx summary', () => { await session.close() } - await verifyCounters( + await verifyUpdates( runnable, 'DROP CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE', null, @@ -628,27 +696,42 @@ describe('#integration-rx summary', () => { * @param {RxSession|RxTransaction} runnable * @param {string} statement * @param {*} parameters - * @param {*} counters + * @param {*} stats + */ + async function verifyUpdates (runnable, statement, parameters, stats) { + const summary = await runnable + .run(statement, parameters) + .summary() + .toPromise() + expect(summary).toBeDefined() + expect(summary.counters.containsUpdates()).toBeTruthy() + expect(summary.counters.updates()).toEqual(stats) + expect(summary.counters.containsSystemUpdates()).toBeFalsy() + } + + /** + * + * @param {RxSession|RxTransaction} runnable + * @param {string} statement + * @param {*} parameters + * @param {number} systemUpdates + * @returns {Promise} */ - async function verifyCounters (runnable, statement, parameters, counters) { + async function verifySystemUpdates ( + runnable, + statement, + parameters, + systemUpdates + ) { const summary = await runnable .run(statement, parameters) .summary() .toPromise() expect(summary).toBeDefined() - expect({ - nodesCreated: summary.counters.nodesCreated(), - nodesDeleted: summary.counters.nodesDeleted(), - relationshipsCreated: summary.counters.relationshipsCreated(), - relationshipsDeleted: summary.counters.relationshipsDeleted(), - propertiesSet: summary.counters.propertiesSet(), - labelsAdded: summary.counters.labelsAdded(), - labelsRemoved: summary.counters.labelsRemoved(), - indexesAdded: summary.counters.indexesAdded(), - indexesRemoved: summary.counters.indexesRemoved(), - constraintsAdded: summary.counters.constraintsAdded(), - constraintsRemoved: summary.counters.constraintsRemoved() - }).toEqual(counters) + + expect(summary.counters.containsSystemUpdates()).toBeTruthy() + expect(summary.counters.systemUpdates()).toBe(systemUpdates) + expect(summary.counters.containsUpdates()).toBeFalsy() } async function dropConstraintsAndIndices (driver) { diff --git a/test/session.test.js b/test/session.test.js index 829667bf5..43a13395c 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -223,7 +223,7 @@ describe('#integration session', () => { expect(sum.statement.text).toBe(statement) expect(sum.statement.parameters).toBe(params) expect(sum.counters.containsUpdates()).toBe(true) - expect(sum.counters.nodesCreated()).toBe(1) + expect(sum.counters.updates().nodesCreated).toBe(1) expect(sum.statementType).toBe(statementType.READ_WRITE) done() }) @@ -649,7 +649,7 @@ describe('#integration session', () => { resultPromise.then(result => { expect(result.records.length).toEqual(1) expect(result.records[0].get('answer').toNumber()).toEqual(42) - expect(result.summary.counters.nodesCreated()).toEqual(1) + expect(result.summary.counters.updates().nodesCreated).toEqual(1) const bookmarkAfter = session.lastBookmark() verifyBookmark(bookmarkAfter) @@ -715,7 +715,7 @@ describe('#integration session', () => { expect(result.records.length).toEqual(1) expect(result.records[0].get('answer').toNumber()).toEqual(42) - expect(result.summary.counters.nodesCreated()).toEqual(1) + expect(result.summary.counters.updates().nodesCreated).toEqual(1) countNodes('Node', 'id', 42).then(count => { expect(count).toEqual(1) @@ -768,7 +768,7 @@ describe('#integration session', () => { resultPromise.then(result => { expect(result.records.length).toEqual(1) expect(result.records[0].get('answer').toNumber()).toEqual(42) - expect(result.summary.counters.nodesCreated()).toEqual(1) + expect(result.summary.counters.updates().nodesCreated).toEqual(1) expect(session.lastBookmark()).toBe(bookmarkBefore) // expect bookmark to not change countNodes('Node', 'id', 42).then(count => { diff --git a/test/summary.test.js b/test/summary.test.js index 839168edb..35203464b 100644 --- a/test/summary.test.js +++ b/test/summary.test.js @@ -107,18 +107,20 @@ describe('#integration result summary', () => { expect(summary.resultConsumedAfter).toBeDefined() expect(summary.resultAvailableAfter).toBeDefined() - const counters = summary.counters - expect(counters.nodesCreated()).toBe(1) - expect(counters.nodesDeleted()).toBe(0) - expect(counters.relationshipsCreated()).toBe(0) - expect(counters.relationshipsDeleted()).toBe(0) - expect(counters.propertiesSet()).toBe(1) - expect(counters.labelsAdded()).toBe(1) - expect(counters.labelsRemoved()).toBe(0) - expect(counters.indexesAdded()).toBe(0) - expect(counters.indexesRemoved()).toBe(0) - expect(counters.constraintsAdded()).toBe(0) - expect(counters.constraintsRemoved()).toBe(0) + expect(summary.counters.containsUpdates()).toBe(true) + const stats = summary.counters.updates() + expect(stats.nodesCreated).toBe(1) + expect(stats.nodesDeleted).toBe(0) + expect(stats.relationshipsCreated).toBe(0) + expect(stats.relationshipsDeleted).toBe(0) + expect(stats.propertiesSet).toBe(1) + expect(stats.labelsAdded).toBe(1) + expect(stats.labelsRemoved).toBe(0) + expect(stats.indexesAdded).toBe(0) + expect(stats.indexesRemoved).toBe(0) + expect(stats.constraintsAdded).toBe(0) + expect(stats.constraintsRemoved).toBe(0) + expect(summary.counters.containsSystemUpdates()).toBe(false) done() }) } diff --git a/test/transaction.test.js b/test/transaction.test.js index 6d35ff37a..74346edcd 100644 --- a/test/transaction.test.js +++ b/test/transaction.test.js @@ -374,7 +374,7 @@ describe('#integration transaction', () => { tx1 .run('CREATE ()') .then(result => { - expect(result.summary.counters.nodesCreated()).toBe(1) + expect(result.summary.counters.updates().nodesCreated).toBe(1) tx1 .commit() diff --git a/test/types/result-summary.test.ts b/test/types/result-summary.test.ts index 90143b913..6d504edf2 100644 --- a/test/types/result-summary.test.ts +++ b/test/types/result-summary.test.ts @@ -40,17 +40,9 @@ const str: string = sum1.statementType const counters: StatementStatistic = sum1.counters const containsUpdates: boolean = counters.containsUpdates() -const nodesCreated: number = counters.nodesCreated() -const nodesDeleted: number = counters.nodesDeleted() -const relationshipsCreated: number = counters.relationshipsCreated() -const relationshipsDeleted: number = counters.relationshipsDeleted() -const propertiesSet: number = counters.propertiesSet() -const labelsAdded: number = counters.labelsAdded() -const labelsRemoved: number = counters.labelsRemoved() -const indexesAdded: number = counters.indexesAdded() -const indexesRemoved: number = counters.indexesRemoved() -const constraintsAdded: number = counters.constraintsAdded() -const constraintsRemoved: number = counters.constraintsRemoved() +const containsSystemUpdates: boolean = counters.containsSystemUpdates() +const systemUpdates: number = counters.systemUpdates() +const updates: { [key: string]: number } = counters.updates() const plan: Plan = sum1.plan const planOperatorType: string = plan.operatorType diff --git a/types/result-summary.d.ts b/types/result-summary.d.ts index fc14a0bcc..1c14273ef 100644 --- a/types/result-summary.d.ts +++ b/types/result-summary.d.ts @@ -55,27 +55,11 @@ declare interface ProfiledPlan { declare interface StatementStatistic { containsUpdates(): boolean - nodesCreated(): number + containsSystemUpdates(): boolean - nodesDeleted(): number + updates(): { [key: string]: number } - relationshipsCreated(): number - - relationshipsDeleted(): number - - propertiesSet(): number - - labelsAdded(): number - - labelsRemoved(): number - - indexesAdded(): number - - indexesRemoved(): number - - constraintsAdded(): number - - constraintsRemoved(): number + systemUpdates(): number } declare type NotificationPosition = { From 966e8c242c343444ccd428070af5737b33d36bd8 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Thu, 31 Oct 2019 13:57:06 +0100 Subject: [PATCH 2/3] bulalal --- src/result-summary.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/result-summary.js b/src/result-summary.js index 41ebdd371..2c0ada812 100644 --- a/src/result-summary.js +++ b/src/result-summary.js @@ -210,10 +210,14 @@ class StatementStatistics { Object.keys(statistics).forEach(index => { // To camelCase const camelCaseIndex = index.replace(/(-\w)/g, m => m[1].toUpperCase()) - if (camelCaseIndex in this._stats) { this._stats[camelCaseIndex] = intValue(statistics[index]) } else if (camelCaseIndex === 'systemUpdates') { + if (camelCaseIndex in this._stats) { + this._stats[camelCaseIndex] = intValue(statistics[index]) + } else if (camelCaseIndex === 'systemUpdates') { this._systemUpdates = intValue(statistics[index]) } }) + + this._stats = Object.freeze(this._stats) } /** From 73e3d4349edad3b9eb8e185aa1cae346fc6ca5d3 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Fri, 1 Nov 2019 11:10:51 +0100 Subject: [PATCH 3/3] Adding missing profile statistics --- src/result-summary.js | 25 +++++++++++++++++++++++-- test/session.test.js | 2 -- test/summary.test.js | 2 ++ test/types/result-summary.test.ts | 5 +++++ types/result-summary.d.ts | 7 +++++++ 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/result-summary.js b/src/result-summary.js index 2c0ada812..9573db53f 100644 --- a/src/result-summary.js +++ b/src/result-summary.js @@ -174,12 +174,24 @@ class ProfiledPlan { this.operatorType = profile.operatorType this.identifiers = profile.identifiers this.arguments = profile.args - this.dbHits = intValue(profile.args.DbHits) - this.rows = intValue(profile.args.Rows) + this.dbHits = valueOrDefault('dbHits', profile) + this.rows = valueOrDefault('rows', profile) + this.pageCacheMisses = valueOrDefault('pageCacheMisses', profile) + this.pageCacheHits = valueOrDefault('pageCacheHits', profile) + this.pageCacheHitRatio = valueOrDefault('pageCacheHitRatio', profile) + this.time = valueOrDefault('time', profile) this.children = profile.children ? profile.children.map(child => new ProfiledPlan(child)) : [] } + + hasPageCacheStats () { + return ( + this.pageCacheMisses > 0 || + this.pageCacheHits > 0 || + this.pageCacheHitRatio > 0 + ) + } } /** @@ -308,6 +320,15 @@ function intValue (value) { return isInt(value) ? value.toInt() : value } +function valueOrDefault (key, values, defaultValue = 0) { + if (key in values) { + const value = values[key] + return isInt(value) ? value.toInt() : value + } else { + return defaultValue + } +} + const statementType = { READ_ONLY: 'r', READ_WRITE: 'rw', diff --git a/test/session.test.js b/test/session.test.js index 43a13395c..fcb2a6ec0 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -25,7 +25,6 @@ import SingleConnectionProvider from '../src/internal/connection-provider-single import FakeConnection from './internal/fake-connection' import sharedNeo4j from './internal/shared-neo4j' import _ from 'lodash' -import { ServerVersion, VERSION_4_0_0 } from '../src/internal/server-version' import { isString } from '../src/internal/util' import testUtils from './internal/test-utils' import { newError, PROTOCOL_ERROR, SESSION_EXPIRED } from '../src/error' @@ -299,7 +298,6 @@ describe('#integration session', () => { expect(sum.profile.identifiers[0]).toBe('n') expect(sum.profile.children[0].operatorType).toBeDefined() expect(sum.profile.rows).toBe(0) - // expect(sum.profile.dbHits).toBeGreaterThan(0); done() }) }) diff --git a/test/summary.test.js b/test/summary.test.js index 35203464b..35556bede 100644 --- a/test/summary.test.js +++ b/test/summary.test.js @@ -153,6 +153,8 @@ describe('#integration result summary', () => { expect(profile.dbHits).toBe(0) expect(profile.rows).toBe(1) + expect(profile.time).toBe(0) + expect(profile.hasPageCacheStats()).toBe(false) done() }) diff --git a/test/types/result-summary.test.ts b/test/types/result-summary.test.ts index 6d504edf2..8f23fbed6 100644 --- a/test/types/result-summary.test.ts +++ b/test/types/result-summary.test.ts @@ -56,6 +56,11 @@ const profileIdentifiers: string[] = profile.identifiers const profileArguments: { [key: string]: string } = profile.arguments const profileDbHits: number = profile.dbHits const profileRows: number = profile.rows +const hasPageCacheStats: boolean = profile.hasPageCacheStats() +const profilePageCacheMisses: number = profile.pageCacheMisses +const profilePageCacheHits: number = profile.pageCacheHits +const profilePageCacheHitRatio: number = profile.pageCacheHitRatio +const time: number = profile.time const profileChildren: ProfiledPlan[] = profile.children const notifications: Notification[] = sum1.notifications diff --git a/types/result-summary.d.ts b/types/result-summary.d.ts index 1c14273ef..53100d850 100644 --- a/types/result-summary.d.ts +++ b/types/result-summary.d.ts @@ -49,6 +49,13 @@ declare interface ProfiledPlan { arguments: { [key: string]: string } dbHits: number rows: number + pageCacheMisses: number + pageCacheHits: number + pageCacheHitRatio: number + time: number + + hasPageCacheStats(): boolean + children: ProfiledPlan[] }