[Lens] Clear out all attribute properties before updating (#74483)

elastic · Aug 10, 2020 · c1b55d5 · c1b55d5
1 parent 244ed04
commit c1b55d5
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 33 deletions.
diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts
@@ -4,19 +4,22 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { SavedObjectsClientContract, SavedObjectsBulkUpdateObject } from 'kibana/public';
 import { SavedObjectIndexStore } from './saved_object_store';
 
 describe('LensStore', () => {
   function testStore(testId?: string) {
     const client = {
       create: jest.fn(() => Promise.resolve({ id: testId || 'testid' })),
-      update: jest.fn((_type: string, id: string) => Promise.resolve({ id })),
+      bulkUpdate: jest.fn(([{ id }]: SavedObjectsBulkUpdateObject[]) =>
+        Promise.resolve({ savedObjects: [{ id }, { id }] })
+      ),
       get: jest.fn(),
     };
 
     return {
       client,
-      store: new SavedObjectIndexStore(client),
+      store: new SavedObjectIndexStore((client as unknown) as SavedObjectsClientContract),
     };
   }
 
@@ -108,19 +111,35 @@ describe('LensStore', () => {
         },
       });
 
-      expect(client.update).toHaveBeenCalledTimes(1);
-      expect(client.update).toHaveBeenCalledWith('lens', 'Gandalf', {
-        title: 'Even the very wise cannot see all ends.',
-        visualizationType: 'line',
-        expression: '',
-        state: {
-          datasourceMetaData: { filterableIndexPatterns: [] },
-          datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } },
-          visualization: { gear: ['staff', 'pointy hat'] },
-          query: { query: '', language: 'lucene' },
-          filters: [],
+      expect(client.bulkUpdate).toHaveBeenCalledTimes(1);
+      expect(client.bulkUpdate).toHaveBeenCalledWith([
+        {
+          type: 'lens',
+          id: 'Gandalf',
+          attributes: {
+            title: null,
+            visualizationType: null,
+            expression: null,
+            state: null,
+          },
         },
-      });
+        {
+          type: 'lens',
+          id: 'Gandalf',
+          attributes: {
+            title: 'Even the very wise cannot see all ends.',
+            visualizationType: 'line',
+            expression: '',
+            state: {
+              datasourceMetaData: { filterableIndexPatterns: [] },
+              datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } },
+              visualization: { gear: ['staff', 'pointy hat'] },
+              query: { query: '', language: 'lucene' },
+              filters: [],
+            },
+          },
+        },
+      ]);
     });
   });
 

diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { SavedObjectAttributes } from 'kibana/server';
+import { SavedObjectAttributes, SavedObjectsClientContract } from 'kibana/public';
 import { Query, Filter } from '../../../../../src/plugins/data/public';
 
 export interface Document {
@@ -27,20 +27,6 @@ export interface Document {
 
 export const DOC_TYPE = 'lens';
 
-interface SavedObjectClient {
-  create: (type: string, object: SavedObjectAttributes) => Promise<{ id: string }>;
-  update: (type: string, id: string, object: SavedObjectAttributes) => Promise<{ id: string }>;
-  get: (
-    type: string,
-    id: string
-  ) => Promise<{
-    id: string;
-    type: string;
-    attributes: SavedObjectAttributes;
-    error?: { statusCode: number; message: string };
-  }>;
-}
-
 export interface DocumentSaver {
   save: (vis: Document) => Promise<{ id: string }>;
 }
@@ -52,9 +38,9 @@ export interface DocumentLoader {
 export type SavedObjectStore = DocumentLoader & DocumentSaver;
 
 export class SavedObjectIndexStore implements SavedObjectStore {
-  private client: SavedObjectClient;
+  private client: SavedObjectsClientContract;
 
-  constructor(client: SavedObjectClient) {
+  constructor(client: SavedObjectsClientContract) {
     this.client = client;
   }
 
@@ -63,13 +49,33 @@ export class SavedObjectIndexStore implements SavedObjectStore {
     // TODO: SavedObjectAttributes should support this kind of object,
     // remove this workaround when SavedObjectAttributes is updated.
     const attributes = (rest as unknown) as SavedObjectAttributes;
+
     const result = await (id
-      ? this.client.update(DOC_TYPE, id, attributes)
+      ? this.safeUpdate(id, attributes)
       : this.client.create(DOC_TYPE, attributes));
 
     return { ...vis, id: result.id };
   }
 
+  // As Lens is using an object to store its attributes, using the update API
+  // will merge the new attribute object with the old one, not overwriting deleted
+  // keys. As Lens is using objects as maps in various places, this is a problem because
+  // deleted subtrees make it back into the object after a load.
+  // This function fixes this by doing two updates - one to empty out the document setting
+  // every key to null, and a second one to load the new content.
+  private async safeUpdate(id: string, attributes: SavedObjectAttributes) {
+    const resetAttributes: SavedObjectAttributes = {};
+    Object.keys(attributes).forEach((key) => {
+      resetAttributes[key] = null;
+    });
+    return (
+      await this.client.bulkUpdate([
+        { type: DOC_TYPE, id, attributes: resetAttributes },
+        { type: DOC_TYPE, id, attributes },
+      ])
+    ).savedObjects[1];
+  }
+
   async load(id: string): Promise<Document> {
     const { type, attributes, error } = await this.client.get(DOC_TYPE, id);
 
@@ -78,7 +84,7 @@ export class SavedObjectIndexStore implements SavedObjectStore {
     }
 
     return {
-      ...attributes,
+      ...(attributes as SavedObjectAttributes),
       id,
       type,
     } as Document;