[data views] Enforce uniqueness by name instead of index pattern (#136071)

* data view uniqueness by name
This commit is contained in:
Matthew Kime 2022-07-14 08:01:33 -05:00 committed by GitHub
parent e3722862ba
commit 9a4eca0a14
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 92 additions and 69 deletions

1
.github/CODEOWNERS vendored
View file

@ -273,6 +273,7 @@
/src/plugins/saved_objects_tagging_oss @elastic/kibana-core
/config/kibana.yml @elastic/kibana-core
/typings/ @elastic/kibana-core
/x-pack/plugins/global_search_providers @elastic/kibana-core
/x-pack/plugins/banners/ @elastic/kibana-core
/x-pack/plugins/features/ @elastic/kibana-core
/x-pack/plugins/licensing/ @elastic/kibana-core

View file

@ -193,10 +193,11 @@ const IndexPatternEditorFlyoutContentComponent = ({
useEffect(() => {
loadSources();
const getTitles = async () => {
const indexPatternTitles = await dataViews.getTitles(editData ? true : false);
const dataViewListItems = await dataViews.getIdsWithTitle(editData ? true : false);
const indexPatternNames = dataViewListItems.map((item) => item.name || item.title);
setExistingIndexPatterns(
editData ? indexPatternTitles.filter((v) => v !== editData.title) : indexPatternTitles
editData ? indexPatternNames.filter((v) => v !== editData.name) : indexPatternNames
);
setIsLoadingIndexPatterns(false);
};
@ -226,9 +227,7 @@ const IndexPatternEditorFlyoutContentComponent = ({
const currentLoadingTimestampFieldsIdx = ++currentLoadingTimestampFieldsRef.current;
let timestampOptions: TimestampOption[] = [];
const isValidResult =
!existingIndexPatterns.includes(query) &&
matchedIndices.exactMatchedIndices.length > 0 &&
!isLoadingMatchedIndices;
matchedIndices.exactMatchedIndices.length > 0 && !isLoadingMatchedIndices;
if (isValidResult) {
setIsLoadingTimestampFields(true);
const getFieldsOptions: GetFieldsOptions = {
@ -249,7 +248,6 @@ const IndexPatternEditorFlyoutContentComponent = ({
return timestampOptions;
},
[
existingIndexPatterns,
dataViews,
requireTimestampField,
rollupIndex,
@ -380,7 +378,7 @@ const IndexPatternEditorFlyoutContentComponent = ({
<EuiSpacer size="l" />
<EuiFlexGroup>
<EuiFlexItem>
<NameField editData={editData} />
<NameField editData={editData} existingDataViewNames={existingIndexPatterns} />
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="l" />
@ -388,7 +386,6 @@ const IndexPatternEditorFlyoutContentComponent = ({
<EuiFlexItem>
<TitleField
isRollup={form.getFields().type?.value === INDEX_PATTERN_TYPE.ROLLUP}
existingIndexPatterns={existingIndexPatterns}
refreshMatchedIndices={reloadMatchedIndices}
matchedIndices={matchedIndices.exactMatchedIndices}
rollupIndicesCapabilities={rollupIndicesCapabilities}
@ -401,7 +398,6 @@ const IndexPatternEditorFlyoutContentComponent = ({
<TimestampField
options={timestampFieldOptions}
isLoadingOptions={isLoadingTimestampFields}
isExistingIndexPattern={existingIndexPatterns.includes(title)}
isLoadingMatchedIndices={isLoadingMatchedIndices}
hasMatchedIndices={!!matchedIndices.exactMatchedIndices.length}
/>

View file

@ -6,20 +6,66 @@
* Side Public License, v 1.
*/
import React, { ChangeEvent } from 'react';
import React, { ChangeEvent, useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiFormRow, EuiFieldText } from '@elastic/eui';
import { DataView, UseField } from '../../shared_imports';
import {
DataView,
UseField,
ValidationConfig,
FieldConfig,
getFieldValidityAndErrorMessage,
} from '../../shared_imports';
import { IndexPatternConfig } from '../../types';
import { schema } from '../form_schema';
interface NameFieldProps {
editData?: DataView;
existingDataViewNames: string[];
}
export const NameField = ({ editData }: NameFieldProps) => {
interface GetNameConfigArgs {
namesNotAllowed: string[];
}
const createNameNoDupesValidator = (
namesNotAllowed: string[]
): ValidationConfig<{}, string, string> => ({
validator: ({ value }) => {
if (namesNotAllowed.includes(value)) {
return {
message: i18n.translate('indexPatternEditor.dataViewExists.ValidationErrorMessage', {
defaultMessage: 'A data view with this name already exists.',
}),
};
}
},
});
const getNameConfig = ({ namesNotAllowed }: GetNameConfigArgs): FieldConfig<string> => {
const nameFieldConfig = schema.name;
const validations = [...nameFieldConfig.validations, createNameNoDupesValidator(namesNotAllowed)];
return {
...nameFieldConfig!,
validations,
};
};
export const NameField = ({ editData, existingDataViewNames }: NameFieldProps) => {
const config = useMemo(
() =>
getNameConfig({
namesNotAllowed: existingDataViewNames,
}),
[existingDataViewNames]
);
return (
<UseField<string, IndexPatternConfig>
path="name"
config={config}
componentProps={{
euiFieldProps: {
'aria-label': i18n.translate('indexPatternEditor.form.nameAriaLabel', {
@ -29,8 +75,9 @@ export const NameField = ({ editData }: NameFieldProps) => {
}}
>
{(field) => {
const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field);
return (
<EuiFormRow label={field.label} fullWidth>
<EuiFormRow label={field.label} fullWidth error={errorMessage} isInvalid={isInvalid}>
<EuiFieldText
value={field.value}
onChange={(e: ChangeEvent<HTMLInputElement>) => {

View file

@ -24,7 +24,6 @@ import { schema } from '../form_schema';
interface Props {
options: TimestampOption[];
isLoadingOptions: boolean;
isExistingIndexPattern: boolean;
isLoadingMatchedIndices: boolean;
hasMatchedIndices: boolean;
}
@ -73,7 +72,6 @@ const timestampFieldHelp = i18n.translate('indexPatternEditor.editor.form.timeFi
export const TimestampField = ({
options = [],
isLoadingOptions = false,
isExistingIndexPattern,
isLoadingMatchedIndices,
hasMatchedIndices,
}: Props) => {
@ -85,11 +83,7 @@ export const TimestampField = ({
const selectTimestampHelp = options.length ? timestampFieldHelp : '';
const timestampNoFieldsHelp =
options.length === 0 &&
!isExistingIndexPattern &&
!isLoadingMatchedIndices &&
!isLoadingOptions &&
hasMatchedIndices
options.length === 0 && !isLoadingMatchedIndices && !isLoadingOptions && hasMatchedIndices
? noTimestampOptionText
: '';

View file

@ -30,7 +30,6 @@ interface RefreshMatchedIndicesResult {
}
interface TitleFieldProps {
existingIndexPatterns: string[];
isRollup: boolean;
matchedIndices: MatchedItem[];
rollupIndicesCapabilities: RollupIndicesCapsResponse;
@ -55,20 +54,6 @@ const mustMatchError = {
}),
};
const createTitlesNoDupesValidator = (
namesNotAllowed: string[]
): ValidationConfig<{}, string, string> => ({
validator: ({ value }) => {
if (namesNotAllowed.includes(value)) {
return {
message: i18n.translate('indexPatternEditor.dataViewExists.ValidationErrorMessage', {
defaultMessage: 'An index pattern with this name already exists.',
}),
};
}
},
});
interface MatchesValidatorArgs {
rollupIndicesCapabilities: Record<string, { error: string }>;
refreshMatchedIndices: (title: string) => Promise<RefreshMatchedIndicesResult>;
@ -122,7 +107,6 @@ const createMatchesIndicesValidator = ({
});
interface GetTitleConfigArgs {
namesNotAllowed: string[];
isRollup: boolean;
matchedIndices: MatchedItem[];
rollupIndicesCapabilities: RollupIndicesCapsResponse;
@ -130,7 +114,6 @@ interface GetTitleConfigArgs {
}
const getTitleConfig = ({
namesNotAllowed,
isRollup,
rollupIndicesCapabilities,
refreshMatchedIndices,
@ -145,7 +128,6 @@ const getTitleConfig = ({
refreshMatchedIndices,
isRollup,
}),
createTitlesNoDupesValidator(namesNotAllowed),
];
return {
@ -155,7 +137,6 @@ const getTitleConfig = ({
};
export const TitleField = ({
existingIndexPatterns,
isRollup,
matchedIndices,
rollupIndicesCapabilities,
@ -166,19 +147,12 @@ export const TitleField = ({
const fieldConfig = useMemo(
() =>
getTitleConfig({
namesNotAllowed: existingIndexPatterns,
isRollup,
matchedIndices,
rollupIndicesCapabilities,
refreshMatchedIndices,
}),
[
existingIndexPatterns,
isRollup,
matchedIndices,
rollupIndicesCapabilities,
refreshMatchedIndices,
]
[isRollup, matchedIndices, rollupIndicesCapabilities, refreshMatchedIndices]
);
return (

View file

@ -50,6 +50,7 @@ export const schema = {
defaultMessage: 'Name',
}),
defaultValue: '',
validations: [],
},
timestampField: {
label: i18n.translate('indexPatternEditor.editor.form.timeFieldLabel', {

View file

@ -33,7 +33,7 @@ import {
} from '../types';
import { META_FIELDS, SavedObject } from '..';
import { DataViewMissingIndices } from '../lib';
import { findByTitle } from '../utils';
import { findByName } from '../utils';
import { DuplicateDataViewError, DataViewInsufficientAccessError } from '../errors';
const MAX_ATTEMPTS_TO_RESOLVE_CONFLICTS = 3;
@ -772,12 +772,17 @@ export class DataViewsService {
* @param skipFetchFields if true, will not fetch fields
* @returns DataView
*/
async create({ id, ...restOfSpec }: DataViewSpec, skipFetchFields = false): Promise<DataView> {
async create(
{ id, name, title, ...restOfSpec }: DataViewSpec,
skipFetchFields = false
): Promise<DataView> {
const shortDotsEnable = await this.config.get(FORMATS_UI_SETTINGS.SHORT_DOTS_ENABLE);
const metaFields = await this.config.get(META_FIELDS);
const spec = {
id: id ?? uuid.v4(),
title,
name: name || title,
...restOfSpec,
};
@ -821,12 +826,13 @@ export class DataViewsService {
if (!(await this.getCanSave())) {
throw new DataViewInsufficientAccessError();
}
const dupe = await findByTitle(this.savedObjectsClient, dataView.title);
const dupe = await findByName(this.savedObjectsClient, dataView.getName());
if (dupe) {
if (override) {
await this.delete(dupe.id);
} else {
throw new DuplicateDataViewError(`Duplicate data view: ${dataView.title}`);
throw new DuplicateDataViewError(`Duplicate data view: ${dataView.getName()}`);
}
}

View file

@ -12,22 +12,22 @@ import type { SavedObjectsClientCommon } from './types';
import { DATA_VIEW_SAVED_OBJECT_TYPE } from './constants';
/**
* Returns an object matching a given title
* Returns an object matching a given name
*
* @param client {SavedObjectsClientCommon}
* @param title {string}
* @returns {Promise<SavedObject|undefined>}
* @param name {string}
* @returns {SavedObject|undefined}
*/
export async function findByTitle(client: SavedObjectsClientCommon, title: string) {
if (title) {
export async function findByName(client: SavedObjectsClientCommon, name: string) {
if (name) {
const savedObjects = await client.find<DataViewSavedObjectAttrs>({
type: DATA_VIEW_SAVED_OBJECT_TYPE,
perPage: 10,
search: `"${title}"`,
searchFields: ['title'],
fields: ['title'],
search: `"${name}"`,
searchFields: ['name'],
fields: ['name'],
});
return savedObjects.find((obj) => obj.attributes.title.toLowerCase() === title.toLowerCase());
return savedObjects ? savedObjects[0] : undefined;
}
}

View file

@ -113,7 +113,7 @@ const registerCreateDataViewRouteFactory =
const dataView = await createDataView({
dataViewsService,
usageCollection,
spec: spec as DataViewSpec,
spec: { ...spec, name: spec.name || spec.title } as DataViewSpec,
override: body.override,
refreshFields: body.refresh_fields,
counterName: `${req.route.method} ${path}`,

View file

@ -18,10 +18,10 @@ export const dataViewSavedObjectType: SavedObjectsType = {
management: {
displayName: 'data view',
icon: 'indexPatternApp',
defaultSearchField: 'title',
defaultSearchField: 'name',
importableAndExportable: true,
getTitle(obj) {
return obj.attributes.title;
return obj.attributes.name || obj.attributes.title;
},
getEditUrl(obj) {
return `/management/kibana/dataViews/dataView/${encodeURIComponent(obj.id)}`;
@ -38,6 +38,7 @@ export const dataViewSavedObjectType: SavedObjectsType = {
properties: {
title: { type: 'text' },
type: { type: 'keyword' },
name: { type: 'text' },
},
},
migrations: indexPatternSavedObjectTypeMigrations as any,

View file

@ -243,7 +243,7 @@ export default function ({ getService }: FtrProviderContext) {
expect(response.body[config.serviceKey].fieldAttrs.foo.customLabel).to.be('test');
});
describe('when creating index pattern with existing title', () => {
describe('when creating index pattern with existing name', () => {
it('returns error, by default', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post(config.path).send({

View file

@ -153,6 +153,7 @@ describe('mapToResults', () => {
management: {
defaultSearchField: 'excerpt',
getInAppUrl: (obj) => ({ path: `/type-c/${obj.id}`, uiCapabilitiesPath: 'test.typeC' }),
getTitle: (obj) => `${obj.attributes.title} ${obj.attributes.name}`,
},
})
);
@ -204,6 +205,7 @@ describe('mapToResults', () => {
{
excerpt: 'titleC',
title: 'foo',
name: 'name',
},
[
{ name: 'tag A', type: 'tag', id: '1' },
@ -235,7 +237,7 @@ describe('mapToResults', () => {
},
{
id: 'resultC',
title: 'titleC',
title: 'foo name',
type: 'typeC',
url: '/type-c/resultC',
score: 42,

View file

@ -41,7 +41,7 @@ export const mapToResult = (
object: SavedObjectsFindResult<unknown>,
type: SavedObjectsType
): GlobalSearchProviderResult => {
const { defaultSearchField, getInAppUrl } = type.management ?? {};
const { defaultSearchField, getInAppUrl, getTitle } = type.management ?? {};
if (defaultSearchField === undefined || getInAppUrl === undefined) {
throw new Error('Trying to map an object from a type without management metadata');
}
@ -49,7 +49,7 @@ export const mapToResult = (
id: object.id,
// defaultSearchField is dynamic and not 'directly' bound to the generic type of the SavedObject
// so we are forced to cast the attributes to any to access the properties associated with it.
title: (object.attributes as any)[defaultSearchField],
title: getTitle ? getTitle(object) : (object.attributes as any)[defaultSearchField],
type: object.type,
icon: type.management?.icon ?? undefined,
url: getInAppUrl(object).path,

File diff suppressed because one or more lines are too long

View file

@ -40,7 +40,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
await kibanaServer.savedObjects.cleanStandardList();
});
it('can search for index patterns', async () => {
it('can search for data views', async () => {
const results = await findResultsWithApi('type:index-pattern logstash');
expect(results.length).to.be(1);
expect(results[0].type).to.be('index-pattern');