From 677e798f89ca17f370be5543420945de19e55e07 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 10 Dec 2020 09:36:06 +0100 Subject: [PATCH] [Graph] Fix graph saved object references (#85295) --- .../services/persistence/deserialize.test.ts | 34 ++++++- .../services/persistence/deserialize.ts | 38 ++++++-- .../services/persistence/serialize.test.ts | 2 +- .../public/services/persistence/serialize.ts | 2 +- .../state_management/datasource.sagas.ts | 5 +- .../graph/public/state_management/mocks.ts | 4 +- .../state_management/persistence.test.ts | 7 +- .../public/state_management/persistence.ts | 30 +++--- .../plugins/graph/public/types/persistence.ts | 4 + x-pack/plugins/graph/server/plugin.ts | 4 +- .../server/saved_objects/graph_workspace.ts | 18 ++++ .../server/saved_objects/migrations.test.ts | 92 +++++++++++++++++++ .../graph/server/saved_objects/migrations.ts | 31 +++++++ .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 15 files changed, 238 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/graph/public/services/persistence/deserialize.test.ts b/x-pack/plugins/graph/public/services/persistence/deserialize.test.ts index e9f116b79f9909..815f5ad3ecafe0 100644 --- a/x-pack/plugins/graph/public/services/persistence/deserialize.test.ts +++ b/x-pack/plugins/graph/public/services/persistence/deserialize.test.ts @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { GraphWorkspaceSavedObject, Workspace } from '../../types'; -import { savedWorkspaceToAppState } from './deserialize'; +import { GraphWorkspaceSavedObject, IndexPatternSavedObject, Workspace } from '../../types'; +import { migrateLegacyIndexPatternRef, savedWorkspaceToAppState } from './deserialize'; import { createWorkspace } from '../../angular/graph_client_workspace'; import { outlinkEncoders } from '../../helpers/outlink_encoders'; import { IndexPattern } from '../../../../../../src/plugins/data/public'; @@ -21,7 +21,7 @@ describe('deserialize', () => { numLinks: 2, numVertices: 4, wsState: JSON.stringify({ - indexPattern: 'Testindexpattern', + indexPattern: '123', selectedFields: [ { color: 'black', name: 'field1', selected: true, iconClass: 'a' }, { color: 'black', name: 'field2', selected: true, iconClass: 'b' }, @@ -208,4 +208,32 @@ describe('deserialize', () => { expect(workspace.edges[1].source).toBe(workspace.nodes[2]); expect(workspace.edges[1].target).toBe(workspace.nodes[4]); }); + + describe('migrateLegacyIndexPatternRef', () => { + it('should migrate legacy index pattern ref', () => { + const workspacePayload = { ...savedWorkspace, legacyIndexPatternRef: 'Testpattern' }; + const success = migrateLegacyIndexPatternRef(workspacePayload, [ + { id: '678', attributes: { title: 'Testpattern' } } as IndexPatternSavedObject, + { id: '123', attributes: { title: 'otherpattern' } } as IndexPatternSavedObject, + ]); + expect(success).toEqual({ success: true }); + expect(workspacePayload.legacyIndexPatternRef).toBeUndefined(); + expect(JSON.parse(workspacePayload.wsState).indexPattern).toBe('678'); + }); + + it('should return false if migration fails', () => { + const workspacePayload = { ...savedWorkspace, legacyIndexPatternRef: 'Testpattern' }; + const success = migrateLegacyIndexPatternRef(workspacePayload, [ + { id: '123', attributes: { title: 'otherpattern' } } as IndexPatternSavedObject, + ]); + expect(success).toEqual({ success: false, missingIndexPattern: 'Testpattern' }); + }); + + it('should not modify migrated workspaces', () => { + const workspacePayload = { ...savedWorkspace }; + const success = migrateLegacyIndexPatternRef(workspacePayload, []); + expect(success).toEqual({ success: true }); + expect(workspacePayload).toEqual(savedWorkspace); + }); + }); }); diff --git a/x-pack/plugins/graph/public/services/persistence/deserialize.ts b/x-pack/plugins/graph/public/services/persistence/deserialize.ts index 324bf10cdd99c3..a25e4b7e5e3dc8 100644 --- a/x-pack/plugins/graph/public/services/persistence/deserialize.ts +++ b/x-pack/plugins/graph/public/services/persistence/deserialize.ts @@ -61,19 +61,39 @@ function deserializeUrlTemplate({ return template; } -// returns the id of the index pattern, lookup is done in app.js -export function lookupIndexPattern( +/** + * Migrates `savedWorkspace` to use the id instead of the title of the referenced index pattern. + * Returns a status indicating successful migration or failure to look up the index pattern by title. + * If the workspace is migrated already, a success status is returned as well. + * @param savedWorkspace The workspace saved object to migrate. The migration will happen in-place and mutate the passed in object + * @param indexPatterns All index patterns existing in the current space + */ +export function migrateLegacyIndexPatternRef( savedWorkspace: GraphWorkspaceSavedObject, indexPatterns: IndexPatternSavedObject[] -) { +): { success: true } | { success: false; missingIndexPattern: string } { + const legacyIndexPatternRef = savedWorkspace.legacyIndexPatternRef; + if (!legacyIndexPatternRef) { + return { success: true }; + } + const indexPatternId = indexPatterns.find( + (pattern) => pattern.attributes.title === legacyIndexPatternRef + )?.id; + if (!indexPatternId) { + return { success: false, missingIndexPattern: legacyIndexPatternRef }; + } const serializedWorkspaceState: SerializedWorkspaceState = JSON.parse(savedWorkspace.wsState); - const indexPattern = indexPatterns.find( - (pattern) => pattern.attributes.title === serializedWorkspaceState.indexPattern - ); + serializedWorkspaceState.indexPattern = indexPatternId!; + savedWorkspace.wsState = JSON.stringify(serializedWorkspaceState); + delete savedWorkspace.legacyIndexPatternRef; + return { success: true }; +} - if (indexPattern) { - return indexPattern; - } +// returns the id of the index pattern, lookup is done in app.js +export function lookupIndexPatternId(savedWorkspace: GraphWorkspaceSavedObject) { + const serializedWorkspaceState: SerializedWorkspaceState = JSON.parse(savedWorkspace.wsState); + + return serializedWorkspaceState.indexPattern; } // returns all graph fields mapped out of the index pattern diff --git a/x-pack/plugins/graph/public/services/persistence/serialize.test.ts b/x-pack/plugins/graph/public/services/persistence/serialize.test.ts index 0c9de0418a7381..f142db9db0f7ab 100644 --- a/x-pack/plugins/graph/public/services/persistence/serialize.test.ts +++ b/x-pack/plugins/graph/public/services/persistence/serialize.test.ts @@ -184,7 +184,7 @@ describe('serialize', () => { "timeoutMillis": 5000, "useSignificance": true, }, - "indexPattern": "Testindexpattern", + "indexPattern": "123", "links": Array [ Object { "label": "", diff --git a/x-pack/plugins/graph/public/services/persistence/serialize.ts b/x-pack/plugins/graph/public/services/persistence/serialize.ts index a3a76a8a08eba0..9ed73de3a620d4 100644 --- a/x-pack/plugins/graph/public/services/persistence/serialize.ts +++ b/x-pack/plugins/graph/public/services/persistence/serialize.ts @@ -109,7 +109,7 @@ export function appStateToSavedWorkspace( const mappedUrlTemplates = urlTemplates.map(serializeUrlTemplate); const persistedWorkspaceState: SerializedWorkspaceState = { - indexPattern: selectedIndex.title, + indexPattern: selectedIndex.id, selectedFields: selectedFields.map(serializeField), blocklist, vertices, diff --git a/x-pack/plugins/graph/public/state_management/datasource.sagas.ts b/x-pack/plugins/graph/public/state_management/datasource.sagas.ts index f468ce5beb21c8..4bf897acce757e 100644 --- a/x-pack/plugins/graph/public/state_management/datasource.sagas.ts +++ b/x-pack/plugins/graph/public/state_management/datasource.sagas.ts @@ -44,7 +44,10 @@ export const datasourceSaga = ({ yield put(setDatasource({ type: 'none' })); notifications.toasts.addDanger( i18n.translate('xpack.graph.loadWorkspace.missingIndexPatternErrorMessage', { - defaultMessage: 'Index pattern not found', + defaultMessage: 'Index pattern "{name}" not found', + values: { + name: action.payload.title, + }, }) ); } diff --git a/x-pack/plugins/graph/public/state_management/mocks.ts b/x-pack/plugins/graph/public/state_management/mocks.ts index f28f51544a6ede..f9245881b32e13 100644 --- a/x-pack/plugins/graph/public/state_management/mocks.ts +++ b/x-pack/plugins/graph/public/state_management/mocks.ts @@ -61,7 +61,9 @@ export function createMockGraphStore({ getWorkspace: jest.fn(() => workspaceMock), getSavedWorkspace: jest.fn(() => savedWorkspace), indexPatternProvider: { - get: jest.fn(() => Promise.resolve(({} as unknown) as IndexPattern)), + get: jest.fn(() => + Promise.resolve(({ id: '123', title: 'test-pattern' } as unknown) as IndexPattern) + ), }, indexPatterns: [ ({ id: '123', attributes: { title: 'test-pattern' } } as unknown) as IndexPatternSavedObject, diff --git a/x-pack/plugins/graph/public/state_management/persistence.test.ts b/x-pack/plugins/graph/public/state_management/persistence.test.ts index efad5f95fd839c..687488b0f396f2 100644 --- a/x-pack/plugins/graph/public/state_management/persistence.test.ts +++ b/x-pack/plugins/graph/public/state_management/persistence.test.ts @@ -11,14 +11,15 @@ import { IndexpatternDatasource, datasourceSelector } from './datasource'; import { fieldsSelector } from './fields'; import { metaDataSelector, updateMetaData } from './meta_data'; import { templatesSelector } from './url_templates'; -import { lookupIndexPattern, appStateToSavedWorkspace } from '../services/persistence'; +import { migrateLegacyIndexPatternRef, appStateToSavedWorkspace } from '../services/persistence'; import { settingsSelector } from './advanced_settings'; import { openSaveModal } from '../services/save_modal'; const waitForPromise = () => new Promise((r) => setTimeout(r)); jest.mock('../services/persistence', () => ({ - lookupIndexPattern: jest.fn(() => ({ id: '123', attributes: { title: 'test-pattern' } })), + lookupIndexPatternId: jest.fn(() => ({ id: '123', attributes: { title: 'test-pattern' } })), + migrateLegacyIndexPatternRef: jest.fn(() => ({ success: true })), savedWorkspaceToAppState: jest.fn(() => ({ urlTemplates: [ { @@ -67,7 +68,7 @@ describe('persistence sagas', () => { }); it('should warn with a toast and abort if index pattern is not found', async () => { - (lookupIndexPattern as jest.Mock).mockReturnValueOnce(undefined); + (migrateLegacyIndexPatternRef as jest.Mock).mockReturnValueOnce({ success: false }); env.store.dispatch(loadSavedWorkspace({} as GraphWorkspaceSavedObject)); await waitForPromise(); expect(env.mockedDeps.notifications.toasts.addDanger).toHaveBeenCalled(); diff --git a/x-pack/plugins/graph/public/state_management/persistence.ts b/x-pack/plugins/graph/public/state_management/persistence.ts index cf6566f0c5f86e..1d635a88714ccd 100644 --- a/x-pack/plugins/graph/public/state_management/persistence.ts +++ b/x-pack/plugins/graph/public/state_management/persistence.ts @@ -15,9 +15,10 @@ import { loadFields, selectedFieldsSelector } from './fields'; import { updateSettings, settingsSelector } from './advanced_settings'; import { loadTemplates, templatesSelector } from './url_templates'; import { - lookupIndexPattern, + migrateLegacyIndexPatternRef, savedWorkspaceToAppState, appStateToSavedWorkspace, + lookupIndexPatternId, } from '../services/persistence'; import { updateMetaData, metaDataSelector } from './meta_data'; import { openSaveModal, SaveWorkspaceHandler } from '../services/save_modal'; @@ -43,23 +44,28 @@ export const loadingSaga = ({ indexPatternProvider, }: GraphStoreDependencies) => { function* deserializeWorkspace(action: Action) { - const selectedIndex = lookupIndexPattern(action.payload, indexPatterns); - if (!selectedIndex) { + const workspacePayload = action.payload; + const migrationStatus = migrateLegacyIndexPatternRef(workspacePayload, indexPatterns); + if (!migrationStatus.success) { notifications.toasts.addDanger( i18n.translate('xpack.graph.loadWorkspace.missingIndexPatternErrorMessage', { - defaultMessage: 'Index pattern not found', + defaultMessage: 'Index pattern "{name}" not found', + values: { + name: migrationStatus.missingIndexPattern, + }, }) ); return; } - const indexPattern = yield call(indexPatternProvider.get, selectedIndex.id); + const selectedIndexPatternId = lookupIndexPatternId(workspacePayload); + const indexPattern = yield call(indexPatternProvider.get, selectedIndexPatternId); const initialSettings = settingsSelector(yield select()); - createWorkspace(selectedIndex.attributes.title, initialSettings); + createWorkspace(indexPattern.title, initialSettings); const { urlTemplates, advancedSettings, allFields } = savedWorkspaceToAppState( - action.payload, + workspacePayload, indexPattern, // workspace won't be null because it's created in the same call stack getWorkspace()! @@ -68,16 +74,16 @@ export const loadingSaga = ({ // put everything in the store yield put( updateMetaData({ - title: action.payload.title, - description: action.payload.description, - savedObjectId: action.payload.id, + title: workspacePayload.title, + description: workspacePayload.description, + savedObjectId: workspacePayload.id, }) ); yield put( setDatasource({ type: 'indexpattern', - id: selectedIndex.id, - title: selectedIndex.attributes.title, + id: indexPattern.id, + title: indexPattern.title, }) ); yield put(loadFields(allFields)); diff --git a/x-pack/plugins/graph/public/types/persistence.ts b/x-pack/plugins/graph/public/types/persistence.ts index 8e7e9c7e8878e4..7640ce282a23d6 100644 --- a/x-pack/plugins/graph/public/types/persistence.ts +++ b/x-pack/plugins/graph/public/types/persistence.ts @@ -27,10 +27,14 @@ export interface GraphWorkspaceSavedObject { type: string; version?: number; wsState: string; + // the title of the index pattern used by this workspace. + // Only set for legacy saved objects. + legacyIndexPatternRef?: string; _source: Record; } export interface SerializedWorkspaceState { + // the id of the index pattern saved object indexPattern: string; selectedFields: SerializedField[]; blocklist: SerializedNode[]; diff --git a/x-pack/plugins/graph/server/plugin.ts b/x-pack/plugins/graph/server/plugin.ts index 85f25542b2efbb..7a827db7aba4b5 100644 --- a/x-pack/plugins/graph/server/plugin.ts +++ b/x-pack/plugins/graph/server/plugin.ts @@ -60,7 +60,7 @@ export class GraphPlugin implements Plugin { all: ['graph-workspace'], read: ['index-pattern'], }, - ui: ['save', 'delete'], + ui: ['save', 'delete', 'show'], }, read: { app: ['graph', 'kibana'], @@ -69,7 +69,7 @@ export class GraphPlugin implements Plugin { all: [], read: ['index-pattern', 'graph-workspace'], }, - ui: [], + ui: ['show'], }, }, }); diff --git a/x-pack/plugins/graph/server/saved_objects/graph_workspace.ts b/x-pack/plugins/graph/server/saved_objects/graph_workspace.ts index 8e8cb64aac1b92..173ec8f8f67907 100644 --- a/x-pack/plugins/graph/server/saved_objects/graph_workspace.ts +++ b/x-pack/plugins/graph/server/saved_objects/graph_workspace.ts @@ -10,6 +10,20 @@ export const graphWorkspace: SavedObjectsType = { name: 'graph-workspace', namespaceType: 'single', hidden: false, + management: { + icon: 'graphApp', + defaultSearchField: 'title', + importableAndExportable: true, + getTitle(obj) { + return obj.attributes.title; + }, + getInAppUrl(obj) { + return { + path: `/app/graph#/workspace/${encodeURIComponent(obj.id)}`, + uiCapabilitiesPath: 'graph.show', + }; + }, + }, migrations: graphMigrations, mappings: { properties: { @@ -38,6 +52,10 @@ export const graphWorkspace: SavedObjectsType = { wsState: { type: 'text', }, + legacyIndexPatternRef: { + type: 'text', + index: false, + }, }, }, }; diff --git a/x-pack/plugins/graph/server/saved_objects/migrations.test.ts b/x-pack/plugins/graph/server/saved_objects/migrations.test.ts index ecf1f3ca3b69eb..bd692b5dc00423 100644 --- a/x-pack/plugins/graph/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/graph/server/saved_objects/migrations.test.ts @@ -108,4 +108,96 @@ describe('graph-workspace', () => { `); }); }); + + describe('7.11', () => { + const migration = graphMigrations['7.11.0']; + + test('remove broken reference and set legacy attribute', () => { + const doc = { + id: '1', + type: 'graph-workspace', + attributes: { + wsState: JSON.stringify( + JSON.stringify({ foo: true, indexPatternRefName: 'indexPattern_0' }) + ), + bar: true, + }, + references: [ + { + id: 'pattern*', + name: 'indexPattern_0', + type: 'index-pattern', + }, + ], + }; + const migratedDoc = migration(doc); + expect(migratedDoc).toMatchInlineSnapshot(` + Object { + "attributes": Object { + "bar": true, + "legacyIndexPatternRef": "pattern*", + "wsState": "\\"{\\\\\\"foo\\\\\\":true}\\"", + }, + "id": "1", + "references": Array [], + "type": "graph-workspace", + } + `); + }); + + test('bails out on missing reference', () => { + const doc = { + id: '1', + type: 'graph-workspace', + attributes: { + wsState: JSON.stringify( + JSON.stringify({ foo: true, indexPatternRefName: 'indexPattern_0' }) + ), + bar: true, + }, + }; + const migratedDoc = migration(doc); + expect(migratedDoc).toBe(doc); + }); + + test('bails out on missing index pattern in state', () => { + const doc = { + id: '1', + type: 'graph-workspace', + attributes: { + wsState: JSON.stringify(JSON.stringify({ foo: true })), + bar: true, + }, + references: [ + { + id: 'pattern*', + name: 'indexPattern_0', + type: 'index-pattern', + }, + ], + }; + const migratedDoc = migration(doc); + expect(migratedDoc).toBe(doc); + }); + + test('bails out on broken wsState', () => { + const doc = { + id: '1', + type: 'graph-workspace', + attributes: { + wsState: '{{[[', + bar: true, + }, + references: [ + { + id: 'pattern*', + name: 'indexPattern_0', + type: 'index-pattern', + }, + ], + }; + const migratedDoc = migration(doc); + expect(migratedDoc).toBe(doc); + }); + }); }); diff --git a/x-pack/plugins/graph/server/saved_objects/migrations.ts b/x-pack/plugins/graph/server/saved_objects/migrations.ts index 34cd59e2220e97..e967a9aeaba70e 100644 --- a/x-pack/plugins/graph/server/saved_objects/migrations.ts +++ b/x-pack/plugins/graph/server/saved_objects/migrations.ts @@ -56,4 +56,35 @@ export const graphMigrations = { doc.attributes.wsState = JSON.stringify(JSON.stringify(state)); return doc; }, + '7.11.0': (doc: SavedObjectUnsanitizedDoc) => { + const wsState = get(doc, 'attributes.wsState'); + if (typeof wsState !== 'string') { + return doc; + } + let state; + try { + state = JSON.parse(JSON.parse(wsState)); + } catch (e) { + // Let it go, the data is invalid and we'll leave it as is + return doc; + } + const indexPatternRefName = state.indexPatternRefName; + const indexPatternReference = doc.references?.find( + (reference) => reference.name === indexPatternRefName + ); + if (!indexPatternReference) { + // This saved object doesn't have an reference, there's something corrupted here, + // leave it as is + return doc; + } + const indexPatternTitle = indexPatternReference.id; + // remove index pattern title from workspace state (this should always be the id) + delete state.indexPatternRefName; + // add index pattern title as legacyIndexPatternRef so it can get resolved to the id on next open + doc.attributes.legacyIndexPatternRef = indexPatternTitle; + doc.attributes.wsState = JSON.stringify(JSON.stringify(state)); + // remove references + doc.references = []; + return doc; + }, }; diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 9f101046e051ec..e125467c143061 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -7763,7 +7763,6 @@ "xpack.graph.listing.table.entityName": "グラフ", "xpack.graph.listing.table.entityNamePlural": "グラフ", "xpack.graph.listing.table.titleColumnName": "タイトル", - "xpack.graph.loadWorkspace.missingIndexPatternErrorMessage": "インデックスパターンが見つかりませんでした", "xpack.graph.missingWorkspaceErrorMessage": "ID でグラフを読み込めませんでした", "xpack.graph.newGraphTitle": "保存されていないグラフ", "xpack.graph.noDataSourceNotificationMessageText": "データソースが見つかりませんでした。{managementIndexPatternsLink} に移動して Elasticsearch インデックスのインデックスパターンを作成してください。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 4d0edfea6c2cd5..2b435f5e5ea2b6 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -7771,7 +7771,6 @@ "xpack.graph.listing.table.entityName": "图表", "xpack.graph.listing.table.entityNamePlural": "图表", "xpack.graph.listing.table.titleColumnName": "标题", - "xpack.graph.loadWorkspace.missingIndexPatternErrorMessage": "未找到索引模式", "xpack.graph.missingWorkspaceErrorMessage": "无法使用 ID 加载图表", "xpack.graph.newGraphTitle": "未保存图表", "xpack.graph.noDataSourceNotificationMessageText": "未找到数据源。前往 {managementIndexPatternsLink}，为您的 Elasticsearch 索引创建索引模式。",