Skip to content

Commit

Permalink
Fix formatting of large duration values (#1958)
Browse files Browse the repository at this point in the history
* Remove luxon duration formatter

* fix duration issue

* self review

* fix unit test

* fix e2e test and add new test

* update type spec

* fix tests and restore older rendering in ascii view
  • Loading branch information
OskarDamkjaer authored Apr 3, 2024
1 parent 364127c commit fc60d6a
Show file tree
Hide file tree
Showing 9 changed files with 328 additions and 341 deletions.
320 changes: 206 additions & 114 deletions LICENSES.txt

Large diffs are not rendered by default.

207 changes: 43 additions & 164 deletions NOTICE.txt

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions e2e_tests/integration/types.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ describe('Types in Browser', () => {
cy.executeCommand(query)
cy.waitForCommandResult()

cy.resultContains('P1Y2M3DT4H5M6S')
cy.resultContains('P14M3DT14706S')
// Go to ascii view
cy.get('[data-testid="cypherFrameSidebarAscii"]').first().click()
cy.resultContains('│P1Y2M3DT4H5M6S')
cy.resultContains('│"P14M3DT14706S')
})
it('presents time type correctly', () => {
cy.executeCommand(':clear')
Expand Down Expand Up @@ -143,7 +143,7 @@ describe('Types in Browser', () => {
// cy.waitForCommandResult()
cy.get('circle.b-outline', { timeout: 10000 }).click()
cy.get('[data-testid="vizInspector"]')
.should('contain', 'P11M2DT2H19')
.should('contain', 'P11M2DT8363.857000000S')
.and('contain', 'srid:4326')
.and('contain', 'elementId')
})
Expand All @@ -154,7 +154,7 @@ describe('Types in Browser', () => {
// cy.waitForCommandResult()
cy.get('circle.b-outline', { timeout: 10000 }).click()
cy.get('[data-testid="vizInspector"]')
.should('contain', 'P11M2DT2H19')
.should('contain', 'P11M2DT8363.857000000S')
.and('contain', 'srid:4326')
.and('contain', 'date')
.and('contain', 'location')
Expand Down
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
"@types/jest": "^24.9.0",
"@types/jsonic": "^0.3.0",
"@types/lodash-es": "^4.17.6",
"@types/luxon": "^2.0.3",
"@types/react": "^17.0.40",
"@types/react-dom": "^17.0.11",
"@types/react-redux": "^7.1.9",
Expand Down Expand Up @@ -204,7 +203,6 @@
"jsonic": "^0.3.0",
"jszip": "3.8.0",
"lodash-es": "^4.17.21",
"luxon": "2.5.2",
"memoize-one": "^5.2.1",
"monaco-editor": "0.23.0",
"neo4j-client-sso": "1.2.3",
Expand Down
41 changes: 34 additions & 7 deletions src/browser/modules/Stream/CypherFrame/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import neo4j, { Record } from 'neo4j-driver'
import neo4j, { Integer, Record } from 'neo4j-driver'

import * as viewTypes from 'shared/modules/frames/frameViewTypes'
import { BrowserRequestResult } from 'shared/modules/requests/requestsDuck'
import {
recordToStringArray,
getRecordsToDisplayInTable,
initialView,
recordToJSONMapper,
recordToStringArray,
resultHasNodes,
resultHasPlan,
resultHasRows,
resultHasWarnings,
resultIsError
} from './helpers'
import * as viewTypes from 'shared/modules/frames/frameViewTypes'
import { BrowserRequestResult } from 'shared/modules/requests/requestsDuck'

describe('helpers', () => {
test('getRecordsToDisplayInTable should report if there are rows or not in the result', () => {
Expand Down Expand Up @@ -646,7 +646,7 @@ describe('helpers', () => {
const res = records.map(record => recordToStringArray(record))

// Then
expect(res).toEqual([['P1M2DT3S']])
expect(res).toEqual([['"P1M2DT3.000000004S"']])
})
})

Expand Down Expand Up @@ -846,7 +846,34 @@ describe('helpers', () => {
elementType: 'node',
labels: ['foo'],
properties: {
bar: 'P2012Y2M2DT14H37M21.545S'
bar: 'P24146M2DT52641.545000000S'
}
}
}

expect(recordToJSONMapper(record)).toEqual(expected)
})

test('handles duration above js safe number', () => {
const node = new (neo4j.types.Node as any)(1, ['foo'], {
bar: new neo4j.types.Duration(
new Integer(0),
new Integer(0),
// RETURN 9223372036854775807 which is maxiumum integer in neo4j
new Integer(-1, 2147483647),
new Integer(0)
)
})
const record = new (neo4j.types.Record as any)(['n'], [node])
const expected = {
n: {
elementId: '1',
identity: 1,
elementType: 'node',
labels: ['foo'],
properties: {
// get same number as above
bar: 'P0M0DT9223372036854775807S'
}
}
}
Expand All @@ -866,7 +893,7 @@ describe('helpers', () => {
elementType: 'node',
labels: ['foo'],
properties: {
bar: 'PT1M40S'
bar: 'P0M0DT100S'
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/browser/modules/Stream/CypherFrame/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,11 @@ import {
some
} from 'lodash-es'
import { CypherDataType, isCypherPropertyType } from 'neo4j-arc/common'
import neo4j, { Path, Record, Node, Relationship } from 'neo4j-driver'
import neo4j, { Node, Path, Record, Relationship } from 'neo4j-driver'

import bolt from 'services/bolt/bolt'
import { recursivelyExtractGraphItems } from 'services/bolt/boltMappings'
import {
durationFormat,
stringModifier
} from 'services/bolt/cypherTypesFormatting'
import { stringModifier } from 'services/bolt/cypherTypesFormatting'
import { stringifyMod, unescapeDoubleQuotesForDisplay } from 'services/utils'
import * as viewTypes from 'shared/modules/frames/frameViewTypes'
import { BrowserRequestResult } from 'shared/modules/requests/requestsDuck'
Expand Down Expand Up @@ -396,7 +393,6 @@ export function mapNeo4jValuesToPlainValues(values: any): any {
function neo4jValueToPlainValue(value: any) {
switch (get(value, 'constructor')) {
case neo4j.types.Duration:
return durationFormat(value)
case neo4j.types.Date:
case neo4j.types.DateTime:
case neo4j.types.LocalDateTime:
Expand Down
60 changes: 38 additions & 22 deletions src/browser/modules/Stream/Queries/QueriesFrame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ import { connect } from 'react-redux'
import { withBus } from 'react-suber'
import { Bus } from 'suber'

import { ConfirmationButton } from 'browser-components/buttons/ConfirmationButton'
import { Duration } from 'neo4j-driver'
import { GlobalState } from 'project-root/src/shared/globalState'
import { gte } from 'semver'
import { NEO4J_BROWSER_USER_ACTION_QUERY } from 'services/bolt/txMetadata'
import {
CONNECTED_STATE,
getConnectionState
} from 'shared/modules/connections/connectionsDuck'
import { CYPHER_REQUEST } from 'shared/modules/cypher/cypherDuck'
import {
getRawVersion,
getSemanticVersion,
hasProcedure,
isOnCluster
} from 'shared/modules/dbMeta/dbMetaDuck'
import { Frame } from 'shared/modules/frames/framesDuck'
import FrameBodyTemplate from '../../Frame/FrameBodyTemplate'
import FrameError from '../../Frame/FrameError'
import {
Expand All @@ -30,6 +47,9 @@ import {
StatusbarWrapper,
StyledStatusBar
} from '../AutoRefresh/styled'
import LegacyQueriesFrame, {
LegacyQueriesFrameProps
} from './LegacyQueriesFrame'
import {
Code,
StyledHeaderRow,
Expand All @@ -38,26 +58,6 @@ import {
StyledTd,
StyledTh
} from './styled'
import { ConfirmationButton } from 'browser-components/buttons/ConfirmationButton'
import { GlobalState } from 'project-root/src/shared/globalState'
import { NEO4J_BROWSER_USER_ACTION_QUERY } from 'services/bolt/txMetadata'
import {
CONNECTED_STATE,
getConnectionState
} from 'shared/modules/connections/connectionsDuck'
import { CYPHER_REQUEST } from 'shared/modules/cypher/cypherDuck'
import { durationFormat } from 'services/bolt/cypherTypesFormatting'
import { Frame } from 'shared/modules/frames/framesDuck'
import LegacyQueriesFrame, {
LegacyQueriesFrameProps
} from './LegacyQueriesFrame'
import {
getRawVersion,
getSemanticVersion,
hasProcedure,
isOnCluster
} from 'shared/modules/dbMeta/dbMetaDuck'
import { gte } from 'semver'

type QueriesFrameState = {
queries: any[]
Expand All @@ -84,6 +84,22 @@ function constructOverviewMessage(queries: any, errors: string[]) {
: successMessage
}

function prettyPrintDuration(duration: Duration) {
const { months, days, seconds, nanoseconds } = duration

let resultsString = ''
if (months.toNumber() > 0) {
resultsString += `${months} months, `
}
if (days.toNumber() > 0) {
resultsString += `${days} days, `
}
const millis = seconds.toNumber() * 1000 + nanoseconds.toNumber() / 1000000
resultsString += `${millis} ms`

return resultsString
}

export class QueriesFrame extends Component<
QueriesFrameProps,
QueriesFrameState
Expand Down Expand Up @@ -153,7 +169,7 @@ export class QueriesFrame extends Component<
...data,
host: `neo4j://${nonNullHost}`,
query: data.currentQuery,
elapsedTimeMillis: durationFormat(data.elapsedTime),
elapsedTimeMillis: prettyPrintDuration(data.elapsedTime),
queryId: data.transactionId
}
}
Expand Down Expand Up @@ -267,7 +283,7 @@ export class QueriesFrame extends Component<
<Code>{JSON.stringify(query.metaData, null, 2)}</Code>
</StyledTd>
<StyledTd key="time" width={tableHeaderSizes[5][1]}>
{query.elapsedTimeMillis} ms
{query.elapsedTimeMillis}
</StyledTd>
<StyledTd key="actions" width={tableHeaderSizes[6][1]}>
<ConfirmationButton
Expand Down
13 changes: 1 addition & 12 deletions src/shared/services/bolt/cypherTypesFormatting.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Duration } from 'luxon'
import neo4j from 'neo4j-driver'

export const csvFormat = (anything: any) => {
Expand Down Expand Up @@ -31,21 +30,11 @@ export const stringModifier = (anything: any) => {
return spacialFormat(anything)
}
if (isTemporalType(anything)) {
if (isDuration(anything)) {
return durationFormat(anything)
} else {
return `"${anything.toString()}"`
}
return `"${anything.toString()}"`
}
return undefined
}

export const durationFormat = (duration: typeof neo4j.types.Duration): string =>
Duration.fromISO(duration.toString())
.shiftTo('years', 'days', 'months', 'hours', 'minutes', 'seconds')
.normalize()
.toISO()

const numberFormat = (anything: any) => {
// Exclude false positives and return early
if ([Infinity, -Infinity, NaN].includes(anything)) {
Expand Down
10 changes: 0 additions & 10 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2366,11 +2366,6 @@
resolved "https://registry.npmjs.org/@types/long/-/long-4.0.1.tgz"
integrity "sha1-RZxl+hhn2v5qjzIsTFFpVmPMVek= sha512-5tXH6Bx/kNGd3MgffdmP4dy2Z+G4eaXw0SE81Tq3BNadtnMR5/ySMzX4SLEzHJzSmPNn4HIdpQsBvXMUykr58w=="

"@types/luxon@^2.0.3":
version "2.4.0"
resolved "https://registry.npmjs.org/@types/luxon/-/luxon-2.4.0.tgz"
integrity sha512-oCavjEjRXuR6URJEtQm0eBdfsBiEcGBZbq21of8iGkeKxU1+1xgKuFPClaBZl2KB8ZZBSWlgk61tH6Mf+nvZVw==

"@types/mdast@^3.0.0":
version "3.0.3"
resolved "https://registry.npmjs.org/@types/mdast/-/mdast-3.0.3.tgz"
Expand Down Expand Up @@ -9096,11 +9091,6 @@ lru-cache@^6.0.0:
dependencies:
yallist "^4.0.0"

luxon@2.5.2:
version "2.5.2"
resolved "https://registry.npmjs.org/luxon/-/luxon-2.5.2.tgz"
integrity sha512-Yg7/RDp4nedqmLgyH0LwgGRvMEKVzKbUdkBYyCosbHgJ+kaOUx0qzSiSatVc3DFygnirTPYnMM2P5dg2uH1WvA==

lz-string@^1.4.4:
version "1.4.4"
resolved "https://registry.npmjs.org/lz-string/-/lz-string-1.4.4.tgz"
Expand Down

0 comments on commit fc60d6a

Please sign in to comment.