more cleanup, reformat and tests

Author: paula-stacho

packages/compass-schema/src/components/compass-schema.tsx

@@ -35,7 +35,7 @@ import { useConnectionInfo } from '@mongodb-js/compass-connections/provider';
 import { getAtlasPerformanceAdvisorLink } from '../utils';
 import { useIsLastAppliedQueryOutdated } from '@mongodb-js/compass-query-bar';
 import { useTelemetry } from '@mongodb-js/compass-telemetry/provider';
-import type { RootState, SchemaThunkDispatch } from '../stores/store';
+import type { RootState } from '../stores/store';
 import { startAnalysis, stopAnalysis } from '../stores/reducer';
 
 const rootStyles = css({
@@ -372,7 +372,7 @@ const Schema: React.FunctionComponent<{
   maxTimeMS?: number;
   schema: MongodbSchema | null;
   count?: number;
-  resultId?: number;
+  resultId?: string;
   onStartAnalysis: () => Promise<void>;
   onStopAnalysis: () => void;
 }> = ({
@@ -387,14 +387,6 @@ const Schema: React.FunctionComponent<{
     void onStartAnalysis();
   }, [onStartAnalysis]);
 
-  const onCancelClicked = useCallback(() => {
-    onStopAnalysis();
-  }, [onStopAnalysis]);
-
-  const onResetClicked = useCallback(() => {
-    onStopAnalysis();
-  }, [onStopAnalysis]);
-
   const outdated = useIsLastAppliedQueryOutdated('schema');
 
   const enablePerformanceAdvisorBanner = usePreference(
@@ -407,12 +399,12 @@ const Schema: React.FunctionComponent<{
         toolbar={
           <SchemaToolbar
             onAnalyzeSchemaClicked={onApplyClicked}
-            onResetClicked={onResetClicked}
+            onResetClicked={onStopAnalysis}
             analysisState={analysisState}
             errorMessage={errorMessage || ''}
             isOutdated={!!outdated}
             sampleSize={schema ? schema.count : 0}
-            schemaResultId={String(resultId) || ''}
+            schemaResultId={resultId || ''}
           />
         }
       >
@@ -422,7 +414,7 @@ const Schema: React.FunctionComponent<{
             <InitialScreen onApplyClicked={onApplyClicked} />
           )}
           {analysisState === ANALYSIS_STATE_ANALYZING && (
-            <AnalyzingScreen onCancelClicked={onCancelClicked} />
+            <AnalyzingScreen onCancelClicked={onStopAnalysis} />
           )}
           {analysisState === ANALYSIS_STATE_COMPLETE && (
             <FieldList schema={schema} analysisState={analysisState} />

packages/compass-schema/src/components/field.spec.tsx

@@ -13,7 +13,6 @@ import {
   type SchemaType,
 } from 'mongodb-schema';
 import { BSON, Decimal128 } from 'bson';
-import { configureActions } from '../actions';
 import Field, { shouldShowUnboundArrayInsight } from './field';
 import QueryBarPlugin from '@mongodb-js/compass-query-bar';
 import {
@@ -44,7 +43,6 @@ function renderField(
     <MockQueryBarPlugin {...(queryBarProps as any)}>
       <Field
         enableMaps={false}
-        actions={configureActions()}
         name="testFieldName"
         path={['test']}
         {...props}

packages/compass-schema/src/components/minichart/minichart.jsx

@@ -34,13 +34,10 @@ class MiniChart extends PureComponent {
     // but it is not noticable to the user.
     this.resizeListener();
     window.addEventListener('resize', this.resizeListener);
-    // this.unsubscribeMiniChartResize =
-    //   this.props.actions.resizeMiniCharts.listen(this.resizeListener); // TODO: what was this doing?
   }
 
   componentWillUnmount() {
     window.removeEventListener('resize', this.resizeListener);
-    // this.unsubscribeMiniChartResize();
   }
 
   /**

packages/compass-schema/src/stores/reducer.ts

@@ -8,8 +8,6 @@ import {
   ANALYSIS_STATE_INITIAL,
   ANALYSIS_STATE_TIMEOUT,
 } from '../constants/analysis-states';
-
-import type { Query } from '@mongodb-js/compass-query-bar';
 import { addLayer, generateGeoQuery } from '../modules/geo';
 import {
   analyzeSchema,
@@ -21,6 +19,7 @@ import { openToast } from '@mongodb-js/compass-components';
 import type { Circle, Layer, LayerGroup, Polygon } from 'leaflet';
 import { mongoLogId } from '@mongodb-js/compass-logging/provider';
 import type { SchemaThunkAction } from './store';
+import { UUID } from 'bson';
 
 const DEFAULT_SAMPLE_SIZE = 1000;
 
@@ -37,7 +36,7 @@ export type SchemaState = {
   analysisState: AnalysisState;
   errorMessage: string;
   schema: Schema | null;
-  resultId: number;
+  resultId: string;
 };
 
 export const enum SchemaActions {
@@ -61,10 +60,6 @@ export type AnalysisFailedAction = {
   error: Error;
 };
 
-export type analysisCancelled = {
-  type: SchemaActions.analysisCancelled;
-};
-
 const reducer: Reducer<SchemaState, Action> = (
   state = getInitialState(),
   action
@@ -99,13 +94,6 @@ const reducer: Reducer<SchemaState, Action> = (
     };
   }
 
-  if (isAction<analysisCancelled>(action, SchemaActions.analysisCancelled)) {
-    return {
-      ...state,
-      analysisState: ANALYSIS_STATE_INITIAL,
-    };
-  }
-
   return state;
 };
 
@@ -124,8 +112,8 @@ function getErrorState(err: Error & { code?: number }) {
   return { analysisState, errorMessage };
 }
 
-function resultId(): number {
-  return Math.floor(Math.random() * 2 ** 53);
+function resultId(): string {
+  return new UUID().toString();
 }
 
 export const handleSchemaShare = (): SchemaThunkAction<void> => {
@@ -182,12 +170,6 @@ const getInitialState = (): SchemaState => ({
   resultId: resultId(),
 });
 
-export const onSchemaSampled = (): SchemaThunkAction<void> => {
-  return (dispatch, getState, { geoLayersRef }) => {
-    geoLayersRef.current = {};
-  };
-};
-
 export const geoLayerAdded = (
   field: string,
   layer: Layer
@@ -228,28 +210,6 @@ export const geoLayersDeleted = (
 export const stopAnalysis = (): SchemaThunkAction<void> => {
   return (dispatch, getState, { abortControllerRef }) => {
     abortControllerRef.current?.abort();
-    dispatch({ type: SchemaActions.analysisCancelled });
-  };
-};
-
-export const _trackSchemaAnalyzed = (
-  analysisTimeMS: number,
-  query: Query
-): SchemaThunkAction<void> => {
-  return (dispatch, getState, { track, connectionInfoRef }) => {
-    const { schema } = getState();
-    // Use a function here to a) ensure that the calculations here
-    // are only made when telemetry is enabled and b) that errors from
-    // those calculations are caught and logged rather than displayed to
-    // users as errors from the core schema analysis logic.
-    const trackEvent = () => ({
-      with_filter: Object.entries(query.filter ?? {}).length > 0,
-      schema_width: schema?.fields?.length ?? 0,
-      schema_depth: schema ? calculateSchemaDepth(schema) : 0,
-      geo_data: schema ? schemaContainsGeoData(schema) : false,
-      analysis_time_ms: analysisTimeMS,
-    });
-    track('Schema Analyzed', trackEvent, connectionInfoRef.current);
   };
 };
 
@@ -269,6 +229,9 @@ export const startAnalysis = (): SchemaThunkAction<
       fieldStoreService,
       abortControllerRef,
       namespace,
+      geoLayersRef,
+      connectionInfoRef,
+      track,
     }
   ) => {
     const query = queryBar.getLastAppliedQuery('schema');
@@ -295,10 +258,6 @@ export const startAnalysis = (): SchemaThunkAction<
 
       dispatch({ type: SchemaActions.analysisStarted });
 
-      abortSignal?.addEventListener('abort', () => {
-        dispatch(stopAnalysis());
-      });
-
       const analysisStartTime = Date.now();
       const schema = await analyzeSchema(
         dataService,
@@ -316,9 +275,17 @@ export const startAnalysis = (): SchemaThunkAction<
 
       dispatch({ type: SchemaActions.analysisFinished, schema });
 
-      _trackSchemaAnalyzed(analysisTime, query);
+      // track schema analyzed
+      const trackEvent = () => ({
+        with_filter: Object.entries(query.filter ?? {}).length > 0,
+        schema_width: schema?.fields?.length ?? 0,
+        schema_depth: schema ? calculateSchemaDepth(schema) : 0,
+        geo_data: schema ? schemaContainsGeoData(schema) : false,
+        analysis_time_ms: analysisTime,
+      });
+      track('Schema Analyzed', trackEvent, connectionInfoRef.current);
 
-      dispatch(onSchemaSampled());
+      geoLayersRef.current = {};
     } catch (err: any) {
       log.error(
         mongoLogId(1_001_000_188),

packages/compass-schema/src/stores/store.spec.ts

@@ -1,5 +1,4 @@
-import type { SchemaStore } from './store';
-import { activateSchemaPlugin } from './store';
+import { activateSchemaPlugin, type SchemaStore } from './store';
 import AppRegistry, { createActivateHelpers } from 'hadron-app-registry';
 import { expect } from 'chai';
 
@@ -9,6 +8,8 @@ import { createNoopLogger } from '@mongodb-js/compass-logging/provider';
 import type { FieldStoreService } from '@mongodb-js/compass-field-store';
 import { createNoopTrack } from '@mongodb-js/compass-telemetry/provider';
 import type { ConnectionInfoRef } from '@mongodb-js/compass-connections/provider';
+import { startAnalysis, stopAnalysis } from './reducer';
+import Sinon from 'sinon';
 
 const dummyLogger = createNoopLogger('TEST');
 const dummyTrack = createNoopTrack();
@@ -28,15 +29,24 @@ describe('Schema Store', function () {
   describe('#configureStore', function () {
     let store: SchemaStore;
     let deactivate: () => void;
+    let sandbox: Sinon.SinonSandbox;
     const localAppRegistry = new AppRegistry();
     const globalAppRegistry = new AppRegistry();
-    const dataService = 'test';
     const namespace = 'db.coll';
     const connectionInfoRef = {
       current: {},
     } as ConnectionInfoRef;
+    let sampleStub: Sinon.SinonStub;
+    let isCancelErrorStub: Sinon.SinonStub;
 
     beforeEach(async function () {
+      sandbox = Sinon.createSandbox();
+      sampleStub = sandbox.stub();
+      isCancelErrorStub = sandbox.stub();
+      const dataService = {
+        sample: sampleStub,
+        isCancelError: isCancelErrorStub,
+      };
       const plugin = activateSchemaPlugin(
         {
           namespace: namespace,
@@ -60,34 +70,42 @@ describe('Schema Store', function () {
 
     afterEach(function () {
       deactivate();
+      sandbox.reset();
     });
 
-    it('sets the local app registry', function () {
-      expect(store.localAppRegistry).to.equal(localAppRegistry);
-    });
-
-    it('sets the global app registry', function () {
-      expect(store.globalAppRegistry).to.equal(globalAppRegistry);
-    });
-
-    it('sets the data provider', function () {
-      expect(store.dataService).to.equal(dataService);
+    it('defaults analysis state to initial', function () {
+      expect(store.getState().analysisState).to.equal(ANALYSIS_STATE_INITIAL);
     });
 
-    it('sets the namespace', function () {
-      expect(store.ns).to.equal(namespace);
+    it('defaults the error to empty', function () {
+      expect(store.getState().errorMessage).to.equal('');
     });
 
-    it('defaults analysis state to initial', function () {
-      expect(store.state.analysisState).to.equal(ANALYSIS_STATE_INITIAL);
+    it('defaults the schema to null', function () {
+      expect(store.getState().schema).to.equal(null);
     });
 
-    it('defaults the error to empty', function () {
-      expect(store.state.errorMessage).to.equal('');
+    it('runs analysis', async function () {
+      const oldResultId = store.getState().resultId;
+      sampleStub.resolves([{ name: 'Hans' }, { name: 'Greta' }]);
+      await store.dispatch(startAnalysis());
+      expect(sampleStub).to.have.been.called;
+      const { analysisState, errorMessage, schema, resultId } =
+        store.getState();
+      expect(analysisState).to.equal('complete');
+      expect(!!errorMessage).to.be.false;
+      expect(schema).not.to.be.null;
+      expect(resultId).not.to.equal(oldResultId);
     });
 
-    it('defaults the schema to null', function () {
-      expect(store.state.schema).to.equal(null);
+    it('analysis can be aborted', async function () {
+      const analysisPromise = store.dispatch(startAnalysis());
+      expect(store.getState().analysisState).to.equal('analyzing');
+      sampleStub.rejects(new Error('abort'));
+      store.dispatch(stopAnalysis());
+      isCancelErrorStub.returns(true);
+      await analysisPromise;
+      expect(store.getState().analysisState).to.equal('initial');
     });
   });
 });

packages/compass-schema/src/stores/store.ts

@@ -98,4 +98,4 @@ export function configureStore(
   return store;
 }
 
-export type AtlasServiceStore = ReturnType<typeof configureStore>;
+export type SchemaStore = ReturnType<typeof configureStore>;