Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Fix bug where csv strings were triple quoted #1970

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Fix bug where csv strings were triple quoted #1970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/browser/modules/Stream/CypherFrame/CypherFrame.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export class CypherFrame extends Component<CypherFrameProps, CypherFrameState> {

const exportData = stringifyResultArray(
csvFormat,
[keys].concat(records.map(record => recordToStringArray(record)))
[keys].concat(records.map(record => recordToStringArray(record, true)))
)
const data = exportData.slice()
const csv = CSVSerializer(data.shift())
Expand Down
25 changes: 25 additions & 0 deletions src/browser/modules/Stream/CypherFrame/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,31 @@ describe('helpers', () => {
// Then
expect(res).toEqual([['"P1M2DT3.000000004S"']])
})

Copy link
Contributor

@OskarDamkjaer OskarDamkjaer Jun 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we test the records that come from:
return "hello " "
or
return '"'
or even
return '
"
'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> hello " ,
=> ",

=> "
""
"

Maybe it should become something like those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried these ones and the results end up looking like:

  1. return "hello \" ":
"""hello \"" """
"hello \"" "
  1. return '"':
"'""'"
"\"""
  1. return ' " ' (with new lines):
"'
""
'"
"
\""
"

Seems like they are all valid csv exports but let me check why in the first case the column name got triple quotes once again instead of just one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tried the below query to see different types as we've talked before.

CREATE (n {string: "string",boolean: true,integer: 1,float: 2.0,point: point({x:1,y:2}),date: date(),time: time(),datetime: datetime(),localtime: localtime(),localdatetime:localdatetime(),duration: duration("P5M1.5D"),numberList: [1,2,3],stringList: ["a"],emptyList: []}) RETURN n, n.string, n.boolean, n.integer, n.float, n.point, n.date, n.time, n.datetime, n.localtime, n.localdatetime, n.duration, n.numberList, n.stringList, n.emptyList

The exported CSV looks as follows:

n,n.string,n.boolean,n.integer,n.float,n.point,n.date,n.time,n.datetime,n.localtime,n.localdatetime,n.duration,n.numberList,n.stringList,n.emptyList
"({localtime: 10:21:41.776000000,date: 2024-06-06,string: string,integer: 1,float: 2.0,point: point({srid:7203, x:1, y:2}),duration: P5M1DT43200S,emptyList: [],datetime: 2024-06-06T10:21:41.776000000Z,boolean: true,stringList: [a],numberList: [1, 2, 3],time: 10:21:41.776000000Z,localdatetime: 2024-06-06T10:21:41.776000000})",string,true,1,2.0,"point({srid:7203, x:1, y:2})",2024-06-06,10:21:41.776000000Z,2024-06-06T10:21:41.776000000Z,10:21:41.776000000,2024-06-06T10:21:41.776000000,P5M1DT43200S,"[1, 2, 3]",[a],[]

which is a valid CSV without triple quotes 💃.

Thinking back on the triple quotes in the column name in the top comment, I believe it is expected as indeed the column name is "hello \" " rather than just hello \" . We can try to remove the quotes in that case as well but for that, we may need to add an if check which removes the quotes from the beginning and the end of the result record.

Will be testing the jar file and merge the pr if all is good.

test('recordToStringArray handles strings correctly when double quotes are disabled', () => {
const records = [
new Record(
['x', 'y', 'n', 'z', '{}'],
[
'xRecord',
neo4j.int(10),
new neo4j.types.Duration(1, 2, 3, 4),
new (neo4j.types.Node as any)('1', ['Person'], { prop1: 'prop1' }),
{}
]
)
]
const res = records.map(record => recordToStringArray(record, true))
expect(res).toEqual([
[
'xRecord',
'10',
'P1M2DT3.000000004S',
'(:Person {prop1: prop1})',
'{}'
]
])
})
})

describe('recordToJSONMapper', () => {
Expand Down
20 changes: 14 additions & 6 deletions src/browser/modules/Stream/CypherFrame/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import neo4j, { Node, Path, Record, Relationship } from 'neo4j-driver'
import bolt from 'services/bolt/bolt'
import { recursivelyExtractGraphItems } from 'services/bolt/boltMappings'
import { stringModifier } from 'services/bolt/cypherTypesFormatting'
import { stringifyMod, unescapeDoubleQuotesForDisplay } from 'services/utils'
import { stringifyMod } from 'services/utils'
import * as viewTypes from 'shared/modules/frames/frameViewTypes'
import { BrowserRequestResult } from 'shared/modules/requests/requestsDuck'

Expand Down Expand Up @@ -231,14 +231,13 @@ export const initialView = (props: any, state: any = {}) => {
*/
export const stringifyResultArray = (
formatter = stringModifier,
arr: any[] = [],
unescapeDoubleQuotes = false
arr: any[] = []
) => {
return arr.map(col => {
if (!col) return col
return col.map((fVal: any) => {
const res = stringifyMod(fVal, formatter)
return unescapeDoubleQuotes ? unescapeDoubleQuotesForDisplay(res) : res
return res
})
})
}
Expand Down Expand Up @@ -423,7 +422,10 @@ function isNeo4jValue(value: any) {
}
}

export const recordToStringArray = (record: Record): string[] => {
export const recordToStringArray = (
record: Record,
discardDoubleQuotes?: boolean
): string[] => {
const recursiveStringify = (value: CypherDataType): string => {
if (Array.isArray(value)) {
if (value.length === 0) return '[]'
Expand All @@ -432,7 +434,13 @@ export const recordToStringArray = (record: Record): string[] => {

if (isCypherPropertyType(value)) {
//Note: later we should use propertyToString here but needs to be updated to show year in durations.
return stringifyMod(value, stringModifier, true)
return stringifyMod(
value,
(v: any) => stringModifier(v, discardDoubleQuotes),
true,
false,
discardDoubleQuotes
)
}

// We have nodes, relationships, paths and cypher maps left.
Expand Down
9 changes: 7 additions & 2 deletions src/shared/services/bolt/cypherTypesFormatting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ export const csvFormat = (anything: any) => {
return undefined
}

export const stringModifier = (anything: any) => {
export const stringModifier = (
anything: any,
discardDoubleQuotes?: boolean
) => {
if (typeof anything === 'number') {
return numberFormat(anything)
}
Expand All @@ -30,7 +33,9 @@ export const stringModifier = (anything: any) => {
return spacialFormat(anything)
}
if (isTemporalType(anything)) {
return `"${anything.toString()}"`
return discardDoubleQuotes
? anything.toString()
: `"${anything.toString()}"`
}
return undefined
}
Expand Down
7 changes: 5 additions & 2 deletions src/shared/services/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ export const stringifyMod = (
value: any,
modFn: any = null,
pretty: boolean | number = false,
skipOpeningIndentation = false
skipOpeningIndentation = false,
discardDoubleQuotes = false
): string => {
const prettyLevel = isNumber(pretty) ? pretty : +pretty
const nextPrettyLevel = prettyLevel ? prettyLevel + 1 : false
Expand Down Expand Up @@ -386,7 +387,9 @@ export const stringifyMod = (
)}${newLine}${endIndentation}}`
}
}
return `${indentation}"${value.toString().replace(escRE, escFunc)}"`
return discardDoubleQuotes
? `${indentation}${value.toString().replace(escRE, escFunc)}`
: `${indentation}"${value.toString().replace(escRE, escFunc)}"`
}

export const unescapeDoubleQuotesForDisplay = (str: any) =>
Expand Down
Loading