Skip to content

chore(schema): move non serializable services off of state to services #6717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ import React from 'react';
import { render, screen } from '@mongodb-js/testing-library-compass';
import { expect } from 'chai';
import sinon from 'sinon';
import {
createSandboxFromDefaultPreferences,
type PreferencesAccess,
} from 'compass-preferences-model';
import { PreferencesProvider } from 'compass-preferences-model/provider';
import type { AllPreferences } from 'compass-preferences-model';
import { SchemaToolbar } from './schema-toolbar';
import QueryBarPlugin from '@mongodb-js/compass-query-bar';
import {
Expand Down Expand Up @@ -35,33 +31,28 @@ const testErrorMessage =
const exportSchemaTestId = 'open-schema-export-button';

describe('SchemaToolbar', function () {
let defaultPreferences: PreferencesAccess;

before(async function () {
defaultPreferences = await createSandboxFromDefaultPreferences();
});

const renderSchemaToolbar = (
props: Partial<ComponentProps<typeof SchemaToolbar>> = {},
preferences: PreferencesAccess = defaultPreferences
preferences: Partial<AllPreferences> = {}
) => {
const queryBarProps = {};
render(
<PreferencesProvider value={preferences}>
<MockQueryBarPlugin {...(queryBarProps as any)}>
<SchemaToolbar
analysisState="complete"
errorMessage={''}
isOutdated={false}
onAnalyzeSchemaClicked={() => {}}
onResetClicked={() => {}}
sampleSize={10}
schemaResultId="123"
onExportSchemaClicked={() => {}}
{...props}
/>
</MockQueryBarPlugin>
</PreferencesProvider>
<MockQueryBarPlugin {...(queryBarProps as any)}>
<SchemaToolbar
analysisState="complete"
errorMessage={''}
isOutdated={false}
onAnalyzeSchemaClicked={() => {}}
onResetClicked={() => {}}
sampleSize={10}
schemaResultId="123"
onExportSchemaClicked={() => {}}
{...props}
/>
</MockQueryBarPlugin>,
{
preferences,
}
);
};

Expand Down Expand Up @@ -125,17 +116,14 @@ describe('SchemaToolbar', function () {
});

describe('when rendered with the enableExportSchema feature flag true', function () {
beforeEach(async function () {
const preferences = await createSandboxFromDefaultPreferences();
await preferences.savePreferences({
enableExportSchema: true,
});

beforeEach(function () {
renderSchemaToolbar(
{
sampleSize: 100,
},
preferences
{
enableExportSchema: true,
}
);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/compass-schema/src/modules/schema-analysis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ describe('schema-analysis', function () {
);
});

it('returns null if is cancelled', async function () {
it('returns undefined if is cancelled', async function () {
const dataService = {
sample: () => Promise.reject(new Error('test error')),
isCancelError: () => true,
Expand All @@ -214,7 +214,7 @@ describe('schema-analysis', function () {
dummyLogger
);

expect(result).to.equal(null);
expect(result).to.equal(undefined);
});

it('throws if sample throws', async function () {
Expand Down
7 changes: 3 additions & 4 deletions packages/compass-schema/src/modules/schema-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { AggregateOptions, Filter, Document } from 'mongodb';
import { analyzeDocuments } from 'mongodb-schema';
import type {
Schema,
SchemaAccessor,
ArraySchemaType,
DocumentSchemaType,
SchemaField,
Expand Down Expand Up @@ -35,8 +36,6 @@ function promoteMongoErrorCode(err?: Error & { code?: unknown }) {
return err;
}

export type SchemaAccessor = Awaited<ReturnType<typeof analyzeDocuments>>;

export const analyzeSchema = async (
dataService: DataService,
abortSignal: AbortSignal,
Expand All @@ -50,7 +49,7 @@ export const analyzeSchema = async (
| undefined,
aggregateOptions: AggregateOptions,
{ log, mongoLogId, debug }: Logger
): Promise<SchemaAccessor | null> => {
): Promise<SchemaAccessor | undefined> => {
try {
log.info(mongoLogId(1001000089), 'Schema', 'Starting schema analysis', {
ns,
Expand Down Expand Up @@ -82,7 +81,7 @@ export const analyzeSchema = async (
});
if (dataService.isCancelError(err)) {
debug('caught background operation terminated error', err);
return null;
return;
}

const error = promoteMongoErrorCode(err);
Expand Down
23 changes: 9 additions & 14 deletions packages/compass-schema/src/stores/schema-analysis-reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { addLayer, generateGeoQuery } from '../modules/geo';
import {
analyzeSchema,
calculateSchemaDepth,
type SchemaAccessor,
schemaContainsGeoData,
} from '../modules/schema-analysis';
import { capMaxTimeMSAtPreferenceLimit } from 'compass-preferences-model/provider';
Expand All @@ -32,7 +31,6 @@ export type SchemaAnalysisState = {
analysisState: AnalysisState;
errorMessage: string;
schema: Schema | null;
schemaAccessor: SchemaAccessor | null;
resultId: string;
};

Expand All @@ -48,7 +46,6 @@ export type AnalysisStartedAction = {

export type AnalysisFinishedAction = {
type: SchemaAnalysisActions.analysisFinished;
schemaAccessor: SchemaAccessor | null;
schema: Schema | null;
};

Expand All @@ -72,7 +69,6 @@ export const schemaAnalysisReducer: Reducer<SchemaAnalysisState, Action> = (
analysisState: ANALYSIS_STATE_ANALYZING,
errorMessage: '',
schema: null,
schemaAccessor: null,
};
}

Expand All @@ -88,7 +84,6 @@ export const schemaAnalysisReducer: Reducer<SchemaAnalysisState, Action> = (
? ANALYSIS_STATE_COMPLETE
: ANALYSIS_STATE_INITIAL,
schema: action.schema,
schemaAccessor: action.schemaAccessor,
resultId: resultId(),
};
}
Expand Down Expand Up @@ -129,7 +124,6 @@ const getInitialState = (): SchemaAnalysisState => ({
analysisState: ANALYSIS_STATE_INITIAL,
errorMessage: '',
schema: null,
schemaAccessor: null,
resultId: resultId(),
});

Expand Down Expand Up @@ -171,9 +165,9 @@ export const geoLayersDeleted = (
};

export const stopAnalysis = (): SchemaThunkAction<void> => {
return (dispatch, getState, { abortControllerRef }) => {
if (!abortControllerRef.current) return;
abortControllerRef.current?.abort();
return (dispatch, getState, { analysisAbortControllerRef }) => {
if (!analysisAbortControllerRef.current) return;
analysisAbortControllerRef.current?.abort();
};
};

Expand All @@ -191,7 +185,8 @@ export const startAnalysis = (): SchemaThunkAction<
dataService,
logger,
fieldStoreService,
abortControllerRef,
analysisAbortControllerRef,
schemaAccessorRef,
namespace,
geoLayersRef,
connectionInfoRef,
Expand Down Expand Up @@ -221,8 +216,8 @@ export const startAnalysis = (): SchemaThunkAction<
maxTimeMS: capMaxTimeMSAtPreferenceLimit(preferences, query.maxTimeMS),
};

abortControllerRef.current = new AbortController();
const abortSignal = abortControllerRef.current.signal;
analysisAbortControllerRef.current = new AbortController();
const abortSignal = analysisAbortControllerRef.current.signal;

try {
debug('analysis started');
Expand All @@ -238,6 +233,7 @@ export const startAnalysis = (): SchemaThunkAction<
driverOptions,
logger
);
schemaAccessorRef.current = schemaAccessor;
let schema: Schema | null = null;
if (schemaAccessor) {
schema = await schemaAccessor.getInternalSchema();
Expand All @@ -254,7 +250,6 @@ export const startAnalysis = (): SchemaThunkAction<
dispatch({
type: SchemaAnalysisActions.analysisFinished,
schema,
schemaAccessor,
});

// track schema analyzed
Expand Down Expand Up @@ -282,7 +277,7 @@ export const startAnalysis = (): SchemaThunkAction<
error: err as Error,
});
} finally {
abortControllerRef.current = undefined;
analysisAbortControllerRef.current = undefined;
}
};
};
43 changes: 17 additions & 26 deletions packages/compass-schema/src/stores/schema-export-reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import type {
InternalSchema,
MongoDBJSONSchema,
ExpandedJSONSchema,
SchemaAccessor,
} from 'mongodb-schema';

import type { SchemaThunkAction } from './store';
import { isAction } from '../utils';
import {
calculateSchemaDepth,
schemaContainsGeoData,
type SchemaAccessor,
} from '../modules/schema-analysis';
import { openToast } from '@mongodb-js/compass-components';

Expand All @@ -23,7 +23,6 @@ export type SchemaFormat =
| 'legacyJSON';
export type ExportStatus = 'inprogress' | 'complete' | 'error';
export type SchemaExportState = {
abortController?: AbortController;
isOpen: boolean;
isLegacyBannerOpen: boolean;
legacyBannerChoice?: 'legacy' | 'export';
Expand Down Expand Up @@ -81,11 +80,8 @@ export const closeExportSchema = (): SchemaThunkAction<
void,
CloseExportSchemaAction
> => {
return (dispatch, getState) => {
const {
schemaExport: { abortController },
} = getState();
abortController?.abort();
return (dispatch, getState, { exportAbortControllerRef }) => {
exportAbortControllerRef.current?.abort();

return dispatch({
type: SchemaExportActions.closeExportSchema,
Expand All @@ -99,7 +95,6 @@ export type CancelExportSchemaAction = {

export type ChangeExportSchemaFormatStartedAction = {
type: SchemaExportActions.changeExportSchemaFormatStarted;
abortController: AbortController;
exportFormat: SchemaFormat;
};

Expand All @@ -117,11 +112,8 @@ export const cancelExportSchema = (): SchemaThunkAction<
void,
CancelExportSchemaAction
> => {
return (dispatch, getState) => {
const {
schemaExport: { abortController },
} = getState();
abortController?.abort();
return (dispatch, getState, { exportAbortControllerRef }) => {
exportAbortControllerRef.current?.abort();

return dispatch({
type: SchemaExportActions.cancelExportSchema,
Expand Down Expand Up @@ -169,19 +161,19 @@ export const changeExportSchemaFormat = (
| ChangeExportSchemaFormatErroredAction
| ChangeExportSchemaFormatCompletedAction
> => {
return async (dispatch, getState, { logger: { log } }) => {
const {
schemaExport: { abortController: existingAbortController },
} = getState();

return async (
dispatch,
getState,
{ logger: { log }, exportAbortControllerRef, schemaAccessorRef }
) => {
// If we're already in progress we abort their current operation.
existingAbortController?.abort();
exportAbortControllerRef.current?.abort();

const abortController = new AbortController();
exportAbortControllerRef.current = new AbortController();
const abortSignal = exportAbortControllerRef.current.signal;

dispatch({
type: SchemaExportActions.changeExportSchemaFormatStarted,
abortController,
exportFormat,
});

Expand All @@ -196,18 +188,18 @@ export const changeExportSchemaFormat = (
}
);

const schemaAccessor = getState().schemaAnalysis.schemaAccessor;
const schemaAccessor = schemaAccessorRef.current;
if (!schemaAccessor) {
throw new Error('No schema analysis available');
}

exportedSchema = await getSchemaByFormat({
schemaAccessor,
exportFormat,
signal: abortController.signal,
signal: abortSignal,
});
} catch (err: any) {
if (abortController.signal.aborted) {
if (abortSignal.aborted) {
return;
}
log.error(
Expand All @@ -226,7 +218,7 @@ export const changeExportSchemaFormat = (
return;
}

if (abortController.signal.aborted) {
if (abortSignal.aborted) {
return;
}

Expand Down Expand Up @@ -319,7 +311,6 @@ export const schemaExportReducer: Reducer<SchemaExportState, Action> = (
) {
return {
...state,
abortController: action.abortController,
exportStatus: 'inprogress',
exportFormat: action.exportFormat,
};
Expand Down
Loading
Loading