From 13b213d19e1dc5423e55826c765cf65fc613d744 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 26 Jul 2018 13:56:07 -0400 Subject: [PATCH 1/5] Remove urlContext in favor of a public space identifier --- x-pack/plugins/spaces/mappings.json | 3 - ....js.snap => space_identifier.test.js.snap} | 24 ++- .../edit_space/manage_space_page.js | 109 ++---------- .../{url_context.js => space_identifier.js} | 49 ++---- ...ntext.test.js => space_identifier.test.js} | 7 +- .../public/views/management/lib/index.js | 8 +- ...ext_utils.js => space_identifier_utils.js} | 6 +- ...test.js => space_identifier_utils.test.js} | 8 +- .../views/management/lib/validate_space.js | 14 +- .../management/lib/validate_space.test.js | 24 +-- .../spaces_grid/spaces_grid_page.js | 4 +- .../views/nav_control/nav_control_modal.js | 1 + .../space_selector/space_selector.test.js | 8 +- .../spaces_url_parser.test.js.snap | 2 +- .../server/lib/check_duplicate_context.js | 36 ---- .../spaces/server/lib/create_default_space.js | 1 - .../server/lib/create_default_space.test.js | 2 +- .../server/lib/create_spaces_service.js | 14 +- .../server/lib/create_spaces_service.test.js | 17 +- .../spaces/server/lib/get_active_space.js | 39 +---- .../spaces_saved_objects_client.js | 42 +---- .../spaces_saved_objects_client.test.js | 5 +- .../server/lib/space_request_interceptors.js | 18 +- .../lib/space_request_interceptors.test.js | 12 +- .../plugins/spaces/server/lib/space_schema.js | 3 +- .../spaces/server/lib/spaces_url_parser.js | 21 ++- .../server/lib/spaces_url_parser.test.js | 29 ++-- .../spaces/server/routes/api/v1/spaces.js | 71 ++------ .../server/routes/api/v1/spaces.test.js | 157 ++++++++++++++---- 29 files changed, 285 insertions(+), 449 deletions(-) rename x-pack/plugins/spaces/public/views/management/edit_space/__snapshots__/{url_context.test.js.snap => space_identifier.test.js.snap} (63%) rename x-pack/plugins/spaces/public/views/management/edit_space/{url_context.js => space_identifier.js} (54%) rename x-pack/plugins/spaces/public/views/management/edit_space/{url_context.test.js => space_identifier.test.js} (80%) rename x-pack/plugins/spaces/public/views/management/lib/{url_context_utils.js => space_identifier_utils.js} (67%) rename x-pack/plugins/spaces/public/views/management/lib/{url_context_utils.test.js => space_identifier_utils.test.js} (71%) delete mode 100644 x-pack/plugins/spaces/server/lib/check_duplicate_context.js 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/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..0f7a53e341ad5 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 @@ -27,8 +27,8 @@ 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'; @@ -124,11 +124,10 @@ export class ManageSpacePage extends Component { ? null : ( - @@ -211,21 +210,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 +238,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 +271,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,7 +283,6 @@ export class ManageSpacePage extends Component { description, initials, color, - urlContext }; if (name && description) { @@ -316,86 +313,6 @@ export class ManageSpacePage extends Component { 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 67% 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..513fe9c11c8f4 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 = '') { +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 71% 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..5e28c146f77a7 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,16 +3,16 @@ * 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', () => { @@ -20,5 +20,5 @@ test('it converts non-alphanumeric characters to dashes', () => { 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..40aafdce986e1 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 { @@ -43,17 +43,17 @@ export class SpaceValidator { 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,7 +62,7 @@ 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: isContextInvalid } = this.validateSpaceIdentifier(space); if (isNameInvalid || isDescriptionInvalid || isContextInvalid) { 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..f8f8f547ba1a9 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 @@ -48,37 +48,37 @@ describe('validateSpaceDescription', () => { }); }); -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 context' }; - 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..960402f958699 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 @@ -127,8 +127,8 @@ export class SpacesGridPage extends Component { name: 'Description', sortable: true }, { - field: 'urlContext', - name: 'URL Context', + field: 'id', + name: 'Identifier', sortable: true }]; } diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js index 6ef9b338645ec..bda7869e91cfd 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js @@ -61,6 +61,7 @@ export class NavControlModal extends Component { } = this.props; if (activeSpace && !activeSpace.valid) { + console.error(activeSpace); const { error = {} } = activeSpace; if (error.message) { this.notifier.error(error.message); 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..fc7751db396f2 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`), 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..cdcddc7e49cca 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 exisitng 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, @@ -173,9 +262,11 @@ describe('Spaces API', () => { }); 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', () => { }, { + const testConfig = { 'server.basePath': '/my/base/path' - }); + }; + + const { response } = await request('POST', '/api/spaces/v1/space/a-space/select', { testConfig }); const { statusCode, From 7163f345ba7415511d4386b51e3a8f764e0bf88d Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 26 Jul 2018 14:46:58 -0400 Subject: [PATCH 2/5] space description should be optional --- .../public/views/components/space_card.js | 6 +- .../edit_space/manage_space_page.js | 208 ++++++++++-------- .../views/management/lib/validate_space.js | 8 +- .../management/lib/validate_space.test.js | 13 +- .../spaces_grid/spaces_grid_page.js | 8 +- .../plugins/spaces/server/lib/space_schema.js | 2 +- 6 files changed, 134 insertions(+), 111 deletions(-) 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/manage_space_page.js b/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.js index 0f7a53e341ad5..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,6 +21,7 @@ import { EuiButton, EuiPageContentBody, EuiHorizontalRule, + EuiLoadingSpinner, } from '@elastic/eui'; import { DeleteSpacesButton } from '../components'; @@ -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,90 +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} @@ -160,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()}

@@ -285,28 +303,26 @@ export class ManageSpacePage extends Component { color, }; - 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 = () => { 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 40aafdce986e1..024bd16c9bcab 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 @@ -36,8 +36,8 @@ 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(); @@ -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.validateSpaceIdentifier(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 f8f8f547ba1a9..5587a3f8c3666 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,12 +39,19 @@ describe('validateSpaceName', () => { }); describe('validateSpaceDescription', () => { - test('it requires a non-empty value', () => { + test('is optional', () => { + const space = { + }; + + expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: false }); + }); + + test('it cannot exceed 2000 characters', () => { const space = { - description: '' + description: new Array(2002).join('A') }; - expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: true, error: `Please provide a space description` }); + expect(validator.validateSpaceDescription(space)).toEqual({ isInvalid: true, error: `Description must not exceed 2000 characters` }); }); }); 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 960402f958699..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 @@ -122,14 +122,14 @@ export class SpacesGridPage extends Component { ); } - }, { - field: 'description', - name: 'Description', - sortable: true }, { field: 'id', name: 'Identifier', sortable: true + }, { + field: 'description', + name: 'Description', + sortable: true }]; } diff --git a/x-pack/plugins/spaces/server/lib/space_schema.js b/x-pack/plugins/spaces/server/lib/space_schema.js index fc7751db396f2..c83287508c247 100644 --- a/x-pack/plugins/spaces/server/lib/space_schema.js +++ b/x-pack/plugins/spaces/server/lib/space_schema.js @@ -10,7 +10,7 @@ import { MAX_SPACE_INITIALS } from '../../common/constants'; export const spaceSchema = Joi.object({ id: Joi.string().regex(/[a-z0-9\-]*/, `lower case, a-z, 0-9, and "-" are allowed`), name: Joi.string().required(), - description: Joi.string().required(), + 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() From 90a699ec361bb5d0d63c6dc971eea10a8a4b4c60 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 26 Jul 2018 15:53:16 -0400 Subject: [PATCH 3/5] update functional tests --- .../apis/saved_objects/bulk_get.js | 10 +++++----- .../apis/saved_objects/create.js | 8 +++----- .../apis/saved_objects/delete.js | 8 ++++---- .../apis/saved_objects/find.js | 10 +++++----- .../apis/saved_objects/get.js | 9 +++++---- .../saved_objects/lib/space_test_utils.js | 10 ++++++---- .../apis/saved_objects/lib/spaces.js | 3 --- .../apis/saved_objects/update.js | 10 +++++----- .../saved_objects/spaces/data.json.gz | Bin 2459 -> 2440 bytes .../saved_objects/spaces/mappings.json | 3 --- 10 files changed, 33 insertions(+), 38 deletions(-) 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 33586e3642245682fc862650efa6eabe1ca9bc8e..4c2d63913ebe0d2595f95f252d1ef79f0c448b30 100644 GIT binary patch delta 2397 zcmYjRX*?9{9yRvuTE^JTWG`c>mq84fFwroU$q3o^m_m8ImKZzZE!!Y_*0EoPP)$;y zvQ1$IV@X-EWPP(F$;GeVz4y!cpYwmtbDs0*DG*8)!k_t@le1sK;RX|nr(3X_n%~`k z+e~YYlegzC0wdpF7h|*|ai({4Ew*7T`b)_3KN7EA;90G3`v6o8x&dsJ&+cdawI`$0 zn~7`puv~re+zBBE-Xe7w*$T_mP)Dvd{#6UP8@{qf4b@x@d8fV$t>D>#+4BR-zkTEVGy1(?btL zDZba36#v$!F$E@dJ$8qZ+fkwN!WltF8@Jko&6eR;h@011WexIx(-#IO2T3FXY^T!z zKRrRG=;*WlUdaPKjCfiPqdmNqY3i49PVln1B4OEA zDoIVwTHe>#2J4_LoE5*pYKi~+D!^OD%Sju4Bf(et-66ypTUm$5t7S7!F$M}C+4)qW zR#eB3p14l}p>I_5cr}$p{BdYWJ}JC-AlY@@KEtAVT*AWXd-I^4TBN_IHD!}29VLaZ zH9s)CYjBSG_I1qsJ;k(?z9QTbIGPJcT6W`!YI+vsrLui_$b99RSmB^X66;MCWEMX& z75PrR`Ehpe!o|h<=9eteBUQ_h4o9P*6`awpV!EyBZ!F(n(N!5umbP~s*n5U|pH#0H zLb&^EZH7PbZQCgSn#1GuIod%1cb@#YPp8AU&fk3*>`2a?D_Z|4Q<`vQ|o;-n)&#>W)Q8vbAw5bneQ;S`5gPUjfXzf!*#nj0h(x8oxoLK*$V=9 z=_WJ+xC+b;bl5#1s)$cg>wAYQ{YtbZR}H&lR`nPOi0dz!zQzpRejgwh4au(ugt0n8 zx?bI58iyNDUojSQem~+ibKIwOD5?~V9-x|Pn5y#c)l7Br-+IA zVjip4Lj_q=6~;UZVk6E>FI`M`O1r7w4)N5~^%B_%P||*tADC`^^8$x+O=DyJ3}_SA zCyR##vRgco4>QMWnN}&oZxNruf}h)d!O%O2%`hc9U=$SN6do4!$I5&ngSC5aho*&k z9ggPP$W<|Y{Y&~wqmcox2_P^0{D9!s`%lG30vu-~`L0}FR3%Zm>ZyVe{%yf&mJ#$` zmmx0%LJGs7@i2nIaUidNZK$>$w9n9P|sPN(74t5mVA85dmC@}D7Qtox>dTRjrm; zmj(;ed`_mOV)?D@-(VtL+xivXXXg>vHEFokxy4>@!@k@VlY^zZWL5w;8XNAVMj7Qt?I=$pHLj_zi%wOIV)GWN&-n~M38rqJd$ zrs;FP-ZXPqAafr$;O(DT3SZCVwilr^469o>5#bV1#XldiugGC5=GM^lWD}ylWRL@* zZlhI`qo5nxKRlJVMjHO;Vsp0v-=W{S2FyezL<-#8jTHPapu0ArQ_H~nUs+Cn7y6pO zdB_D$O*C>pi#oa!U!2zN77W>6q<$Kv_#zj*lj#}zW5NI(@4gC8c){AkLeB~1vHn~7 z3I}nigRw4!SW)ku#L=?g$vt{VAiT69GQ}7E22ItalXYs?g-5L^KXNBydsRaj6FH^3 z*4h)BizeS&ceNruUcuk@_>Lv^vNdc&MW6vs#8--3JN)zaruvJWJGJ%4zIEKihcXTp z-xj>kxKmiE7ap0Y4L(A5HP0a#L*IUC7jLB6hJb>h)%X2HAN$tI5W@^1gH4aBMr=(e zWeG5ib8|ivl9oUfxe>gdd<%WA*Q8q)qM)dFh%0hl*r`jX#UE|zq`NFZ^=vX_Bdt*U zBD8}m-5yg*4@$UMYZ181g0+ZKw#pCu%}TiA)1U&?%w%DCn;3*}^26r1Q~59MQ#q}k za3W*=3xFP+HQTK4xUq0DIGu2^2IQbltem;HGH!hL?a=t*on)v0+*QEF8B~i{jX~PP zj7wI&4bn9c=Cp~49z6bA1Zok_3ylJUq_EOfZ0@2er6=u5xVzVfjvsk4m(Prq-bDHFz z_n0abt^8&yL#_OnPnf#_mE&X{3uk_Dc^S5>d_)bQs3{&JbZkE~ETwg$K<6}LFhUeB z?%&4WSD8~7WwdVPCIUtXE-iF=rjmVXy}&T8Axv*_>bC+b6+^;bN>|S-XXTHx{Cym$ zjY^vfPCZEn6qt>h$YkY*>Lh)dy@*1W(7M&^3~RVf8%&p9t|6Rp_AAS#Z?l-9Ltj=Z pI@BGvIL-ebfNQw;n`p+$vE?*lw$l!?_A^^&V{$rEg8G=4{sRqYWwrnS literal 2459 zcmYjLc{~%09~M$1$`~a%nlU*iCFg|9T*F+IbB>wSNUogA`;z-UHb;(;t;mogIdVjf zy=3mDQjQ#vt6X`@ug~ZG{quaE=lML(_xlx%JAPbsT=6Cgo2&CZXL+nI{tnCXEie41 z%fS77M`za-ITzhjWuGFmWmBa%$n4)>4B;|cWF{s?J+&+qb^heudn1cf$?-(M75|VX zUxE&J+9-N%ZK@YO9i+OjnK&G3nzTRV(;fK{e z%xc1wyJ7f=EM|_6pZRqQ^))lezoy*gcxKN_1obNGuPkX!sEB6}oEls-l&x%((lqzY zI%{l9%A^Dhgipv&>I>c15d1Y%qR6nehe365akR+Y+u@1K(7U&7$@h1qhJ#~KD?~El?5gZL!6&u4+ENJflx@L^In@rI9l?8Nx9qbLcR<

OQ}ac7sXl&dExAZdCr~jrhWKomttBrCS+>=qpDLw9_=M) z-ssJnj5r6AEkzeLh^Bu)e}r*qr3LHHSOYhJt?Zmb8jfX-tS zQZcUS#@yDjZYhVJG!Vov3+gV7k3z5W^Sx&4Hb|cB1UW;s*vnEmxERHeo2ue&g9gbm zd;||mGX)aztYv;xotMBq`={)ChIDT>IS-}NNB(^VN;ctFnAM471AmMf^k}s zmiM^51e-ZPIQiu{q2!<%7J_g6Q{&Mm03`vr^D&!po{5uvAu5;;32!YIEhB9f@C-Q(J%{uc@sm>_?9pzYxE& zNYPw8HR5$Qu-#RMZ%a@qTx7>tm8+W({P7Hpp8XGcCt)$T{nh+*P3OIU>|u|NDUu}f z(~!e+<&TODp>Ai#T))%2rnJdP93YeY0)+M*fn)H+#zgeQNUm0& zyW?hweL`>b_H)^MGq>|JtpUmn6@7L3+8!TsPqKPw7{c3I6(5%lGV8bXwxW-A^g)c$ zx7A$lyjKaCcl?rvwa%gaun~4WTD)e|RQVJ06%$>Y^t9tDjsjmT-`O&31VC2Yqz0PD z-{c0@<+_vsBJJ}J3KufpF2!xXcC1xph)qFHgKDhGa*!$OMew}+mL;w^io&G)UG_Ufsz`H9~0TgI1^4WIZu zdrg=<_eQJ~(omt5#wTm*y-PHy9Il)Kw3Fs9ClXN_RBdTof`VuI$g!LWpU(5B=W;+( zfg5XhxTm#+w8QM`wWLSQ_|c#lqxN(+w8&JKUuJ+AbzQ13u>oK&WNk?b&GVyQQ5FV; zH}CCHxYD653N))H72{}E{nYftz~6fAw?+? zc^LfN1SaX$MEnrzjnHT1R|8EQp3C-!j>*f2?CYzH?u;&?C*{TMP-DkZ+?j$$`f&qV z`NgCr_e`Fcc&Xfn1CItiVpKu?MH`|X^V@;{^wBRIFwW!#dz3G|{U0wM}vT@_VncXLTL+YG3jF*^U!ENri>rWlk-c zs_Z*n)MI$`!65oit2wCff>`$d2b2t-AMjW)m?j@t{8sNMAOno_oc=QQE!F~X-kZY7 zTLR8}#9P2%N0_|i=S7$tvL*13!&$)SSQL*0ks~Ys(ke88OeB`I1bq1lKf>gx9^ufl zFFo}BgMxawUz=ESe&vrs&41@xEn Date: Thu, 26 Jul 2018 16:33:08 -0400 Subject: [PATCH 4/5] remove stray console statement --- .../plugins/spaces/public/views/nav_control/nav_control_modal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js index bda7869e91cfd..6ef9b338645ec 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js @@ -61,7 +61,6 @@ export class NavControlModal extends Component { } = this.props; if (activeSpace && !activeSpace.valid) { - console.error(activeSpace); const { error = {} } = activeSpace; if (error.message) { this.notifier.error(error.message); From b5c090cfc947086ab57d7aba658e4bcb589e1393 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Fri, 27 Jul 2018 08:49:50 -0400 Subject: [PATCH 5/5] address PR feedback --- .../public/views/management/lib/space_identifier_utils.js | 2 +- .../views/management/lib/space_identifier_utils.test.js | 4 ++-- .../spaces/public/views/management/lib/validate_space.js | 2 +- .../spaces/public/views/management/lib/validate_space.test.js | 4 ++-- x-pack/plugins/spaces/server/lib/space_schema.js | 2 +- x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js index 513fe9c11c8f4..d7defc266b715 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js +++ b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.js @@ -5,7 +5,7 @@ */ export function toSpaceIdentifier(value = '') { - return value.toLowerCase().replace(/[^a-z0-9]/g, '-'); + return value.toLowerCase().replace(/[^a-z0-9_]/g, '-'); } export function isValidSpaceIdentifier(value = '') { diff --git a/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js index 5e28c146f77a7..1a3cd091bb861 100644 --- a/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js +++ b/x-pack/plugins/spaces/public/views/management/lib/space_identifier_utils.test.js @@ -15,8 +15,8 @@ test('it converts everything to lowercase', () => { 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('-'); 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 024bd16c9bcab..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 @@ -53,7 +53,7 @@ export class SpaceValidator { } if (!isValidSpaceIdentifier(space.id)) { - return invalid('Space Identifier only allows a-z, 0-9, and the "-" character'); + return invalid('Space Identifier only allows a-z, 0-9, "_", and the "-" character'); } return valid(); 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 5587a3f8c3666..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 @@ -74,11 +74,11 @@ describe('validateSpaceIdentifier', () => { test('it requires a valid Space Identifier', () => { const space = { - id: 'invalid context' + id: 'invalid identifier' }; expect(validator.validateSpaceIdentifier(space)) - .toEqual({ isInvalid: true, error: 'Space Identifier only allows a-z, 0-9, and the "-" character' }); + .toEqual({ isInvalid: true, error: 'Space Identifier only allows a-z, 0-9, "_", and the "-" character' }); }); test('it allows a valid Space Identifier', () => { diff --git a/x-pack/plugins/spaces/server/lib/space_schema.js b/x-pack/plugins/spaces/server/lib/space_schema.js index c83287508c247..dc60c10a36bc2 100644 --- a/x-pack/plugins/spaces/server/lib/space_schema.js +++ b/x-pack/plugins/spaces/server/lib/space_schema.js @@ -8,7 +8,7 @@ import Joi from 'joi'; import { MAX_SPACE_INITIALS } from '../../common/constants'; export const spaceSchema = Joi.object({ - id: Joi.string().regex(/[a-z0-9\-]*/, `lower case, a-z, 0-9, and "-" are allowed`), + id: Joi.string().regex(/[a-z0-9_\-]*/, `lower case, a-z, 0-9, "_", and "-" are allowed`), name: Joi.string().required(), description: Joi.string(), initials: Joi.string().max(MAX_SPACE_INITIALS), 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 cdcddc7e49cca..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 @@ -212,7 +212,7 @@ describe('Spaces API', () => { }); }); - test('PUT /space should update an exisitng space with the provided ID', async () => { + test('PUT /space should update an existing space with the provided ID', async () => { const payload = { id: 'a-space', name: 'my updated space', @@ -261,7 +261,7 @@ 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 () => { + 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' };