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.

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

Allow database names to be escaped with back ticks with the :use command #1042

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 43 additions & 10 deletions e2e_tests/integration/multi-db.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@ describe('Multi database', () => {
cy.get('[data-testid="dbs-command-list"] li', {
timeout: 5000
})
const databaseOptionList = () =>
const databaseOptionListOptions = () =>
cy.get('[data-testid="database-selection-list"] option', {
timeout: 5000
})
const databaseOptionList = () =>
cy.get('[data-testid="database-selection-list"]', {
timeout: 5000
})

before(() => {
cy.visit(Cypress.config('url'))
Expand Down Expand Up @@ -83,19 +87,48 @@ describe('Multi database', () => {
it('lists databases in sidebar', () => {
cy.executeCommand(':clear')
cy.get('[data-testid="drawerDBMS"]').click()
databaseOptionList().should('have.length', 2)
databaseOptionListOptions().should('have.length', 2)
cy.get('[data-testid="drawerDBMS"]').click()
})
if (isEnterpriseEdition()) {
it('lists new databases in sidebar', () => {
it('adds databases to the sidebar and adds backticks to special db names', () => {
// Add db
cy.executeCommand(':use system')
cy.executeCommand('CREATE DATABASE sidebartest')
databaseOptionList().should('have.length', 3)
databaseOptionList().contains('system')
databaseOptionList().contains('neo4j')
databaseOptionList().contains('sidebartest')
cy.executeCommand('CREATE DATABASE `name-with-dash`')
cy.resultContains('1 system update')
cy.executeCommand(':clear')

cy.executeCommand('DROP DATABASE sidebartest')
databaseOptionList().should('have.length', 2)
// Count items in list
cy.get('[data-testid="drawerDBMS"]').click()
databaseOptionListOptions().should('have.length', 3)
databaseOptionListOptions().contains('system')
databaseOptionListOptions().contains('neo4j')
databaseOptionListOptions().contains('name-with-dash')

// Select to use db, make sure backticked
databaseOptionList().select('name-with-dash')
cy.get('[data-testid="frameCommand"]', { timeout: 10000 })
.first()
.should('contain', ':use `name-with-dash`')
cy.resultContains(
'Queries from this point and forward are using the database'
)

// Try without backticks
cy.executeCommand(':use system')
cy.resultContains(
'Queries from this point and forward are using the database'
)
cy.executeCommand(':clear')
cy.executeCommand(':use name-with-dash')
cy.resultContains(
'Queries from this point and forward are using the database'
)

// Cleanup
cy.executeCommand(':use system')
cy.executeCommand('DROP DATABASE `name-with-dash`')
databaseOptionListOptions().should('have.length', 2)
cy.get('[data-testid="drawerDBMS"]').click()
})
}
Expand Down
3 changes: 2 additions & 1 deletion src/browser/modules/DBMSInfo/DatabaseSelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
DrawerSectionBody
} from 'browser-components/drawer/index'
import { uniqBy } from 'lodash-es'
import { escapeCypherIdentifier } from 'services/utils'

const Select = styled.select`
width: 100%;
Expand All @@ -47,7 +48,7 @@ export const DatabaseSelector = ({
if (target.value === EMPTY_OPTION) {
return
}
onChange(target.value)
onChange(escapeCypherIdentifier(target.value))
}

let databasesList = databases
Expand Down
28 changes: 28 additions & 0 deletions src/browser/modules/DBMSInfo/DatabaseSelector.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,32 @@ describe('DatabaseSelector', () => {
expect(onChange).toHaveBeenCalledTimes(2)
expect(onChange).toHaveBeenLastCalledWith('stella')
})
it('escapes db names when needed', () => {
// Given
const databases = [
{ name: 'regulardb', status: 'online' },
{ name: 'db-with-dash', status: 'online' }
]
const onChange = jest.fn()

// When
const { getByTestId } = render(
<DatabaseSelector databases={databases} onChange={onChange} />
)
const select = getByTestId(testId)

// Select something
fireEvent.change(select, { target: { value: 'regulardb' } })

// Then
expect(onChange).toHaveBeenCalledTimes(1)
expect(onChange).toHaveBeenLastCalledWith('regulardb')

// Select something else
fireEvent.change(select, { target: { value: 'db-with-dash' } })

// Then
expect(onChange).toHaveBeenCalledTimes(2)
expect(onChange).toHaveBeenLastCalledWith('`db-with-dash`')
})
})
18 changes: 9 additions & 9 deletions src/browser/modules/DBMSInfo/MetaItems.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import { ecsapeCypherMetaItem } from 'services/utils'
import { escapeCypherIdentifier } from 'services/utils'
import classNames from 'classnames'
import styles from './style_meta.css'
import {
Expand Down Expand Up @@ -96,7 +96,7 @@ const LabelItems = ({
if (i === 0) {
return 'MATCH (n) RETURN n LIMIT 25'
}
return `MATCH (n:${ecsapeCypherMetaItem(text)}) RETURN n LIMIT 25`
return `MATCH (n:${escapeCypherIdentifier(text)}) RETURN n LIMIT 25`
}
labelItems = createItems(
labels,
Expand Down Expand Up @@ -140,7 +140,7 @@ const RelationshipItems = ({
if (i === 0) {
return 'MATCH p=()-->() RETURN p LIMIT 25'
}
return `MATCH p=()-[r:${ecsapeCypherMetaItem(
return `MATCH p=()-[r:${escapeCypherIdentifier(
text
)}]->() RETURN p LIMIT 25`
}
Expand Down Expand Up @@ -182,17 +182,17 @@ const PropertyItems = ({
let propertyItems = <p>There are no properties in database</p>
if (properties.length > 0) {
const editorCommandTemplate = text => {
return `MATCH (n) WHERE EXISTS(n.${ecsapeCypherMetaItem(
return `MATCH (n) WHERE EXISTS(n.${escapeCypherIdentifier(
text
)}) RETURN DISTINCT "node" as entity, n.${ecsapeCypherMetaItem(
)}) RETURN DISTINCT "node" as entity, n.${escapeCypherIdentifier(
text
)} AS ${ecsapeCypherMetaItem(
)} AS ${escapeCypherIdentifier(
text
)} LIMIT 25 UNION ALL MATCH ()-[r]-() WHERE EXISTS(r.${ecsapeCypherMetaItem(
)} LIMIT 25 UNION ALL MATCH ()-[r]-() WHERE EXISTS(r.${escapeCypherIdentifier(
text
)}) RETURN DISTINCT "relationship" AS entity, r.${ecsapeCypherMetaItem(
)}) RETURN DISTINCT "relationship" AS entity, r.${escapeCypherIdentifier(
text
)} AS ${ecsapeCypherMetaItem(text)} LIMIT 25`
)} AS ${escapeCypherIdentifier(text)} LIMIT 25`
}
propertyItems = createItems(
properties,
Expand Down
8 changes: 6 additions & 2 deletions src/browser/modules/Stream/Auth/DbsFrame.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
} from './styled'
import { H3 } from 'browser-components/headers'
import Render from 'browser-components/Render/index'
import { toKeyString } from 'services/utils'
import { toKeyString, escapeCypherIdentifier } from 'services/utils'
import { UnstyledList } from '../styled'
import { useDbCommand } from 'shared/modules/commands/commandsDuck'
import TextCommand from 'browser/modules/DecoratedText/TextCommand'
Expand Down Expand Up @@ -59,7 +59,11 @@ export const DbsFrame = props => {
{dbsToShow.map(db => {
return (
<StyledDbsRow key={toKeyString(db.name)}>
<TextCommand command={`${useDbCommand} ${db.name}`} />
<TextCommand
command={`${useDbCommand} ${escapeCypherIdentifier(
db.name
)}`}
/>
</StyledDbsRow>
)
})}
Expand Down
11 changes: 8 additions & 3 deletions src/shared/services/commandInterpreterHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ import {
getCommandAndParam,
tryGetRemoteInitialSlideFromUrl
} from './commandUtils'
import { unescapeCypherIdentifier } from './utils'

const availableCommands = [
{
Expand Down Expand Up @@ -207,20 +208,24 @@ const availableCommands = [
const databaseNames = getDatabases(store.getState()).map(db =>
db.name.toLowerCase()
)

const normalizedName = dbName.toLowerCase()
const cleanDbName = unescapeCypherIdentifier(normalizedName)

// Do we have a db with that name?
if (!databaseNames.includes(dbName.toLowerCase())) {
if (!databaseNames.includes(cleanDbName)) {
const error = new Error(
`A database with the "${dbName}" name could not be found.`
)
error.code = 'NotFound'
throw error
}
put(useDb(dbName))
put(useDb(cleanDbName))
put(
frames.add({
...action,
type: 'use-db',
useDb: dbName
useDb: cleanDbName
})
)
if (action.requestId) {
Expand Down
9 changes: 8 additions & 1 deletion src/shared/services/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import parseUrl from 'url-parse'
import { DESKTOP, CLOUD, WEB } from 'shared/modules/app/appDuck'
import { trimStart, trimEnd } from 'lodash-es'

/**
* The work objects expected shape:
Expand Down Expand Up @@ -296,11 +297,17 @@ export const canUseDOM = () =>
window.document.createElement
)

export const ecsapeCypherMetaItem = str =>
export const escapeCypherIdentifier = str =>
/^[A-Za-z][A-Za-z0-9_]*$/.test(str)
? str
: '`' + str.replace(/`/g, '``') + '`'

export const unescapeCypherIdentifier = str =>
[str]
.map(s => trimStart(s, '`'))
.map(s => trimEnd(s, '`'))
.map(s => s.replace(/``/g, '`'))[0]

export const parseTimeMillis = timeWithOrWithoutUnit => {
timeWithOrWithoutUnit += '' // cast to string
const readUnit = timeWithOrWithoutUnit.match(/\D+/)
Expand Down
18 changes: 16 additions & 2 deletions src/shared/services/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ describe('utils', () => {
expect(utils.parseTimeMillis(time.test)).toEqual(time.expect)
})
})
describe('ecsapeCypherMetaItem', () => {
describe('escapeCypherIdentifier', () => {
// Given
const items = [
{ test: 'Label', expect: 'Label' },
Expand All @@ -574,7 +574,21 @@ describe('utils', () => {

// When && Then
items.forEach(item => {
expect(utils.ecsapeCypherMetaItem(item.test)).toEqual(item.expect)
expect(utils.escapeCypherIdentifier(item.test)).toEqual(item.expect)
})
})
describe('unescapeCypherIdentifier', () => {
// Given
const items = [
{ test: 'Label', expect: 'Label' },
{ test: '`Label Space`', expect: 'Label Space' },
{ test: '`Label-dash`', expect: 'Label-dash' },
{ test: '`Label``Backtick`', expect: 'Label`Backtick' }
]

// When && Then
items.forEach(item => {
expect(utils.unescapeCypherIdentifier(item.test)).toEqual(item.expect)
})
})
})
Expand Down