diff --git a/x-pack/plugins/spaces/mappings.json b/x-pack/plugins/spaces/mappings.json index 77e0c47c00ddd..91c6e324e197e 100644 --- a/x-pack/plugins/spaces/mappings.json +++ b/x-pack/plugins/spaces/mappings.json @@ -13,9 +13,6 @@ } } }, - "urlContext": { - "type": "keyword" - }, "description": { "type": "text" }, diff --git a/x-pack/plugins/spaces/public/views/components/space_card.js b/x-pack/plugins/spaces/public/views/components/space_card.js index 7441366d9206e..f83f009bdb728 100644 --- a/x-pack/plugins/spaces/public/views/components/space_card.js +++ b/x-pack/plugins/spaces/public/views/components/space_card.js @@ -36,11 +36,11 @@ function renderSpaceAvatar(space) { } function renderSpaceDescription(space) { - let description = space.description; - const needsTruncation = space.description.length > 120; + let description = space.description || ''; + const needsTruncation = description.length > 120; if (needsTruncation) { description = ( - {space.description.substr(0, 120) + '…'} + {description.substr(0, 120) + '…'} ); } return {description}; diff --git a/x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/url_context.test.js.snap b/x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/space_identifier.test.js.snap similarity index 63% rename from x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/url_context.test.js.snap rename to x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/space_identifier.test.js.snap index 363f3951202c6..db250113e044d 100644 --- a/x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/url_context.test.js.snap +++ b/x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/space_identifier.test.js.snap @@ -14,7 +14,7 @@ exports[`renders without crashing 1`] = ` isInvalid={false} label={

- URL Context + Space Identifier } > -

- -
+ `; diff --git a/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.js b/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.js index c5e2624466412..8e4da3e399541 100644 --- a/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.js +++ b/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.js @@ -21,14 +21,15 @@ import { EuiButton, EuiPageContentBody, EuiHorizontalRule, + EuiLoadingSpinner, } from '@elastic/eui'; import { DeleteSpacesButton } from '../components'; import { SpaceAvatar } from '../../components'; import { Notifier, toastNotifications } from 'ui/notify'; -import { UrlContext } from './url_context'; -import { toUrlContext, isValidUrlContext } from '../lib/url_context_utils'; +import { SpaceIdentifier } from './space_identifier'; +import { toSpaceIdentifier } from '../lib'; import { CustomizeSpaceAvatar } from './customize_space_avatar'; import { isReservedSpace } from '../../../../common'; import { ReservedSpaceBadge } from './reserved_space_badge'; @@ -37,6 +38,7 @@ import { SpaceValidator } from '../lib/validate_space'; export class ManageSpacePage extends Component { state = { space: {}, + isLoading: true, }; constructor(props) { @@ -57,7 +59,8 @@ export class ManageSpacePage extends Component { .then(result => { if (result.data) { this.setState({ - space: result.data + space: result.data, + isLoading: false }); } }) @@ -69,91 +72,21 @@ export class ManageSpacePage extends Component { toastNotifications.addDanger(`Error loading space: ${message}`); this.backToSpacesList(); }); + } else { + this.setState({ isLoading: false }); } } render() { - const { - name = '', - description = '', - } = this.state.space; + + const content = this.state.isLoading ? this.getLoadingIndicator() : this.getForm(); return ( - - {this.getFormHeading()} - - - - - - - - - - { - name && ( - - - - - - - - - - - ) - } - - - - - {isReservedSpace(this.state.space) - ? null - : ( - - - - ) - } - - - - - - - - - - {this.getFormButtons()} - - + {content} @@ -161,6 +94,90 @@ export class ManageSpacePage extends Component { ); } + getLoadingIndicator = () => { + return

Loading...

; + } + + getForm = () => { + const { + name = '', + description = '', + } = this.state.space; + + return ( + + {this.getFormHeading()} + + + + + + + + + + { + name && ( + + + + + + + + + + + ) + } + + + + + {isReservedSpace(this.state.space) + ? null + : ( + + + + ) + } + + + + + + + + + + {this.getFormButtons()} + + + ); + } + getFormHeading = () => { return (

{this.getTitle()}

@@ -211,21 +228,21 @@ export class ManageSpacePage extends Component { }; onNameChange = (e) => { - const canUpdateContext = !this.editingExistingSpace(); + const canUpdateId = !this.editingExistingSpace(); let { - urlContext + id } = this.state.space; - if (canUpdateContext) { - urlContext = toUrlContext(e.target.value); + if (canUpdateId) { + id = toSpaceIdentifier(e.target.value); } this.setState({ space: { ...this.state.space, name: e.target.value, - urlContext + id } }); }; @@ -239,11 +256,11 @@ export class ManageSpacePage extends Component { }); }; - onUrlContextChange = (e) => { + onSpaceIdentifierChange = (e) => { this.setState({ space: { ...this.state.space, - urlContext: toUrlContext(e.target.value) + id: toSpaceIdentifier(e.target.value) } }); }; @@ -272,11 +289,10 @@ export class ManageSpacePage extends Component { _performSave = () => { const { name = '', - id = toUrlContext(name), + id = toSpaceIdentifier(name), description, initials, color, - urlContext } = this.state.space; const params = { @@ -285,117 +301,34 @@ export class ManageSpacePage extends Component { description, initials, color, - urlContext }; - if (name && description) { - let action; - if (this.editingExistingSpace()) { - action = this.props.spacesManager.updateSpace(params); - } else { - action = this.props.spacesManager.createSpace(params); - } - - action - .then(result => { - this.props.spacesNavState.refreshSpacesList(); - toastNotifications.addSuccess(`Saved '${result.data.name}'`); - window.location.hash = `#/management/spaces/list`; - }) - .catch(error => { - const { - message = '' - } = error.data || {}; - - toastNotifications.addDanger(`Error saving space: ${message}`); - }); + let action; + if (this.editingExistingSpace()) { + action = this.props.spacesManager.updateSpace(params); + } else { + action = this.props.spacesManager.createSpace(params); } + + action + .then(result => { + this.props.spacesNavState.refreshSpacesList(); + toastNotifications.addSuccess(`Saved '${result.data.name}'`); + window.location.hash = `#/management/spaces/list`; + }) + .catch(error => { + const { + message = '' + } = error.data || {}; + + toastNotifications.addDanger(`Error saving space: ${message}`); + }); }; backToSpacesList = () => { window.location.hash = `#/management/spaces/list`; }; - validateName = () => { - if (!this.state.validate) { - return {}; - } - - const { - name - } = this.state.space; - - if (!name) { - return { - isInvalid: true, - error: 'Name is required' - }; - } - - return {}; - }; - - validateDescription = () => { - if (!this.state.validate) { - return {}; - } - - const { - description - } = this.state.space; - - if (!description) { - return { - isInvalid: true, - error: 'Description is required' - }; - } - - return {}; - }; - - validateUrlContext = () => { - if (!this.state.validate) { - return {}; - } - - const { - urlContext - } = this.state.space; - - if (!urlContext) { - return { - isInvalid: true, - error: 'URL Context is required' - }; - } - - if (!isValidUrlContext(urlContext)) { - return { - isInvalid: true, - error: 'URL Context only allows a-z, 0-9, and the "-" character' - }; - } - - return {}; - - }; - - validateForm = () => { - if (!this.state.validate) { - return {}; - } - - const validations = [this.validateName(), this.validateDescription(), this.validateUrlContext()]; - if (validations.some(validation => validation.isInvalid)) { - return { - isInvalid: true - }; - } - - return {}; - }; - editingExistingSpace = () => !!this.props.space; } diff --git a/x-pack/plugins/spaces/public/views/management/edit_space/url_context.js b/x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.js similarity index 54% rename from x-pack/plugins/spaces/public/views/management/edit_space/url_context.js rename to x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.js index 853c0abde745e..9fcd7460114b0 100644 --- a/x-pack/plugins/spaces/public/views/management/edit_space/url_context.js +++ b/x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.js @@ -10,11 +10,9 @@ import { EuiFormRow, EuiLink, EuiFieldText, - EuiCallOut, - EuiSpacer } from '@elastic/eui'; -export class UrlContext extends Component { +export class SpaceIdentifier extends Component { textFieldRef = null; state = { @@ -23,7 +21,7 @@ export class UrlContext extends Component { render() { const { - urlContext = '' + id = '' } = this.props.space; return ( @@ -31,18 +29,15 @@ export class UrlContext extends Component { -
- this.textFieldRef = ref} - /> - {this.getCallOut()} -
+ this.textFieldRef = ref} + /> ); @@ -50,34 +45,17 @@ export class UrlContext extends Component { getLabel = () => { if (!this.props.editable) { - return (

URL Context

); + return (

Space Identifier

); } const editLinkText = this.state.editing ? `[stop editing]` : `[edit]`; - return (

URL Context {editLinkText}

); + return (

Space Identifier {editLinkText}

); }; getHelpText = () => { return (

Links within Kibana will include this space identifier

); }; - getCallOut = () => { - if (this.props.editingExistingSpace && this.state.editing) { - return ( - - - - - ); - } - - return null; - }; - onEditClick = () => { this.setState({ editing: !this.state.editing @@ -94,10 +72,9 @@ export class UrlContext extends Component { }; } -UrlContext.propTypes = { +SpaceIdentifier.propTypes = { space: PropTypes.object.isRequired, editable: PropTypes.bool.isRequired, - editingExistingSpace: PropTypes.bool.isRequired, onChange: PropTypes.func, validator: PropTypes.object.isRequired, }; diff --git a/x-pack/plugins/spaces/public/views/management/edit_space/url_context.test.js b/x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.test.js similarity index 80% rename from x-pack/plugins/spaces/public/views/management/edit_space/url_context.test.js rename to x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.test.js index 13c1caca89987..7970b2633ca85 100644 --- a/x-pack/plugins/spaces/public/views/management/edit_space/url_context.test.js +++ b/x-pack/plugins/spaces/public/views/management/edit_space/space_identifier.test.js @@ -6,17 +6,16 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { UrlContext } from './url_context'; +import { SpaceIdentifier } from './space_identifier'; import { SpaceValidator } from '../lib'; test('renders without crashing', () => { const props = { space: {}, editable: true, - editingExistingSpace: false, onChange: jest.fn(), validator: new SpaceValidator() }; - const wrapper = shallow(); + const wrapper = shallow(); expect(wrapper).toMatchSnapshot(); -}); \ No newline at end of file +}); diff --git a/x-pack/plugins/spaces/public/views/management/lib/index.js b/x-pack/plugins/spaces/public/views/management/lib/index.js index 0d213ec10b93c..ab5331a58a742 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/index.js +++ b/x-pack/plugins/spaces/public/views/management/lib/index.js @@ -5,10 +5,10 @@ */ export { - toUrlContext, - isValidUrlContext -} from './url_context_utils'; + toSpaceIdentifier, + isValidSpaceIdentifier, +} from './space_identifier_utils'; export { SpaceValidator -} from './validate_space'; \ No newline at end of file +} from './validate_space'; diff --git a/x-pack/plugins/spaces/public/views/management/lib/url_context_utils.js b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js similarity index 54% rename from x-pack/plugins/spaces/public/views/management/lib/url_context_utils.js rename to x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js index bf9fe48d4037a..d7defc266b715 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/url_context_utils.js +++ b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js @@ -4,10 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -export function toUrlContext(value = '') { - return value.toLowerCase().replace(/[^a-z0-9]/g, '-'); +export function toSpaceIdentifier(value = '') { + return value.toLowerCase().replace(/[^a-z0-9_]/g, '-'); } -export function isValidUrlContext(value = '') { - return value === toUrlContext(value); +export function isValidSpaceIdentifier(value = '') { + return value === toSpaceIdentifier(value); } diff --git a/x-pack/plugins/spaces/public/views/management/lib/url_context_utils.test.js b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js similarity index 56% rename from x-pack/plugins/spaces/public/views/management/lib/url_context_utils.test.js rename to x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js index 522436163271f..1a3cd091bb861 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/url_context_utils.test.js +++ b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js @@ -3,22 +3,22 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { toUrlContext } from './url_context_utils'; +import { toSpaceIdentifier } from './space_identifier_utils'; test('it converts whitespace to dashes', () => { const input = `this is a test`; - expect(toUrlContext(input)).toEqual('this-is-a-test'); + expect(toSpaceIdentifier(input)).toEqual('this-is-a-test'); }); test('it converts everything to lowercase', () => { const input = `THIS IS A TEST`; - expect(toUrlContext(input)).toEqual('this-is-a-test'); + expect(toSpaceIdentifier(input)).toEqual('this-is-a-test'); }); -test('it converts non-alphanumeric characters to dashes', () => { - const input = `~!@#$%^&*()_+-=[]{}\|';:"/.,<>?` + "`"; +test('it converts non-alphanumeric characters except for "_" to dashes', () => { + const input = `~!@#$%^&*()+-=[]{}\|';:"/.,<>?` + "`"; const expectedResult = new Array(input.length + 1).join('-'); - expect(toUrlContext(input)).toEqual(expectedResult); + expect(toSpaceIdentifier(input)).toEqual(expectedResult); }); diff --git a/x-pack/plugins/spaces/public/views/management/lib/validate_space.js b/x-pack/plugins/spaces/public/views/management/lib/validate_space.js index a59a5ee0ef6f8..56d22ca39aad5 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/validate_space.js +++ b/x-pack/plugins/spaces/public/views/management/lib/validate_space.js @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { isValidUrlContext } from './url_context_utils'; +import { isValidSpaceIdentifier } from './space_identifier_utils'; import { isReservedSpace } from '../../../../common/is_reserved_space'; export class SpaceValidator { @@ -36,24 +36,24 @@ export class SpaceValidator { validateSpaceDescription(space) { if (!this._shouldValidate) return valid(); - if (!space.description) { - return invalid(`Please provide a space description`); + if (space.description && space.description.length > 2000) { + return invalid(`Description must not exceed 2000 characters`); } return valid(); } - validateUrlContext(space) { + validateSpaceIdentifier(space) { if (!this._shouldValidate) return valid(); if (isReservedSpace(space)) return valid(); - if (!space.urlContext) { - return invalid(`URL Context is required`); + if (!space.id) { + return invalid(`Space Identifier is required`); } - if (!isValidUrlContext(space.urlContext)) { - return invalid('URL Context only allows a-z, 0-9, and the "-" character'); + if (!isValidSpaceIdentifier(space.id)) { + return invalid('Space Identifier only allows a-z, 0-9, "_", and the "-" character'); } return valid(); @@ -62,9 +62,9 @@ export class SpaceValidator { validateForSave(space) { const { isInvalid: isNameInvalid } = this.validateSpaceName(space); const { isInvalid: isDescriptionInvalid } = this.validateSpaceDescription(space); - const { isInvalid: isContextInvalid } = this.validateUrlContext(space); + const { isInvalid: isIdentifierInvalid } = this.validateSpaceIdentifier(space); - if (isNameInvalid || isDescriptionInvalid || isContextInvalid) { + if (isNameInvalid || isDescriptionInvalid || isIdentifierInvalid) { return invalid(); } diff --git a/x-pack/plugins/spaces/public/views/management/lib/validate_space.test.js b/x-pack/plugins/spaces/public/views/management/lib/validate_space.test.js index 6c0f2c2bcc10e..5adbb2ec5090e 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/validate_space.test.js +++ b/x-pack/plugins/spaces/public/views/management/lib/validate_space.test.js @@ -39,46 +39,53 @@ describe('validateSpaceName', () => { }); describe('validateSpaceDescription', () => { - test('it requires a non-empty value', () => { + test('is optional', () => { const space = { - description: '' }; - expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: true, error: `Please provide a space description` }); + expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: false }); + }); + + test('it cannot exceed 2000 characters', () => { + const space = { + description: new Array(2002).join('A') + }; + + expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: true, error: `Description must not exceed 2000 characters` }); }); }); -describe('validateUrlContext', () => { +describe('validateSpaceIdentifier', () => { test('it does not validate reserved spaces', () => { const space = { _reserved: true }; - expect(validator.validateUrlContext(space)).toEqual({ isInvalid: false }); + expect(validator.validateSpaceIdentifier(space)).toEqual({ isInvalid: false }); }); test('it requires a non-empty value', () => { const space = { - urlContext: '' + id: '' }; - expect(validator.validateUrlContext(space)).toEqual({ isInvalid: true, error: `URL Context is required` }); + expect(validator.validateSpaceIdentifier(space)).toEqual({ isInvalid: true, error: `Space Identifier is required` }); }); - test('it requires a valid URL Context', () => { + test('it requires a valid Space Identifier', () => { const space = { - urlContext: 'invalid context' + id: 'invalid identifier' }; - expect(validator.validateUrlContext(space)) - .toEqual({ isInvalid: true, error: 'URL Context only allows a-z, 0-9, and the "-" character' }); + expect(validator.validateSpaceIdentifier(space)) + .toEqual({ isInvalid: true, error: 'Space Identifier only allows a-z, 0-9, "_", and the "-" character' }); }); - test('it allows a valid URL Context', () => { + test('it allows a valid Space Identifier', () => { const space = { - urlContext: '01-valid-context-01' + id: '01-valid-context-01' }; - expect(validator.validateUrlContext(space)).toEqual({ isInvalid: false }); + expect(validator.validateSpaceIdentifier(space)).toEqual({ isInvalid: false }); }); -}); \ No newline at end of file +}); diff --git a/x-pack/plugins/spaces/public/views/management/spaces_grid/spaces_grid_page.js b/x-pack/plugins/spaces/public/views/management/spaces_grid/spaces_grid_page.js index f18db68dc5a74..8a5c3ecd9ae81 100644 --- a/x-pack/plugins/spaces/public/views/management/spaces_grid/spaces_grid_page.js +++ b/x-pack/plugins/spaces/public/views/management/spaces_grid/spaces_grid_page.js @@ -123,12 +123,12 @@ export class SpacesGridPage extends Component { ); } }, { - field: 'description', - name: 'Description', + field: 'id', + name: 'Identifier', sortable: true }, { - field: 'urlContext', - name: 'URL Context', + field: 'description', + name: 'Description', sortable: true }]; } diff --git a/x-pack/plugins/spaces/public/views/space_selector/space_selector.test.js b/x-pack/plugins/spaces/public/views/space_selector/space_selector.test.js index 2277ec4757a7e..eccdddb16c1e7 100644 --- a/x-pack/plugins/spaces/public/views/space_selector/space_selector.test.js +++ b/x-pack/plugins/spaces/public/views/space_selector/space_selector.test.js @@ -13,7 +13,7 @@ import { SpacesManager } from '../../lib/spaces_manager'; function getHttpAgent(spaces = []) { - const httpAgent = () => {}; + const httpAgent = () => { }; httpAgent.get = jest.fn(() => Promise.resolve({ data: spaces })); return httpAgent; @@ -32,7 +32,7 @@ function getSpacesManager(spaces = []) { test('it renders without crashing', () => { const spacesManager = getSpacesManager(); const component = renderer.create( - + ); expect(component).toMatchSnapshot(); }); @@ -44,7 +44,6 @@ test('it uses the spaces on props, when provided', () => { id: 'space-1', name: 'Space 1', description: 'This is the first space', - urlContext: 'space-1-context' }]; const component = render( @@ -64,13 +63,12 @@ test('it queries for spaces when not provided on props', () => { id: 'space-1', name: 'Space 1', description: 'This is the first space', - urlContext: 'space-1-context' }]; const spacesManager = getSpacesManager(spaces); shallow( - + ); return Promise diff --git a/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_url_parser.test.js.snap b/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_url_parser.test.js.snap index a42d029097b67..d08be39f9282e 100644 --- a/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_url_parser.test.js.snap +++ b/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_url_parser.test.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`addSpaceUrlContext it throws an error when the requested path does not start with a slash 1`] = `"path must start with a /"`; +exports[`addSpaceIdToPath it throws an error when the requested path does not start with a slash 1`] = `"path must start with a /"`; diff --git a/x-pack/plugins/spaces/server/lib/check_duplicate_context.js b/x-pack/plugins/spaces/server/lib/check_duplicate_context.js deleted file mode 100644 index a3f0125fb777c..0000000000000 --- a/x-pack/plugins/spaces/server/lib/check_duplicate_context.js +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -export function createDuplicateContextQuery(index, { id, urlContext }) { - let mustNotClause = {}; - - if (id) { - mustNotClause = { - must_not: { - term: { - '_id': `space:${id}` - } - } - }; - } - - return { - index, - ignore: [404], - body: { - query: { - bool: { - must: { - term: { - [`space.urlContext`]: urlContext - } - }, - ...mustNotClause - } - } - } - }; -} diff --git a/x-pack/plugins/spaces/server/lib/create_default_space.js b/x-pack/plugins/spaces/server/lib/create_default_space.js index 0408105a6944c..fc19ee325d14f 100644 --- a/x-pack/plugins/spaces/server/lib/create_default_space.js +++ b/x-pack/plugins/spaces/server/lib/create_default_space.js @@ -27,7 +27,6 @@ export async function createDefaultSpace(server) { await savedObjectsRepository.create('space', { name: 'Default Space', description: 'This is your Default Space!', - urlContext: '', _reserved: true }, options); } diff --git a/x-pack/plugins/spaces/server/lib/create_default_space.test.js b/x-pack/plugins/spaces/server/lib/create_default_space.test.js index d2b4df363eb78..1d5a243315e92 100644 --- a/x-pack/plugins/spaces/server/lib/create_default_space.test.js +++ b/x-pack/plugins/spaces/server/lib/create_default_space.test.js @@ -78,7 +78,7 @@ test(`it creates the default space when one does not exist`, async () => { expect(repository.create).toHaveBeenCalledTimes(1); expect(repository.create).toHaveBeenCalledWith( 'space', - { "_reserved": true, "description": "This is your Default Space!", "name": "Default Space", "urlContext": "" }, + { "_reserved": true, "description": "This is your Default Space!", "name": "Default Space" }, { "id": "default" } ); }); diff --git a/x-pack/plugins/spaces/server/lib/create_spaces_service.js b/x-pack/plugins/spaces/server/lib/create_spaces_service.js index d393a6937ef50..3ee55354da8dc 100644 --- a/x-pack/plugins/spaces/server/lib/create_spaces_service.js +++ b/x-pack/plugins/spaces/server/lib/create_spaces_service.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getSpaceUrlContext } from './spaces_url_parser'; +import { getSpaceIdFromPath } from './spaces_url_parser'; export function createSpacesService(server) { @@ -12,24 +12,24 @@ export function createSpacesService(server) { const contextCache = new WeakMap(); - function getUrlContext(request) { + function getSpaceId(request) { if (!contextCache.has(request)) { populateCache(request); } - const { urlContext } = contextCache.get(request); - return urlContext; + const { spaceId } = contextCache.get(request); + return spaceId; } function populateCache(request) { - const urlContext = getSpaceUrlContext(request.getBasePath(), serverBasePath); + const spaceId = getSpaceIdFromPath(request.getBasePath(), serverBasePath); contextCache.set(request, { - urlContext + spaceId }); } return { - getUrlContext, + getSpaceId, }; } diff --git a/x-pack/plugins/spaces/server/lib/create_spaces_service.test.js b/x-pack/plugins/spaces/server/lib/create_spaces_service.test.js index 578a161229c41..c27a505616018 100644 --- a/x-pack/plugins/spaces/server/lib/create_spaces_service.test.js +++ b/x-pack/plugins/spaces/server/lib/create_spaces_service.test.js @@ -5,9 +5,10 @@ */ import { createSpacesService } from "./create_spaces_service"; +import { DEFAULT_SPACE_ID } from '../../common/constants'; -const createRequest = (urlContext, serverBasePath = '') => ({ - getBasePath: () => urlContext ? `${serverBasePath}/s/${urlContext}` : serverBasePath +const createRequest = (spaceId, serverBasePath = '') => ({ + getBasePath: () => spaceId && spaceId !== DEFAULT_SPACE_ID ? `${serverBasePath}/s/${spaceId}` : serverBasePath }); const createMockServer = (config) => { @@ -22,31 +23,31 @@ const createMockServer = (config) => { }; }; -test('returns empty string for the default space', () => { +test('returns the default space ID', () => { const server = createMockServer({ 'server.basePath': '' }); const service = createSpacesService(server); - expect(service.getUrlContext(createRequest())).toEqual(''); + expect(service.getSpaceId(createRequest())).toEqual(DEFAULT_SPACE_ID); }); -test('returns the urlContext for the current space', () => { +test('returns the id for the current space', () => { const request = createRequest('my-space-context'); const server = createMockServer({ 'server.basePath': '' }); const service = createSpacesService(server); - expect(service.getUrlContext(request)).toEqual('my-space-context'); + expect(service.getSpaceId(request)).toEqual('my-space-context'); }); -test(`returns the urlContext for the current space when a server basepath is defined`, () => { +test(`returns the id for the current space when a server basepath is defined`, () => { const request = createRequest('my-space-context', '/foo'); const server = createMockServer({ 'server.basePath': '/foo' }); const service = createSpacesService(server); - expect(service.getUrlContext(request)).toEqual('my-space-context'); + expect(service.getSpaceId(request)).toEqual('my-space-context'); }); diff --git a/x-pack/plugins/spaces/server/lib/get_active_space.js b/x-pack/plugins/spaces/server/lib/get_active_space.js index 834682228eaa4..5ab9bb744b3c6 100644 --- a/x-pack/plugins/spaces/server/lib/get_active_space.js +++ b/x-pack/plugins/spaces/server/lib/get_active_space.js @@ -6,20 +6,15 @@ import Boom from 'boom'; import { wrapError } from './errors'; -import { getSpaceUrlContext } from './spaces_url_parser'; -import { DEFAULT_SPACE_ID } from '../../common/constants'; +import { getSpaceIdFromPath } from './spaces_url_parser'; export async function getActiveSpace(savedObjectsClient, requestBasePath, serverBasePath) { - const spaceContext = getSpaceUrlContext(requestBasePath, serverBasePath); + const spaceId = getSpaceIdFromPath(requestBasePath, serverBasePath); let space; try { - if (spaceContext) { - space = await getSpaceByUrlContext(savedObjectsClient, spaceContext); - } else { - space = await getDefaultSpace(savedObjectsClient); - } + space = await getSpaceById(savedObjectsClient, spaceId); } catch (e) { throw wrapError(e); @@ -37,30 +32,6 @@ export async function getActiveSpace(savedObjectsClient, requestBasePath, server }; } -async function getDefaultSpace(savedObjectsClient) { - return savedObjectsClient.get('space', DEFAULT_SPACE_ID); -} - -async function getSpaceByUrlContext(savedObjectsClient, urlContext) { - const { - saved_objects: savedObjects - } = await savedObjectsClient.find({ - type: 'space', - search: `"${urlContext}"`, - search_fields: ['urlContext'], - }); - - if (savedObjects.length === 0) { - return null; - } - - if (savedObjects.length > 1) { - const spaceNames = savedObjects.map(s => s.attributes.name).join(', '); - - throw Boom.badRequest( - `Multiple Spaces share this URL Context: (${spaceNames}). Please correct this in the Management Section.` - ); - } - - return savedObjects[0]; +async function getSpaceById(savedObjectsClient, spaceId) { + return savedObjectsClient.get('space', spaceId); } diff --git a/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js b/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js index 28471c3d928ba..9fc4064723184 100644 --- a/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js +++ b/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js @@ -23,7 +23,7 @@ export class SpacesSavedObjectsClient { this._client = baseClient; this._types = types; - this._spaceUrlContext = spacesService.getUrlContext(request); + this._spaceId = spacesService.getSpaceId(request); } /** @@ -39,7 +39,7 @@ export class SpacesSavedObjectsClient { */ async create(type, attributes = {}, options = {}) { - const spaceId = await this._getSpaceId(); + const spaceId = this._spaceId; const createOptions = { ...options, @@ -66,7 +66,7 @@ export class SpacesSavedObjectsClient { * @returns {promise} - { saved_objects: [{ id, type, version, attributes, error: { message } }]} */ async bulkCreate(objects, options = {}) { - const spaceId = await this._getSpaceId(); + const spaceId = this._spaceId; const objectsToCreate = objects.map(object => { const objectToCreate = { @@ -127,7 +127,7 @@ export class SpacesSavedObjectsClient { const filters = options.filters || []; - const spaceId = await this._getSpaceId(); + const spaceId = this._spaceId; spaceOptions.filters = [...filters, ...getSpacesQueryFilters(spaceId, types)]; @@ -150,7 +150,7 @@ export class SpacesSavedObjectsClient { */ async bulkGet(objects = [], options = {}) { // ES 'mget' does not support queries, so we have to filter results after the fact. - const thisSpaceId = await this._getSpaceId(); + const thisSpaceId = this._spaceId; const extraDocumentProperties = this._collectExtraDocumentProperties(['spaceId', 'type'], options.extraDocumentProperties); @@ -200,7 +200,7 @@ export class SpacesSavedObjectsClient { const { spaceId: objectSpaceId = DEFAULT_SPACE_ID } = response; if (isTypeSpaceAware(type)) { - const thisSpaceId = await this._getSpaceId(); + const thisSpaceId = this._spaceId; if (objectSpaceId !== thisSpaceId) { throw this._client.errors.createGenericNotFoundError(); } @@ -232,7 +232,7 @@ export class SpacesSavedObjectsClient { if (isTypeSpaceAware(type)) { await this.get(type, id); - const spaceId = await this._getSpaceId(); + const spaceId = this._spaceId; if (this._shouldAssignSpaceId(type, spaceId)) { updateOptions.extraDocumentProperties.spaceId = spaceId; @@ -244,34 +244,6 @@ export class SpacesSavedObjectsClient { return await this._client.update(type, id, attributes, updateOptions); } - async _getSpaceId() { - if (!this._spaceUrlContext) { - return DEFAULT_SPACE_ID; - } - - if (!this._spaceId) { - this._spaceId = await this._findSpace(this._spaceUrlContext); - } - - return this._spaceId; - } - - async _findSpace(urlContext) { - const { - saved_objects: spaces = [] - } = await this._client.find({ - type: 'space', - search: `${urlContext}`, - search_fields: ['urlContext'] - }); - - if (spaces.length > 0) { - return spaces[0].id; - } - - return null; - } - _collectExtraDocumentProperties(thisClientProperties, optionalProperties = []) { return uniq([...thisClientProperties, ...optionalProperties]).value(); } diff --git a/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.test.js b/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.test.js index 4c7454d43f5da..7e9fab3116d70 100644 --- a/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.test.js +++ b/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.test.js @@ -6,6 +6,7 @@ import { SpacesSavedObjectsClient } from './spaces_saved_objects_client'; import { createSpacesService } from '../create_spaces_service'; +import { DEFAULT_SPACE_ID } from '../../../common/constants'; const createObjectEntry = (type, id, spaceId) => ({ [id]: { @@ -35,7 +36,7 @@ const server = { }; const createMockRequest = (space) => ({ - getBasePath: () => space.urlContext ? `/s/${space.urlContext}` : '', + getBasePath: () => space.id !== DEFAULT_SPACE_ID ? `/s/${space.id}` : '', }); const createMockClient = (space) => { @@ -74,7 +75,6 @@ const createMockClient = (space) => { describe('default space', () => { const currentSpace = { id: 'default', - urlContext: '' }; describe('#get', () => { @@ -723,7 +723,6 @@ describe('default space', () => { describe('current space (space_1)', () => { const currentSpace = { id: 'space_1', - urlContext: 'space-1' }; describe('#get', () => { diff --git a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js index a5ec62b2f7e76..5f0612f821b65 100644 --- a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js +++ b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js @@ -5,7 +5,8 @@ */ import { wrapError } from './errors'; -import { addSpaceUrlContext, getSpaceUrlContext } from './spaces_url_parser'; +import { addSpaceIdToPath, getSpaceIdFromPath } from './spaces_url_parser'; +import { DEFAULT_SPACE_ID } from '../../common/constants'; export function initSpacesRequestInterceptors(server) { @@ -16,10 +17,10 @@ export function initSpacesRequestInterceptors(server) { // If navigating within the context of a space, then we store the Space's URL Context on the request, // and rewrite the request to not include the space identifier in the URL. - const spaceUrlContext = getSpaceUrlContext(path, serverBasePath); + const spaceId = getSpaceIdFromPath(path, serverBasePath); - if (spaceUrlContext) { - const reqBasePath = `/s/${spaceUrlContext}`; + if (spaceId !== DEFAULT_SPACE_ID) { + const reqBasePath = `/s/${spaceId}`; request.setBasePath(reqBasePath); const newLocation = path.substr(reqBasePath.length) || '/'; @@ -41,13 +42,11 @@ export function initSpacesRequestInterceptors(server) { const path = request.path; const isRequestingKibanaRoot = path === '/'; - const { spaces } = server; - const urlContext = await spaces.getUrlContext(request); // if requesting the application root, then show the Space Selector UI to allow the user to choose which space // they wish to visit. This is done "onPostAuth" to allow the Saved Objects Client to use the request's auth scope, // which is not available at the time of "onRequest". - if (isRequestingKibanaRoot && !urlContext) { + if (isRequestingKibanaRoot) { try { const client = request.getSavedObjectsClient(); const { total, saved_objects: spaceObjects } = await client.find({ @@ -62,11 +61,8 @@ export function initSpacesRequestInterceptors(server) { // If only one space is available, then send user there directly. // No need for an interstitial screen where there is only one possible outcome. const space = spaceObjects[0]; - const { - urlContext - } = space.attributes; - const destination = addSpaceUrlContext(basePath, urlContext, defaultRoute); + const destination = addSpaceIdToPath(basePath, space.id, defaultRoute); return reply.redirect(destination); } diff --git a/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js b/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js index 05fddbe43fbba..feb6bebeef5f1 100644 --- a/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js +++ b/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js @@ -164,10 +164,9 @@ describe('interceptors', () => { }); const spaces = [{ - id: 'space:a-space', + id: 'a-space', attributes: { name: 'a space', - urlContext: 'a-space', } }]; @@ -197,10 +196,9 @@ describe('interceptors', () => { }); const spaces = [{ - id: 'space:default', + id: 'default', attributes: { name: 'Default Space', - urlContext: '', } }]; @@ -216,16 +214,14 @@ describe('interceptors', () => { test('it redirects to the Space Selector App when navigating to Kibana root', async () => { const spaces = [{ - id: 'space:a-space', + id: 'a-space', attributes: { name: 'a space', - urlContext: 'a-space', } }, { - id: 'space:b-space', + id: 'b-space', attributes: { name: 'b space', - urlContext: 'b-space', } }]; diff --git a/x-pack/plugins/spaces/server/lib/space_schema.js b/x-pack/plugins/spaces/server/lib/space_schema.js index 82dd81ee1705e..dc60c10a36bc2 100644 --- a/x-pack/plugins/spaces/server/lib/space_schema.js +++ b/x-pack/plugins/spaces/server/lib/space_schema.js @@ -8,10 +8,9 @@ import Joi from 'joi'; import { MAX_SPACE_INITIALS } from '../../common/constants'; export const spaceSchema = Joi.object({ - id: Joi.string(), + id: Joi.string().regex(/[a-z0-9_\-]*/, `lower case, a-z, 0-9, "_", and "-" are allowed`), name: Joi.string().required(), - description: Joi.string().required(), - urlContext: Joi.string().regex(/[a-z0-9\-]*/, `lower case, a-z, 0-9, and "-" are allowed`), + description: Joi.string(), initials: Joi.string().max(MAX_SPACE_INITIALS), color: Joi.string().regex(/^#[a-z0-9]{6}$/, `6 digit hex color, starting with a #`), _reserved: Joi.boolean() diff --git a/x-pack/plugins/spaces/server/lib/spaces_url_parser.js b/x-pack/plugins/spaces/server/lib/spaces_url_parser.js index 7c0cf5d32776b..397863785e86c 100644 --- a/x-pack/plugins/spaces/server/lib/spaces_url_parser.js +++ b/x-pack/plugins/spaces/server/lib/spaces_url_parser.js @@ -3,33 +3,38 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +import { DEFAULT_SPACE_ID } from '../../common/constants'; -export function getSpaceUrlContext(requestBasePath = '/', serverBasePath = '/') { +export function getSpaceIdFromPath(requestBasePath = '/', serverBasePath = '/') { let pathToCheck = requestBasePath; if (serverBasePath && serverBasePath !== '/' && requestBasePath.startsWith(serverBasePath)) { pathToCheck = requestBasePath.substr(serverBasePath.length); } // Look for `/s/space-url-context` in the base path - const matchResult = pathToCheck.match(/^\/s\/([a-z0-9\-]+)/); + const matchResult = pathToCheck.match(/^\/s\/([a-z0-9_\-]+)/); if (!matchResult || matchResult.length === 0) { - return ''; + return DEFAULT_SPACE_ID; } // Ignoring first result, we only want the capture group result at index 1 - const [, urlContext = ''] = matchResult; + const [, spaceId] = matchResult; - return urlContext; + if (!spaceId) { + throw new Error(`Unable to determine Space ID from request path: ${requestBasePath}`); + } + + return spaceId; } -export function addSpaceUrlContext(basePath = '/', urlContext = '', requestedPath = '') { +export function addSpaceIdToPath(basePath = '/', spaceId = '', requestedPath = '') { if (requestedPath && !requestedPath.startsWith('/')) { throw new Error(`path must start with a /`); } - if (urlContext) { - return `${basePath}/s/${urlContext}${requestedPath}`; + if (spaceId && spaceId !== DEFAULT_SPACE_ID) { + return `${basePath}/s/${spaceId}${requestedPath}`; } return `${basePath}${requestedPath}`; } diff --git a/x-pack/plugins/spaces/server/lib/spaces_url_parser.test.js b/x-pack/plugins/spaces/server/lib/spaces_url_parser.test.js index 3d2ddaee6137f..68234b3ba48fb 100644 --- a/x-pack/plugins/spaces/server/lib/spaces_url_parser.test.js +++ b/x-pack/plugins/spaces/server/lib/spaces_url_parser.test.js @@ -3,65 +3,66 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { getSpaceUrlContext, addSpaceUrlContext } from './spaces_url_parser'; +import { getSpaceIdFromPath, addSpaceIdToPath } from './spaces_url_parser'; +import { DEFAULT_SPACE_ID } from '../../common/constants'; -describe('getSpaceUrlContext', () => { +describe('getSpaceIdFromPath', () => { describe('without a serverBasePath defined', () => { test('it identifies the space url context', () => { const basePath = `/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath)).toEqual('my-awesome-space-lives-here'); + expect(getSpaceIdFromPath(basePath)).toEqual('my-awesome-space-lives-here'); }); test('ignores space identifiers in the middle of the path', () => { const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath)).toEqual(''); + expect(getSpaceIdFromPath(basePath)).toEqual(DEFAULT_SPACE_ID); }); test('it handles base url without a space url context', () => { const basePath = `/this/is/a/crazy/path/s`; - expect(getSpaceUrlContext(basePath)).toEqual(''); + expect(getSpaceIdFromPath(basePath)).toEqual(DEFAULT_SPACE_ID); }); }); describe('with a serverBasePath defined', () => { test('it identifies the space url context', () => { const basePath = `/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath, '/')).toEqual('my-awesome-space-lives-here'); + expect(getSpaceIdFromPath(basePath, '/')).toEqual('my-awesome-space-lives-here'); }); test('it identifies the space url context following the server base path', () => { const basePath = `/server-base-path-here/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath, '/server-base-path-here')).toEqual('my-awesome-space-lives-here'); + expect(getSpaceIdFromPath(basePath, '/server-base-path-here')).toEqual('my-awesome-space-lives-here'); }); test('ignores space identifiers in the middle of the path', () => { const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath, '/this/is/a')).toEqual(''); + expect(getSpaceIdFromPath(basePath, '/this/is/a')).toEqual(DEFAULT_SPACE_ID); }); test('it handles base url without a space url context', () => { const basePath = `/this/is/a/crazy/path/s`; - expect(getSpaceUrlContext(basePath, basePath)).toEqual(''); + expect(getSpaceIdFromPath(basePath, basePath)).toEqual(DEFAULT_SPACE_ID); }); }); }); -describe('addSpaceUrlContext', () => { +describe('addSpaceIdToPath', () => { test('handles no parameters', () => { - expect(addSpaceUrlContext()).toEqual(`/`); + expect(addSpaceIdToPath()).toEqual(`/`); }); test('it adds to the basePath correctly', () => { - expect(addSpaceUrlContext('/my/base/path', 'url-context')).toEqual('/my/base/path/s/url-context'); + expect(addSpaceIdToPath('/my/base/path', 'url-context')).toEqual('/my/base/path/s/url-context'); }); test('it appends the requested path to the end of the url context', () => { - expect(addSpaceUrlContext('/base', 'context', '/final/destination')).toEqual('/base/s/context/final/destination'); + expect(addSpaceIdToPath('/base', 'context', '/final/destination')).toEqual('/base/s/context/final/destination'); }); test('it throws an error when the requested path does not start with a slash', () => { expect(() => { - addSpaceUrlContext('', '', 'foo'); + addSpaceIdToPath('', '', 'foo'); }).toThrowErrorMatchingSnapshot(); }); }); diff --git a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js index 60a60c01088ee..740b444e65e69 100644 --- a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js +++ b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js @@ -6,13 +6,11 @@ import Boom from 'boom'; import { omit } from 'lodash'; -import { getClient } from '../../../../../../server/lib/get_client_shield'; import { routePreCheckLicense } from '../../../lib/route_pre_check_license'; import { spaceSchema } from '../../../lib/space_schema'; import { wrapError } from '../../../lib/errors'; import { isReservedSpace } from '../../../../common/is_reserved_space'; -import { createDuplicateContextQuery } from '../../../lib/check_duplicate_context'; -import { addSpaceUrlContext } from '../../../lib/spaces_url_parser'; +import { addSpaceIdToPath } from '../../../lib/spaces_url_parser'; export function initSpacesApi(server) { const routePreCheckLicenseFn = routePreCheckLicense(server); @@ -24,32 +22,6 @@ export function initSpacesApi(server) { }; } - const callWithInternalUser = getClient(server).callWithInternalUser; - - const config = server.config(); - - async function checkForDuplicateContext(space) { - const query = createDuplicateContextQuery(config.get('kibana.index'), space); - - // TODO(legrego): Once the SOC is split into a client & repository, this "callWithInternalUser" call should - // be replaced to use the repository instead. - const { hits } = await callWithInternalUser('search', query); - - const { total, hits: conflicts } = hits; - - let error; - - if (total > 0) { - const firstConflictName = conflicts[0]._source.space.name; - - error = Boom.badRequest( - `Another Space (${firstConflictName}) already uses this URL Context. Please choose a different URL Context.` - ); - } - - return { error }; - } - server.route({ method: 'GET', path: '/api/spaces/v1/spaces', @@ -102,36 +74,21 @@ export function initSpacesApi(server) { async handler(request, reply) { const client = request.getSavedObjectsClient(); - const { - overwrite = false - } = request.query; - const space = omit(request.payload, ['id', '_reserved']); - const { error: contextError } = await checkForDuplicateContext(space); + const id = request.payload.id; - if (contextError) { - return reply(wrapError(contextError)); + const existingSpace = await getSpaceById(client, id); + if (existingSpace) { + return reply(Boom.conflict(`A space with the identifier ${id} already exists. Please choose a different identifier`)); } - const id = request.params.id; - - let result; try { - const existingSpace = await getSpaceById(client, id); - - // Reserved Spaces cannot have their _reserved or urlContext properties altered. - if (isReservedSpace(existingSpace)) { - space._reserved = true; - space.urlContext = existingSpace.urlContext; - } - - result = await client.create('space', { ...space }, { id, overwrite }); + return reply(await client.create('space', { ...space }, { id, overwrite: false })); } catch (error) { return reply(wrapError(error)); } - return reply(convertSavedObjectToSpace(result)); }, config: { validate: { @@ -147,22 +104,20 @@ export function initSpacesApi(server) { async handler(request, reply) { const client = request.getSavedObjectsClient(); - const { - overwrite = false - } = request.query; - const space = omit(request.payload, ['id']); const id = request.params.id; - const { error } = await checkForDuplicateContext({ ...space, id }); + const existingSpace = await getSpaceById(client, id); - if (error) { - return reply(wrapError(error)); + if (existingSpace) { + space._reserved = existingSpace._reserved; + } else { + return reply(Boom.notFound(`Unable to find space with ID ${id}`)); } let result; try { - result = await client.create('space', { ...space }, { id, overwrite }); + result = await client.update('space', id, { ...space }); } catch (error) { return reply(wrapError(error)); } @@ -219,7 +174,7 @@ export function initSpacesApi(server) { const config = server.config(); return reply({ - location: addSpaceUrlContext(config.get('server.basePath'), existingSpace.urlContext, config.get('server.defaultRoute')) + location: addSpaceIdToPath(config.get('server.basePath'), existingSpace.id, config.get('server.defaultRoute')) }); } catch (error) { diff --git a/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js b/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js index cd3910e539873..fb957e4586343 100644 --- a/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js +++ b/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js @@ -24,22 +24,19 @@ jest.mock('../../../../../../server/lib/get_client_shield', () => { }); const spaces = [{ - id: 'space:a-space', + id: 'a-space', attributes: { name: 'a space', - urlContext: 'a-space', } }, { - id: 'space:b-space', + id: 'b-space', attributes: { name: 'b space', - urlContext: 'b-space', } }, { - id: 'space:default', + id: 'default', attributes: { name: 'Default Space', - urlContext: '', _reserved: true } }]; @@ -53,7 +50,13 @@ describe('Spaces API', () => { }; beforeEach(() => { - request = async (method, path, setupFn = () => { }, testConfig = {}) => { + request = async (method, path, options = {}) => { + + const { + setupFn = () => { }, + testConfig = {}, + payload, + } = options; const server = new Server(); @@ -78,27 +81,37 @@ describe('Spaces API', () => { server.decorate('request', 'setBasePath', jest.fn()); // Mock server.getSavedObjectsClient() - server.decorate('request', 'getSavedObjectsClient', () => { - return { - get: jest.fn((type, id) => { - return spaces.filter(s => s.id === `${type}:${id}`)[0]; - }), - find: jest.fn(() => { - return { - total: spaces.length, - saved_objects: spaces - }; - }), - delete: jest.fn() - }; - }); + const mockSavedObjectsClient = { + get: jest.fn((type, id) => { + return spaces.filter(s => s.id === id)[0]; + }), + find: jest.fn(() => { + return { + total: spaces.length, + saved_objects: spaces + }; + }), + create: jest.fn(() => ({})), + update: jest.fn(() => ({})), + delete: jest.fn(), + errors: { + isNotFoundError: jest.fn(() => true) + } + }; + + server.decorate('request', 'getSavedObjectsClient', () => mockSavedObjectsClient); teardowns.push(() => server.stop()); - return await server.inject({ - method, - url: path, - }); + return { + server, + mockSavedObjectsClient, + response: await server.inject({ + method, + url: path, + payload, + }) + }; }; }); @@ -107,7 +120,7 @@ describe('Spaces API', () => { }); test(`'GET spaces' returns all available spaces`, async () => { - const response = await request('GET', '/api/spaces/v1/spaces'); + const { response } = await request('GET', '/api/spaces/v1/spaces'); const { statusCode, @@ -120,7 +133,7 @@ describe('Spaces API', () => { }); test(`'GET spaces/{id}' returns the space with that id`, async () => { - const response = await request('GET', '/api/spaces/v1/space/default'); + const { response } = await request('GET', '/api/spaces/v1/space/default'); const { statusCode, @@ -129,11 +142,11 @@ describe('Spaces API', () => { expect(statusCode).toEqual(200); const resultSpace = JSON.parse(payload); - expect(resultSpace.id).toEqual('space:default'); + expect(resultSpace.id).toEqual('default'); }); test(`'DELETE spaces/{id}' deletes the space`, async () => { - const response = await request('DELETE', '/api/spaces/v1/space/a-space'); + const { response } = await request('DELETE', '/api/spaces/v1/space/a-space'); const { statusCode @@ -143,7 +156,7 @@ describe('Spaces API', () => { }); test(`'DELETE spaces/{id}' cannot delete reserved spaces`, async () => { - const response = await request('DELETE', '/api/spaces/v1/space/default'); + const { response } = await request('DELETE', '/api/spaces/v1/space/default'); const { statusCode, @@ -158,8 +171,84 @@ describe('Spaces API', () => { }); }); + test('POST /space should create a new space with the provided ID', async () => { + const payload = { + id: 'my-space-id', + name: 'my new space', + description: 'with a description', + }; + + const { mockSavedObjectsClient, response } = await request('POST', '/api/spaces/v1/space', { payload }); + + const { + statusCode, + } = response; + + expect(statusCode).toEqual(200); + expect(mockSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(mockSavedObjectsClient.create) + .toHaveBeenCalledWith('space', { name: 'my new space', description: 'with a description' }, { id: 'my-space-id', overwrite: false }); + }); + + test('POST /space should not allow a space to be updated', async () => { + const payload = { + id: 'a-space', + name: 'my updated space', + description: 'with a description', + }; + + const { response } = await request('POST', '/api/spaces/v1/space', { payload }); + + const { + statusCode, + payload: responsePayload, + } = response; + + expect(statusCode).toEqual(409); + expect(JSON.parse(responsePayload)).toEqual({ + error: 'Conflict', + message: "A space with the identifier a-space already exists. Please choose a different identifier", + statusCode: 409 + }); + }); + + test('PUT /space should update an existing space with the provided ID', async () => { + const payload = { + id: 'a-space', + name: 'my updated space', + description: 'with a description', + }; + + const { mockSavedObjectsClient, response } = await request('PUT', '/api/spaces/v1/space/a-space', { payload }); + + const { + statusCode, + } = response; + + expect(statusCode).toEqual(200); + expect(mockSavedObjectsClient.update).toHaveBeenCalledTimes(1); + expect(mockSavedObjectsClient.update) + .toHaveBeenCalledWith('space', 'a-space', { name: 'my updated space', description: 'with a description' }); + }); + + test('PUT /space should not allow a new space to be created', async () => { + const payload = { + id: 'a-new-space', + name: 'my new space', + description: 'with a description', + }; + + const { response } = await request('PUT', '/api/spaces/v1/space/a-new-space', { payload }); + + const { + statusCode, + } = response; + + expect(statusCode).toEqual(404); + }); + test('POST space/{id}/select should respond with the new space location', async () => { - const response = await request('POST', '/api/spaces/v1/space/a-space/select'); + const { response } = await request('POST', '/api/spaces/v1/space/a-space/select'); const { statusCode, @@ -172,10 +261,12 @@ describe('Spaces API', () => { expect(result.location).toEqual('/s/a-space'); }); - test('POST space/{id}/select should respond with the new space location when a baseUrl is provided', async () => { - const response = await request('POST', '/api/spaces/v1/space/a-space/select', () => { }, { + test('POST space/{id}/select should respond with the new space location when a server.basePath is in use', async () => { + const testConfig = { 'server.basePath': '/my/base/path' - }); + }; + + const { response } = await request('POST', '/api/spaces/v1/space/a-space/select', { testConfig }); const { statusCode, diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/bulk_get.js b/x-pack/test/spaces_api_integration/apis/saved_objects/bulk_get.js index 3d747051d8c43..37bd80f56a5c8 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/bulk_get.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/bulk_get.js @@ -106,15 +106,15 @@ export default function ({ getService }) { }); }; - const bulkGetTest = (description, { spaceId, urlContext, tests }) => { + const bulkGetTest = (description, { spaceId, tests, otherSpaceId = spaceId }) => { describe(description, () => { before(() => esArchiver.load('saved_objects/spaces')); after(() => esArchiver.unload('saved_objects/spaces')); it(`should return ${tests.default.statusCode}`, async () => { await supertest - .post(`${getUrlPrefix(urlContext)}/api/saved_objects/_bulk_get`) - .send(createBulkRequests(spaceId)) + .post(`${getUrlPrefix(spaceId)}/api/saved_objects/_bulk_get`) + .send(createBulkRequests(otherSpaceId)) .expect(tests.default.statusCode) .then(tests.default.response); }); @@ -133,11 +133,11 @@ export default function ({ getService }) { bulkGetTest(`objects within another space`, { ...SPACES.SPACE_1, - urlContext: SPACES.SPACE_2.urlContext, + otherSpaceId: SPACES.SPACE_2.spaceId, tests: { default: { statusCode: 200, - response: expectNotFoundResults(SPACES.SPACE_1.spaceId) + response: expectNotFoundResults(SPACES.SPACE_2.spaceId) }, } }); diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/create.js b/x-pack/test/spaces_api_integration/apis/saved_objects/create.js index 2eee219f739f6..59f18c131b1ec 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/create.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/create.js @@ -62,7 +62,6 @@ export default function ({ getService }) { version: 1, attributes: { name: 'My favorite space', - urlContext: 'my-favorite-space' } }); @@ -80,13 +79,13 @@ export default function ({ getService }) { expect(actualSpaceId).to.eql('**not defined**'); }; - const createTest = (description, { urlContext, tests }) => { + const createTest = (description, { spaceId, tests }) => { describe(description, () => { before(() => esArchiver.load('saved_objects/spaces')); after(() => esArchiver.unload('saved_objects/spaces')); it(`should return ${tests.spaceAware.statusCode} for a space-aware type`, async () => { await supertest - .post(`${getUrlPrefix(urlContext)}/api/saved_objects/visualization`) + .post(`${getUrlPrefix(spaceId)}/api/saved_objects/visualization`) .send({ attributes: { title: 'My favorite vis' @@ -98,11 +97,10 @@ export default function ({ getService }) { it(`should return ${tests.notSpaceAware.statusCode} for a non space-aware type`, async () => { await supertest - .post(`${getUrlPrefix(urlContext)}/api/saved_objects/space`) + .post(`${getUrlPrefix(spaceId)}/api/saved_objects/space`) .send({ attributes: { name: 'My favorite space', - urlContext: 'my-favorite-space' } }) .expect(tests.notSpaceAware.statusCode) diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/delete.js b/x-pack/test/spaces_api_integration/apis/saved_objects/delete.js index dc1efb057d4e7..0e7b72a5553f2 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/delete.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/delete.js @@ -26,28 +26,28 @@ export default function ({ getService }) { }); }; - const deleteTest = (description, { urlContext, spaceId, tests }) => { + const deleteTest = (description, { spaceId, tests }) => { describe(description, () => { before(() => esArchiver.load('saved_objects/spaces')); after(() => esArchiver.unload('saved_objects/spaces')); it(`should return ${tests.spaceAware.statusCode} when deleting a space-aware doc`, async () => ( await supertest - .delete(`${getUrlPrefix(urlContext)}/api/saved_objects/dashboard/${getIdPrefix(spaceId)}be3733a0-9efe-11e7-acb3-3dab96693fab`) + .delete(`${getUrlPrefix(spaceId)}/api/saved_objects/dashboard/${getIdPrefix(spaceId)}be3733a0-9efe-11e7-acb3-3dab96693fab`) .expect(tests.spaceAware.statusCode) .then(tests.spaceAware.response) )); it(`should return ${tests.notSpaceAware.statusCode} when deleting a non-space-aware doc`, async () => ( await supertest - .delete(`${getUrlPrefix(urlContext)}/api/saved_objects/space/space_2`) + .delete(`${getUrlPrefix(spaceId)}/api/saved_objects/space/space_2`) .expect(tests.notSpaceAware.statusCode) .then(tests.notSpaceAware.response) )); it(`should return ${tests.inOtherSpace.statusCode} when deleting a doc belonging to another space`, async () => { await supertest - .delete(`${getUrlPrefix(urlContext)}/api/saved_objects/dashboard/${getIdPrefix('space_2')}be3733a0-9efe-11e7-acb3-3dab96693fab`) + .delete(`${getUrlPrefix(spaceId)}/api/saved_objects/dashboard/${getIdPrefix('space_2')}be3733a0-9efe-11e7-acb3-3dab96693fab`) .expect(tests.inOtherSpace.statusCode) .then(tests.inOtherSpace.response); }); diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/find.js b/x-pack/test/spaces_api_integration/apis/saved_objects/find.js index 1daa2b2d04f4e..01383433774ef 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/find.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/find.js @@ -106,14 +106,14 @@ export default function ({ getService }) { }); }; - const findTest = (description, { urlContext, tests }) => { + const findTest = (description, { spaceId, tests }) => { describe(description, () => { before(() => esArchiver.load('saved_objects/spaces')); after(() => esArchiver.unload('saved_objects/spaces')); it(`should return ${tests.normal.statusCode} with ${tests.normal.description}`, async () => ( await supertest - .get(`${getUrlPrefix(urlContext)}/api/saved_objects/_find?type=visualization&fields=title`) + .get(`${getUrlPrefix(spaceId)}/api/saved_objects/_find?type=visualization&fields=title`) .expect(tests.normal.statusCode) .then(tests.normal.response) )); @@ -121,7 +121,7 @@ export default function ({ getService }) { describe('page beyond total', () => { it(`should return ${tests.pageBeyondTotal.statusCode} with ${tests.pageBeyondTotal.description}`, async () => ( await supertest - .get(`${getUrlPrefix(urlContext)}/api/saved_objects/_find?type=visualization&page=100&per_page=100`) + .get(`${getUrlPrefix(spaceId)}/api/saved_objects/_find?type=visualization&page=100&per_page=100`) .expect(tests.pageBeyondTotal.statusCode) .then(tests.pageBeyondTotal.response) )); @@ -130,7 +130,7 @@ export default function ({ getService }) { describe('unknown search field', () => { it(`should return ${tests.unknownSearchField.statusCode} with ${tests.unknownSearchField.description}`, async () => ( await supertest - .get(`${getUrlPrefix(urlContext)}/api/saved_objects/_find?type=wigwags&search_fields=a`) + .get(`${getUrlPrefix(spaceId)}/api/saved_objects/_find?type=wigwags&search_fields=a`) .expect(tests.unknownSearchField.statusCode) .then(tests.unknownSearchField.response) )); @@ -139,7 +139,7 @@ export default function ({ getService }) { describe('no type', () => { it(`should return ${tests.noType.statusCode} with ${tests.noType.description}`, async () => ( await supertest - .get(`${getUrlPrefix(urlContext)}/api/saved_objects/_find`) + .get(`${getUrlPrefix(spaceId)}/api/saved_objects/_find`) .expect(tests.noType.statusCode) .then(tests.noType.response) )); diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/get.js b/x-pack/test/spaces_api_integration/apis/saved_objects/get.js index 2516752cbff17..092b9ae499b5c 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/get.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/get.js @@ -51,14 +51,15 @@ export default function ({ getService }) { }); }; - const getTest = (description, { spaceId, urlContext = '', tests }) => { + const getTest = (description, { spaceId, tests, otherSpaceId = spaceId }) => { describe(description, () => { before(async () => esArchiver.load(`saved_objects/spaces`)); after(async () => esArchiver.unload(`saved_objects/spaces`)); it(`should return ${tests.exists.statusCode}`, async () => { + const objectId = `${getIdPrefix(otherSpaceId)}dd7caf20-9efd-11e7-acb3-3dab96693fab`; return supertest - .get(`${getUrlPrefix(urlContext)}/api/saved_objects/visualization/${getIdPrefix(spaceId)}dd7caf20-9efd-11e7-acb3-3dab96693fab`) + .get(`${getUrlPrefix(spaceId)}/api/saved_objects/visualization/${objectId}`) .expect(tests.exists.statusCode) .then(tests.exists.response); }); @@ -77,7 +78,7 @@ export default function ({ getService }) { getTest(`cannot access objects belonging to a different space (space_1)`, { ...SPACES.SPACE_1, - urlContext: 'space-2', + otherSpaceId: SPACES.SPACE_2.spaceId, tests: { exists: { statusCode: 404, @@ -98,7 +99,7 @@ export default function ({ getService }) { getTest(`cannot access objects belonging to a different space (default)`, { ...SPACES.DEFAULT, - urlContext: 'space-1', + otherSpaceId: SPACES.SPACE_1.spaceId, tests: { exists: { statusCode: 404, diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/lib/space_test_utils.js b/x-pack/test/spaces_api_integration/apis/saved_objects/lib/space_test_utils.js index 186b75ccacf88..73df273c6e6e9 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/lib/space_test_utils.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/lib/space_test_utils.js @@ -4,17 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -export function getUrlPrefix(urlContext) { - return urlContext ? `/s/${urlContext}` : ``; +import { DEFAULT_SPACE_ID } from '../../../../../plugins/spaces/common/constants'; + +export function getUrlPrefix(spaceId) { + return spaceId && spaceId !== DEFAULT_SPACE_ID ? `/s/${spaceId}` : ``; } // Spaces do not actually prefix the ID, but this simplifies testing positive and negative flows. export function getIdPrefix(spaceId) { - return spaceId === 'default' ? '' : `${spaceId}-`; + return spaceId === DEFAULT_SPACE_ID ? '' : `${spaceId}-`; } export function getExpectedSpaceIdProperty(spaceId) { - if (spaceId === 'default') { + if (spaceId === DEFAULT_SPACE_ID) { return {}; } return { diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/lib/spaces.js b/x-pack/test/spaces_api_integration/apis/saved_objects/lib/spaces.js index f5cd9109c9a9e..905b360c99949 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/lib/spaces.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/lib/spaces.js @@ -7,14 +7,11 @@ export const SPACES = { SPACE_1: { spaceId: 'space_1', - urlContext: 'space-1' }, SPACE_2: { spaceId: 'space_2', - urlContext: 'space-2' }, DEFAULT: { spaceId: 'default', - urlContext: '' } }; diff --git a/x-pack/test/spaces_api_integration/apis/saved_objects/update.js b/x-pack/test/spaces_api_integration/apis/saved_objects/update.js index d87d03d9b7250..409e4b9db549e 100644 --- a/x-pack/test/spaces_api_integration/apis/saved_objects/update.js +++ b/x-pack/test/spaces_api_integration/apis/saved_objects/update.js @@ -53,13 +53,13 @@ export default function ({ getService }) { }); }; - const updateTest = (description, { urlContext, spaceId, tests }) => { + const updateTest = (description, { spaceId, tests }) => { describe(description, () => { before(() => esArchiver.load('saved_objects/spaces')); after(() => esArchiver.unload('saved_objects/spaces')); it(`should return ${tests.spaceAware.statusCode} for a space-aware doc`, async () => { await supertest - .put(`${getUrlPrefix(urlContext)}/api/saved_objects/visualization/${getIdPrefix(spaceId)}dd7caf20-9efd-11e7-acb3-3dab96693fab`) + .put(`${getUrlPrefix(spaceId)}/api/saved_objects/visualization/${getIdPrefix(spaceId)}dd7caf20-9efd-11e7-acb3-3dab96693fab`) .send({ attributes: { title: 'My second favorite vis' @@ -71,7 +71,7 @@ export default function ({ getService }) { it(`should return ${tests.notSpaceAware.statusCode} for a non space-aware doc`, async () => { await supertest - .put(`${getUrlPrefix(urlContext)}/api/saved_objects/space/space_1`) + .put(`${getUrlPrefix(spaceId)}/api/saved_objects/space/space_1`) .send({ attributes: { name: 'My second favorite space' @@ -84,7 +84,7 @@ export default function ({ getService }) { it(`should return ${tests.inOtherSpace.statusCode} for a doc in another space`, async () => { const id = `${getIdPrefix('space_2')}dd7caf20-9efd-11e7-acb3-3dab96693fab`; await supertest - .put(`${getUrlPrefix(urlContext)}/api/saved_objects/visualization/${id}`) + .put(`${getUrlPrefix(spaceId)}/api/saved_objects/visualization/${id}`) .send({ attributes: { title: 'My second favorite vis' @@ -97,7 +97,7 @@ export default function ({ getService }) { describe('unknown id', () => { it(`should return ${tests.doesntExist.statusCode}`, async () => { await supertest - .put(`${getUrlPrefix(urlContext)}/api/saved_objects/visualization/not an id`) + .put(`${getUrlPrefix(spaceId)}/api/saved_objects/visualization/not an id`) .send({ attributes: { title: 'My second favorite vis' diff --git a/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/data.json.gz b/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/data.json.gz index 33586e3642245..4c2d63913ebe0 100644 Binary files a/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/data.json.gz and b/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/data.json.gz differ diff --git a/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/mappings.json b/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/mappings.json index 2e1a6d1f65fd9..9cca6b52a1caa 100644 --- a/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/mappings.json +++ b/x-pack/test/spaces_api_integration/fixtures/es_archiver/saved_objects/spaces/mappings.json @@ -44,9 +44,6 @@ } } }, - "urlContext": { - "type": "keyword" - }, "description": { "type": "text" },