Ver Fonte

[frontend] Improve resiliency in the dataCatalog and add tests

Johan Ahlen há 5 anos atrás
pai
commit
e699a62fff

+ 110 - 0
desktop/core/src/desktop/js/catalog/dataCatalog.test.ts

@@ -0,0 +1,110 @@
+// Licensed to Cloudera, Inc. under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  Cloudera, Inc. licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { Compute, Connector, Namespace } from 'types/config';
+import dataCatalog from './dataCatalog';
+
+const connectorOne: Connector = {
+  buttonName: '',
+  displayName: '',
+  id: 'connectorA',
+  page: '',
+  tooltip: '',
+  type: ''
+};
+
+const connectorTwo: Connector = {
+  buttonName: '',
+  displayName: '',
+  id: 'connectorB',
+  page: '',
+  tooltip: '',
+  type: ''
+};
+
+const compute: Compute = { id: 'computeId', name: '', type: '' };
+
+const namespaceOne: Namespace = { computes: [compute], id: 'namespaceOne', name: '', status: '' };
+const namespaceTwo: Namespace = { computes: [compute], id: 'namespaceTwo', name: '', status: '' };
+
+const getEntry = (path: string | string[], connector?: Connector, namespace?: Namespace) =>
+  dataCatalog.getEntry({
+    connector: connector || connectorOne,
+    compute,
+    path: path,
+    namespace: namespace || namespaceOne
+  });
+
+const clearStorage = async (): Promise<void> => {
+  await dataCatalog.getCatalog(connectorOne).clearStorageCascade();
+  await dataCatalog.getCatalog(connectorTwo).clearStorageCascade();
+};
+
+describe('dataCatalog.ts', () => {
+  beforeEach(clearStorage);
+
+  afterAll(clearStorage);
+
+  describe('getEntry', () => {
+    it('Should always return the same entry for the same path, connector and namespace', async () => {
+      const promiseA = getEntry('someDb.someTable');
+      const promiseB = getEntry('someDb.someTable');
+      const [entryA, entryB] = await Promise.all([promiseA, promiseB]);
+      expect(entryA).toBeDefined();
+      expect(entryB).toBeDefined();
+      expect(entryA).toEqual(entryB);
+    });
+
+    it('Should not return the same entry for the same path with different connectors', async () => {
+      const promiseA = getEntry('someDb.someTable', connectorOne);
+      const promiseB = getEntry('someDb.someTable', connectorTwo);
+      const [entryA, entryB] = await Promise.all([promiseA, promiseB]);
+      expect(entryA).toBeDefined();
+      expect(entryB).toBeDefined();
+      expect(entryA).not.toEqual(entryB);
+    });
+
+    it('Should not return the same entry for the same path with different namespaces', async () => {
+      const promiseA = getEntry('someDb.someTable', connectorOne, namespaceOne);
+      const promiseB = getEntry('someDb.someTable', connectorOne, namespaceTwo);
+      const [entryA, entryB] = await Promise.all([promiseA, promiseB]);
+      expect(entryA).toBeDefined();
+      expect(entryB).toBeDefined();
+      expect(entryA).not.toEqual(entryB);
+    });
+  });
+
+  describe('getChildren', () => {
+    it('should handle getChildren of non-existing path with cachedOnly set to true', async () => {
+      let caught = false;
+      try {
+        const childPromise = dataCatalog.getChildren({
+          connector: connectorOne,
+          namespace: namespaceOne,
+          compute: compute,
+          path: 'bad.path',
+          cachedOnly: true
+        });
+
+        await childPromise;
+      } catch (err) {
+        caught = true;
+      }
+
+      expect(caught).toBeTruthy();
+    });
+  });
+});

+ 72 - 70
desktop/core/src/desktop/js/catalog/dataCatalog.ts

@@ -112,7 +112,7 @@ export interface OptimizerResponseValues {
   selectColumns?: OptimizerResponsePopularity[];
 }
 
-export interface OptimizerResponse {
+export interface OptimizerResponse extends TimestampedData {
   top_tables?: OptimizerResponsePopularity[];
   values?: OptimizerResponseValues;
 }
@@ -315,12 +315,12 @@ export class DataCatalog {
    * Clears the data catalog and cache for the given path and any children thereof.
    */
   async clearStorageCascade(
-    namespace: Namespace,
-    compute: Compute,
-    pathToClear: string[]
+    namespace?: Namespace,
+    compute?: Compute,
+    pathToClear?: string[]
   ): Promise<void> {
     if (!namespace || !compute) {
-      if (pathToClear.length === 0) {
+      if (!pathToClear || pathToClear.length === 0) {
         this.entries = {};
         return this.store.clear();
       }
@@ -436,74 +436,73 @@ export class DataCatalog {
           return;
         }
 
-        const fetchPromise = getOptimizer(this.connector).fetchPopularity({
+        const optimizer = getOptimizer(this.connector);
+
+        const fetchPromise = optimizer.fetchPopularity({
           silenceErrors: true,
           paths: pathsToLoad
         });
         cancellablePromises.push(fetchPromise);
 
-        fetchPromise
-          .done((data: OptimizerResponse) => {
-            const perTable: { [path: string]: OptimizerResponse } = {};
-
-            const splitOptimizerValuesPerTable = (
-              listName: keyof OptimizerResponseValues
-            ): void => {
-              const values = data.values && data.values[listName];
-              if (values) {
-                values.forEach(column => {
-                  let tableMeta = perTable[column.dbName + '.' + column.tableName];
-                  if (!tableMeta) {
-                    tableMeta = { values: {} };
-                    perTable[column.dbName + '.' + column.tableName] = tableMeta;
-                  }
-                  if (tableMeta.values) {
-                    let valuesList = tableMeta.values[listName];
-                    if (!valuesList) {
-                      valuesList = [];
-                      tableMeta.values[listName] = valuesList;
-                    }
-                    valuesList.push(column);
+        try {
+          const data = await fetchPromise;
+          const perTable: { [path: string]: OptimizerResponse } = {};
+
+          const splitOptimizerValuesPerTable = (listName: keyof OptimizerResponseValues): void => {
+            const values = data.values && data.values[listName];
+            if (values) {
+              values.forEach(column => {
+                let tableMeta = perTable[column.dbName + '.' + column.tableName];
+                if (!tableMeta) {
+                  tableMeta = { values: {} };
+                  perTable[column.dbName + '.' + column.tableName] = tableMeta;
+                }
+                if (tableMeta.values) {
+                  let valuesList = tableMeta.values[listName];
+                  if (!valuesList) {
+                    valuesList = [];
+                    tableMeta.values[listName] = valuesList;
                   }
-                });
-              }
-            };
-
-            if (data.values) {
-              splitOptimizerValuesPerTable('filterColumns');
-              splitOptimizerValuesPerTable('groupbyColumns');
-              splitOptimizerValuesPerTable('joinColumns');
-              splitOptimizerValuesPerTable('orderbyColumns');
-              splitOptimizerValuesPerTable('selectColumns');
+                  valuesList.push(column);
+                }
+              });
             }
+          };
+
+          if (data.values) {
+            splitOptimizerValuesPerTable('filterColumns');
+            splitOptimizerValuesPerTable('groupbyColumns');
+            splitOptimizerValuesPerTable('joinColumns');
+            splitOptimizerValuesPerTable('orderbyColumns');
+            splitOptimizerValuesPerTable('selectColumns');
+          }
 
-            const tablePromises: Promise<void>[] = Object.keys(perTable).map(
-              path =>
-                new Promise<void>(async resolve => {
-                  try {
-                    const entry = await this.getEntry({
-                      namespace: options.namespace,
-                      compute: options.compute,
-                      path: path
-                    });
-                    const applyPromise = entry.applyOptimizerResponseToChildren(perTable[path], {
-                      ...options,
-                      silenceErrors: true
-                    });
-                    cancellablePromises.push(applyPromise);
-                    popularEntries.push(...(await applyPromise));
-                  } catch (err) {}
-                  resolve();
-                })
-            );
-
-            Promise.all(tablePromises).finally(() => {
-              resolve(popularEntries);
-            });
-          })
-          .fail(() => {
+          const tablePromises: Promise<void>[] = Object.keys(perTable).map(
+            path =>
+              new Promise<void>(async resolve => {
+                try {
+                  const entry = await this.getEntry({
+                    namespace: options.namespace,
+                    compute: options.compute,
+                    path: path
+                  });
+                  const applyPromise = entry.applyOptimizerResponseToChildren(perTable[path], {
+                    ...options,
+                    silenceErrors: true
+                  });
+                  cancellablePromises.push(applyPromise);
+                  popularEntries.push(...(await applyPromise));
+                } catch (err) {}
+                resolve();
+              })
+          );
+
+          Promise.all(tablePromises).finally(() => {
             resolve(popularEntries);
           });
+        } catch (err) {
+          resolve(popularEntries);
+        }
       }
     );
     return applyCancellable(popularityPromise);
@@ -856,14 +855,17 @@ export default {
     } & CatalogGetOptions
   ): CancellablePromise<DataCatalogEntry[]> =>
     new CancellablePromise<DataCatalogEntry[]>(async (resolve, reject, onCancel) => {
-      const entry = await getCatalog(options.connector).getEntry(options);
-
-      const childPromise = entry.getChildren(options);
-      onCancel(() => {
-        childPromise.cancel();
-      });
+      try {
+        const entry = await getCatalog(options.connector).getEntry(options);
+        const childPromise = entry.getChildren(options);
+        onCancel(() => {
+          childPromise.cancel();
+        });
 
-      return applyCancellable(childPromise, options);
+        resolve(applyCancellable(childPromise, options));
+      } catch (err) {
+        reject(err);
+      }
     }),
 
   getCatalog,

+ 180 - 0
desktop/core/src/desktop/js/catalog/dataCatalogEntry.test.ts

@@ -0,0 +1,180 @@
+// Licensed to Cloudera, Inc. under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  Cloudera, Inc. licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { CancellablePromise } from 'api/cancellablePromise';
+import dataCatalog from 'catalog/dataCatalog';
+import DataCatalogEntry, { Sample } from 'catalog/DataCatalogEntry';
+import { Compute, Connector, Namespace } from 'types/config';
+import * as CatalogApi from './api';
+
+const connectorOne: Connector = {
+  buttonName: '',
+  displayName: '',
+  id: 'connectorA',
+  page: '',
+  tooltip: '',
+  type: ''
+};
+
+const connectorTwo: Connector = {
+  buttonName: '',
+  displayName: '',
+  id: 'connectorB',
+  page: '',
+  tooltip: '',
+  type: ''
+};
+
+const compute: Compute = { id: 'computeId', name: '', type: '' };
+
+const namespaceOne: Namespace = { computes: [compute], id: 'namespaceOne', name: '', status: '' };
+
+const getEntry = (path: string | string[], connector?: Connector, namespace?: Namespace) =>
+  dataCatalog.getEntry({
+    connector: connector || connectorOne,
+    compute,
+    path: path,
+    namespace: namespace || namespaceOne
+  });
+
+const clearStorage = async (): Promise<void> => {
+  await dataCatalog.getCatalog(connectorOne).clearStorageCascade();
+  await dataCatalog.getCatalog(connectorTwo).clearStorageCascade();
+};
+
+describe('dataCatalogEntry.ts', () => {
+  beforeEach(clearStorage);
+
+  afterAll(clearStorage);
+
+  describe('getSample', () => {
+    const emptySampleApiSpy = () => {
+      spyOn(CatalogApi, 'fetchSample').and.returnValue(
+        CancellablePromise.resolve<Sample>({
+          data: [],
+          meta: [],
+          type: ''
+        })
+      );
+    };
+
+    it('should return the same sample promise for the same entry', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      const entryB = await getEntry('someDb.someTable');
+      expect(entryA.getSample()).toEqual(entryB.getSample());
+    });
+
+    it('should not return the same sample promise for different entries', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTableOne');
+      const entryB = await getEntry('someDb.someTableTwo');
+      expect(entryA.getSample()).not.toEqual(entryB.getSample());
+    });
+
+    it('should not return the same sample promise if the operation is different', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      const entryB = await getEntry('someDb.someTable');
+      expect(entryA.getSample({ operation: 'distinct' })).not.toEqual(entryB.getSample());
+    });
+
+    it('should keep the sample promise for future session use', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      await entryA.clearCache();
+      const samplePromise = entryA.getSample();
+      expect(entryA.samplePromise).toEqual(samplePromise);
+      const entryB = await getEntry('someDb.someTable');
+      expect(entryB.samplePromise).toEqual(samplePromise);
+    });
+
+    it('should not keep the sample promise if the operation is non-default', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      entryA.getSample({ operation: 'distinct' });
+      expect(entryA.samplePromise).toBeUndefined();
+    });
+
+    it('should bubble up exceptions', async () => {
+      spyOn(CatalogApi, 'fetchSample').and.returnValue(CancellablePromise.reject('failed!'));
+      const entryA = await getEntry('someDb.someTable');
+      let caught = false;
+      try {
+        await entryA.getSample();
+      } catch (err) {
+        expect(err).toEqual('failed!');
+        caught = true;
+      }
+      expect(caught).toBeTruthy();
+    });
+
+    it('should not cancel when cancellable option is not set to true', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      const samplePromise = entryA.getSample({ cancellable: false });
+      await samplePromise.cancel();
+      expect(samplePromise.cancelled).toBeFalsy();
+      expect(entryA.samplePromise).toEqual(samplePromise);
+    });
+
+    it('should not return a cancelled sample promise', async () => {
+      emptySampleApiSpy();
+      const entryA = await getEntry('someDb.someTable');
+      const cancelledPromise = entryA.getSample({ cancellable: true });
+      await cancelledPromise.cancel();
+      const newPromise = entryA.getSample();
+      expect(cancelledPromise.cancelled).toBeTruthy();
+      expect(newPromise).not.toEqual(cancelledPromise);
+    });
+
+    it('should check the parent table samples for a column and re-use if set', async () => {
+      const tableEntry = await getEntry('someDb.someTable');
+      const tableSample: Sample = {
+        data: [
+          [1, 2],
+          [3, 4]
+        ],
+        meta: [
+          { name: 'someOtherCol', type: 'int' },
+          { name: 'someCol', type: 'int' }
+        ],
+        type: 'table'
+      };
+      tableEntry.samplePromise = CancellablePromise.resolve<Sample>(tableSample);
+      const colEntry = await getEntry('someDb.someTable.someCol');
+      const colSample = await colEntry.getSample();
+      expect(colSample.data[0][0]).toEqual(2);
+      expect(colSample.data[1][0]).toEqual(4);
+    });
+
+    it('should check the parent table samples for a column and if not set reload the sample', async () => {
+      let fetchCalledOnEntry: DataCatalogEntry | undefined;
+      jest.spyOn(CatalogApi, 'fetchSample').mockImplementation(options => {
+        fetchCalledOnEntry = options.entry;
+        return CancellablePromise.resolve<Sample>({
+          data: [],
+          meta: [],
+          type: ''
+        });
+      });
+      const entryA = await getEntry('someDb.someTable.someCol');
+      await entryA.getSample();
+      expect(fetchCalledOnEntry).toBeDefined();
+      expect(entryA).toEqual(fetchCalledOnEntry);
+    });
+  });
+});