Merge pull request #1042 from oskarhane/escape-db-names

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

Author: oskarhane

e2e_tests/integration/multi-db.spec.js

@@ -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'))
@@ -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()
       })
     }

src/browser/modules/DBMSInfo/DatabaseSelector.jsx

@@ -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%;
@@ -47,7 +48,7 @@ export const DatabaseSelector = ({
     if (target.value === EMPTY_OPTION) {
       return
     }
-    onChange(target.value)
+    onChange(escapeCypherIdentifier(target.value))
   }
 
   let databasesList = databases

src/browser/modules/DBMSInfo/DatabaseSelector.test.jsx

@@ -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`')
+  })
 })

src/browser/modules/DBMSInfo/MetaItems.jsx

@@ -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 {
@@ -103,7 +103,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,
@@ -143,7 +143,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`
     }
@@ -181,17 +181,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,

src/browser/modules/Stream/Auth/DbsFrame.jsx

@@ -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'
@@ -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>
                 )
               })}

src/shared/services/commandInterpreterHelper.js

@@ -89,6 +89,7 @@ import {
   getCommandAndParam,
   tryGetRemoteInitialSlideFromUrl
 } from './commandUtils'
+import { unescapeCypherIdentifier } from './utils'
 
 const availableCommands = [
   {
@@ -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) {

src/shared/services/utils.js

@@ -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:
@@ -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+/)

src/shared/services/utils.test.js

@@ -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' },
@@ -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)
     })
   })
 })