From 64ae5fd22809f3a38e6774942b43d094ec612a60 Mon Sep 17 00:00:00 2001 From: Basit Date: Thu, 21 Apr 2022 12:09:44 +0200 Subject: [PATCH 01/11] feat(aggregation-count): count docs --- .../pipeline-pagination-count.tsx | 90 +++++++++++++++ .../pipeline-pagination.tsx | 17 ++- .../src/modules/count-documents.ts | 109 ++++++++++++++++++ .../compass-aggregations/src/modules/index.ts | 6 + 4 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx create mode 100644 packages/compass-aggregations/src/modules/count-documents.ts diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx new file mode 100644 index 00000000000..d26ad690f30 --- /dev/null +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx @@ -0,0 +1,90 @@ +import React from 'react'; +import { connect } from 'react-redux'; +import { + css, + spacing, + Body, + Link, + uiColors, + Tooltip, + SpinLoader, +} from '@mongodb-js/compass-components'; + +import type { RootState } from '../../modules'; +import { countDocuments } from '../../modules/count-documents'; + +type PipelinePaginationCountProps = { + loading: boolean; + count?: number; + onCount: () => void; +}; + +const countButtonStyles = css({ + backgroundColor: 'transparent', + border: 'none', + display: 'inline', + height: spacing[4] + spacing[1], + ':focus': { + outline: `${spacing[1]}px auto ${uiColors.focus}`, + }, +}); + +export const PipelinePaginationCount: React.FunctionComponent = + ({ loading, count, onCount }) => { + const countDefinition = ` + In order to have the final count of documents we need to run the + aggregation again. This will be the equivalent of adding a + $count as the last stage of the pipeline. + `; + + const testId = 'pipeline-pagination-count'; + + if (loading) { + return ( +
+ +
+ ); + } + + if (count) { + return ( +
+ of {count} +
+ ); + } + + return ( +
+ ( + onCount()} + > + {children} + count results + + )} + > + {countDefinition} + +
+ ); + }; + +const mapState = ({ countDocuments: { loading, count } }: RootState) => ({ + loading, + count, +}); + +const mapDispatch = { + onCount: countDocuments, +}; + +export default connect(mapState, mapDispatch)(PipelinePaginationCount); diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.tsx index 80004bdb593..f549af796f5 100644 --- a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.tsx +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.tsx @@ -11,6 +11,8 @@ import { import type { RootState } from '../../modules'; import { fetchNextPage, fetchPrevPage } from '../../modules/aggregation'; +import PipelinePaginationCount from './pipeline-pagination-count'; + type PipelinePaginationProps = { showingFrom: number; showingTo: number; @@ -27,6 +29,12 @@ const containerStyles = css({ gap: spacing[2], }); +const paginationStyles = css({ + display: 'flex', + gap: spacing[1], + alignItems: 'center', +}); + export const PipelinePagination: React.FunctionComponent = ({ showingFrom, @@ -40,9 +48,12 @@ export const PipelinePagination: React.FunctionComponent {!isCountDisabled && ( - - Showing {showingFrom} – {showingTo} - +
+ + Showing {showingFrom} – {showingTo} + + +
)}
= (state = INITIAL_STATE, action) => { + switch (action.type) { + case WorkspaceActionTypes.WorkspaceChanged: + return { + ...INITIAL_STATE, + }; + case ActionTypes.CountStarted: + return { + loading: true, + }; + case ActionTypes.CountFinished: + return { + loading: false, + count: action.count, + }; + case ActionTypes.CountFailed: + return { + ...state, + loading: false, + }; + default: + return state; + } +}; + +export const countDocuments = (): ThunkAction< + void, + RootState, + void, + Actions +> => { + return async (dispatch, getState) => { + const { + pipeline, + namespace, + maxTimeMS, + collation, + dataService: { dataService } + } = getState(); + + if (!dataService) { + return; + } + + try { + dispatch({ type: ActionTypes.CountStarted }); + const stages = pipeline.map(generateStage).filter(x => Object.keys(x).length > 0); + const options: AggregateOptions = { + maxTimeMS: maxTimeMS || DEFAULT_MAX_TIME_MS, + allowDiskUse: true, + collation: collation || undefined, + }; + const cursor = dataService + .aggregate(namespace, [...stages, { $count: 'count' }], options) + .skip(0).limit(1) + const documents = await cursor.toArray(); + dispatch({ + type: ActionTypes.CountFinished, + count: documents[0].count, + }); + } catch (e) { + dispatch({ type: ActionTypes.CountFailed }); + } + } +} +export default reducer; \ No newline at end of file diff --git a/packages/compass-aggregations/src/modules/index.ts b/packages/compass-aggregations/src/modules/index.ts index 7bd460a7027..ee3e088bc72 100644 --- a/packages/compass-aggregations/src/modules/index.ts +++ b/packages/compass-aggregations/src/modules/index.ts @@ -119,6 +119,10 @@ import aggregation, { INITIAL_STATE as AGGREGATION_INITIAL_STATE } from './aggregation'; +import countDocuments, { + INITIAL_STATE as COUNT_INITIAL_STATE +} from './count-documents'; + import workspace, { INITIAL_STATE as WORKSPACE_INITIAL_STATE } from './workspace'; @@ -174,6 +178,7 @@ export const INITIAL_STATE = { updateViewError: UPDATE_VIEW_ERROR_INITIAL_STATE, aggregation: AGGREGATION_INITIAL_STATE, workspace: WORKSPACE_INITIAL_STATE, + countDocuments: COUNT_INITIAL_STATE, }; export type RootState = typeof INITIAL_STATE; @@ -254,6 +259,7 @@ const appReducer = combineReducers({ updateViewError, aggregation, workspace, + countDocuments, }); /** From 4c93906d7fbd2410330f13a2a4887ec565613ec6 Mon Sep 17 00:00:00 2001 From: Basit Date: Thu, 21 Apr 2022 14:24:22 +0200 Subject: [PATCH 02/11] feat(aggregation-count): count fix --- .../pipeline-pagination-count.tsx | 3 ++- .../pipeline-pagination.tsx | 21 ++++++++++++------- .../src/modules/count-documents.ts | 7 ++----- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx index d26ad690f30..c26053bd176 100644 --- a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx @@ -47,7 +47,7 @@ export const PipelinePaginationCount: React.FunctionComponent of {count} @@ -61,6 +61,7 @@ export const PipelinePaginationCount: React.FunctionComponent ( ({ - showingFrom: calculateShowingFrom({ limit, page }), - showingTo: calculateShowingTo({ + countDocuments: { count }, +}: RootState) => { + const showingFrom = calculateShowingFrom({ limit, page }); + const showingTo = calculateShowingTo({ limit, page, documentCount: documents.length, - }), - isCountDisabled: Boolean(error), - isPrevDisabled: page <= 1 || loading || Boolean(error), - isNextDisabled: isLast || loading || Boolean(error), -}); + }); + return { + showingFrom, + showingTo, + isCountDisabled: Boolean(error), + isPrevDisabled: page <= 1 || loading || Boolean(error), + isNextDisabled: isLast || loading || Boolean(error) || count === showingTo, + }; +}; const mapDispatch = { onPrev: fetchPrevPage, diff --git a/packages/compass-aggregations/src/modules/count-documents.ts b/packages/compass-aggregations/src/modules/count-documents.ts index 187e695855a..3509aae7cc8 100644 --- a/packages/compass-aggregations/src/modules/count-documents.ts +++ b/packages/compass-aggregations/src/modules/count-documents.ts @@ -44,9 +44,7 @@ export const INITIAL_STATE: State = { const reducer: Reducer = (state = INITIAL_STATE, action) => { switch (action.type) { case WorkspaceActionTypes.WorkspaceChanged: - return { - ...INITIAL_STATE, - }; + return INITIAL_STATE; case ActionTypes.CountStarted: return { loading: true, @@ -58,7 +56,6 @@ const reducer: Reducer = (state = INITIAL_STA }; case ActionTypes.CountFailed: return { - ...state, loading: false, }; default: @@ -99,7 +96,7 @@ export const countDocuments = (): ThunkAction< const documents = await cursor.toArray(); dispatch({ type: ActionTypes.CountFinished, - count: documents[0].count, + count: documents[0]?.count ?? 0, }); } catch (e) { dispatch({ type: ActionTypes.CountFailed }); From 1b55829f1d43b83b4a51524656db090c9964e8c2 Mon Sep 17 00:00:00 2001 From: Basit Date: Thu, 21 Apr 2022 14:43:35 +0200 Subject: [PATCH 03/11] feat(aggregation-count): count tests --- .../pipeline-pagination-count.spec.tsx | 48 +++++++++++++++++++ .../pipeline-pagination-count.tsx | 2 +- .../pipeline-pagination.spec.tsx | 27 +++++++---- .../src/stores/store.spec.js | 1 + 4 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.spec.tsx diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.spec.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.spec.tsx new file mode 100644 index 00000000000..bff678094de --- /dev/null +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.spec.tsx @@ -0,0 +1,48 @@ +import React from 'react'; +import { render, screen, within } from '@testing-library/react'; +import { expect } from 'chai'; +import { spy } from 'sinon'; +import userEvent from '@testing-library/user-event'; + +import { PipelinePaginationCount } from './pipeline-pagination-count'; + +describe('PipelinePagination', function () { + it('renders count button by default', function () { + render( + {}} + /> + ); + + const container = screen.getByTestId('pipeline-pagination-count'); + expect(within(container).getByTestId('pipeline-pagination-count-action')).to + .exist; + }); + it('calls onCount when clicked', function () { + const onCountSpy = spy(); + render( + + ); + + expect(onCountSpy.calledOnce).to.be.false; + const container = screen.getByTestId('pipeline-pagination-count'); + userEvent.click( + within(container).getByTestId('pipeline-pagination-count-action') + ); + expect(onCountSpy.calledOnce).to.be.true; + }); + it('renders count of results', function () { + render( + {}} /> + ); + + const container = screen.getByTestId('pipeline-pagination-count'); + expect(within(container).getByText('of 20')).to.exist; + }); +}); diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx index c26053bd176..6d6a85406c2 100644 --- a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination-count.tsx @@ -63,7 +63,7 @@ export const PipelinePaginationCount: React.FunctionComponent onCount()} diff --git a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.spec.tsx b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.spec.tsx index bb9121f3993..002c23f4045 100644 --- a/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.spec.tsx +++ b/packages/compass-aggregations/src/components/pipeline-results-workspace/pipeline-pagination.spec.tsx @@ -3,6 +3,9 @@ import { render, screen, within } from '@testing-library/react'; import { expect } from 'chai'; import { spy } from 'sinon'; import userEvent from '@testing-library/user-event'; +import { Provider } from 'react-redux'; + +import configureStore from '../../stores/store'; import { PipelinePagination, @@ -12,16 +15,18 @@ import { const renderPipelinePagination = (props: Record = {}) => { render( - {}} - onNext={() => {}} - {...props} - /> + + {}} + onNext={() => {}} + {...props} + /> + ); }; @@ -33,6 +38,8 @@ describe('PipelinePagination', function () { expect( within(container).getByTestId('pipeline-pagination-desc').textContent ).to.equal('Showing 1 – 20'); + expect(within(container).getByTestId('pipeline-pagination-count')).to + .exist; expect(within(container).getByTestId('pipeline-pagination-prev-action')) .to.exist; expect(within(container).getByTestId('pipeline-pagination-next-action')) diff --git a/packages/compass-aggregations/src/stores/store.spec.js b/packages/compass-aggregations/src/stores/store.spec.js index 862d7adab0e..4cafcf709c6 100644 --- a/packages/compass-aggregations/src/stores/store.spec.js +++ b/packages/compass-aggregations/src/stores/store.spec.js @@ -259,6 +259,7 @@ describe('Aggregation Store', function() { updateViewError: INITIAL_STATE.updateViewError, aggregation: INITIAL_STATE.aggregation, workspace: INITIAL_STATE.workspace, + countDocuments: INITIAL_STATE.countDocuments, }); }); }); From cc391a701f4c4501e00a054aa6cacd2cfd939262 Mon Sep 17 00:00:00 2001 From: Basit Date: Thu, 21 Apr 2022 15:51:51 +0200 Subject: [PATCH 04/11] feat(aggregation-count): use aggregation count on export --- .../src/modules/aggregation.ts | 4 +- .../export-select-output.jsx | 1 - .../components/progress-bar/progress-bar.jsx | 50 ++++++------------- .../src/modules/export.js | 22 ++++---- 4 files changed, 28 insertions(+), 49 deletions(-) diff --git a/packages/compass-aggregations/src/modules/aggregation.ts b/packages/compass-aggregations/src/modules/aggregation.ts index 0fd152d7bfb..b4a62752aef 100644 --- a/packages/compass-aggregations/src/modules/aggregation.ts +++ b/packages/compass-aggregations/src/modules/aggregation.ts @@ -282,6 +282,7 @@ export const exportAggregationResults = (): ThunkAction< namespace, maxTimeMS, collation, + countDocuments: { count } } = getState(); const stages = pipeline @@ -298,7 +299,8 @@ export const exportAggregationResults = (): ThunkAction< aggregation: { stages, options - } + }, + count, }) ); return; diff --git a/packages/compass-import-export/src/components/export-select-output/export-select-output.jsx b/packages/compass-import-export/src/components/export-select-output/export-select-output.jsx index b336b648294..4e21de1eb1e 100644 --- a/packages/compass-import-export/src/components/export-select-output/export-select-output.jsx +++ b/packages/compass-import-export/src/components/export-select-output/export-select-output.jsx @@ -112,7 +112,6 @@ class ExportSelectOutput extends PureComponent { cancel={this.props.cancelExport} message={MESSAGES[this.props.status]} docsWritten={this.props.exportedDocsCount} - isAggregation={this.props.isAggregation} />
); diff --git a/packages/compass-import-export/src/components/progress-bar/progress-bar.jsx b/packages/compass-import-export/src/components/progress-bar/progress-bar.jsx index 11125137f47..e590688c20c 100644 --- a/packages/compass-import-export/src/components/progress-bar/progress-bar.jsx +++ b/packages/compass-import-export/src/components/progress-bar/progress-bar.jsx @@ -37,21 +37,20 @@ class ProgressBar extends PureComponent { progressLabel: PropTypes.func, progressTitle: PropTypes.func, withErrors: PropTypes.bool, - isAggregation: PropTypes.bool, }; static defaultProps = { - progressLabel(formattedWritten, formattedTotal, isAggregation) { - if (isAggregation) { - return formattedWritten; + progressLabel(written, total) { + if (!total) { + return formatNumber(written); } - return `${formattedWritten}\u00A0/\u00A0${formattedTotal}`; + return `${formatNumber(written)}\u00A0/\u00A0${formatNumber(total)}`; }, - progressTitle(formattedWritten, formattedTotal, isAggregation) { - if (isAggregation) { - return `${formattedWritten} documents`; + progressTitle(written, total) { + if (!total) { + return `${formatNumber(written)} documents`; } - return `${formattedWritten} documents out of ${formattedTotal}`; + return `${formatNumber(written)} documents out of ${formatNumber(total)}`; }, }; @@ -109,23 +108,12 @@ class ProgressBar extends PureComponent { }; renderStats() { - const { - docsTotal, - docsWritten, - progressLabel, - progressTitle, - isAggregation, - } = this.props; - - const formattedWritten = formatNumber(docsWritten); - const formattedTotal = formatNumber(docsTotal); - + const { docsTotal, docsWritten, progressLabel, progressTitle } = this.props; + const title = progressTitle(docsWritten, docsTotal); + const label = progressLabel(docsWritten, docsTotal); return ( -

- {progressLabel(formattedWritten, formattedTotal, isAggregation)} +

+ {label}

); } @@ -136,14 +124,8 @@ class ProgressBar extends PureComponent { * @returns {React.Component} The component. */ render() { - const { - message, - status, - docsProcessed, - docsTotal, - docsWritten, - isAggregation, - } = this.props; + const { message, status, docsProcessed, docsTotal, docsWritten } = + this.props; if (status === UNSPECIFIED) { return null; @@ -151,7 +133,7 @@ class ProgressBar extends PureComponent { return (
- {!isAggregation && ( + {docsTotal && (
{ const getExportSource = (dataService, ns, exportData) => { if (exportData.aggregation) { const { stages, options } = exportData.aggregation; - return getAggregationExportSource(dataService, ns, stages, options); + return { + columns: true, + source: dataService.aggregate(ns, stages, options), + numDocsToExport: exportData.count, + }; } return getQueryExportSource( dataService, @@ -550,14 +554,6 @@ const getExportSource = (dataService, ns, exportData) => { ); }; -const getAggregationExportSource = (dataService, ns, stages, options) => { - return { - columns: true, - source: dataService.aggregate(ns, stages, options), - numDocsToExport: 0, - }; -}; - const getQueryExportSource = async ( dataService, ns, From 03a3ef41da0b96522762cd3be02b57ab9a5a84f4 Mon Sep 17 00:00:00 2001 From: Basit Date: Sat, 23 Apr 2022 01:48:24 +0200 Subject: [PATCH 05/11] feat(aggregation-count): cancel count --- .../src/modules/count-documents.ts | 53 ++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/packages/compass-aggregations/src/modules/count-documents.ts b/packages/compass-aggregations/src/modules/count-documents.ts index 3509aae7cc8..a1fb398e50b 100644 --- a/packages/compass-aggregations/src/modules/count-documents.ts +++ b/packages/compass-aggregations/src/modules/count-documents.ts @@ -4,6 +4,7 @@ import type { ThunkAction } from 'redux-thunk'; import type { RootState } from '.'; import { DEFAULT_MAX_TIME_MS } from '../constants'; import { generateStage } from './stage'; +import { aggregatePipeline } from '../utils/cancellable-aggregation'; import type { Actions as WorkspaceActions } from './workspace'; import { ActionTypes as WorkspaceActionTypes } from './workspace'; @@ -11,11 +12,13 @@ import { ActionTypes as WorkspaceActionTypes } from './workspace'; export enum ActionTypes { CountStarted = 'compass-aggregations/countStarted', CountFinished = 'compass-aggregations/countFinished', + CountCancelled = 'compass-aggregations/countCancelled', CountFailed = 'compass-aggregations/countFailed', }; type CountStartedAction = { type: ActionTypes.CountStarted; + abortController: AbortController; }; type CountFinishedAction = { @@ -23,6 +26,10 @@ type CountFinishedAction = { count: number; }; +type CountCancelledAction = { + type: ActionTypes.CountCancelled; +}; + type CountFailedAction = { type: ActionTypes.CountFailed; }; @@ -30,11 +37,13 @@ type CountFailedAction = { export type Actions = | CountStartedAction | CountFinishedAction + | CountCancelledAction | CountFailedAction; export type State = { count?: number; loading: boolean; + abortController?: AbortController; }; export const INITIAL_STATE: State = { @@ -48,21 +57,39 @@ const reducer: Reducer = (state = INITIAL_STA case ActionTypes.CountStarted: return { loading: true, + abortController: action.abortController, }; case ActionTypes.CountFinished: return { loading: false, + abortController: undefined, count: action.count, }; + case ActionTypes.CountCancelled: + return { + ...state, + abortController: undefined, + }; case ActionTypes.CountFailed: return { + ...state, loading: false, + abortController: undefined, }; default: return state; } }; +export const cancelCount = (): ThunkAction => { + return (_dispatch, getState) => { + const { + countDocuments: { abortController } + } = getState(); + abortController?.abort(); + }; +}; + export const countDocuments = (): ThunkAction< void, RootState, @@ -83,23 +110,37 @@ export const countDocuments = (): ThunkAction< } try { - dispatch({ type: ActionTypes.CountStarted }); + const abortController = new AbortController(); + const signal = abortController.signal; + dispatch({ + type: ActionTypes.CountStarted, + abortController, + }); + const stages = pipeline.map(generateStage).filter(x => Object.keys(x).length > 0); const options: AggregateOptions = { maxTimeMS: maxTimeMS || DEFAULT_MAX_TIME_MS, allowDiskUse: true, collation: collation || undefined, }; - const cursor = dataService - .aggregate(namespace, [...stages, { $count: 'count' }], options) - .skip(0).limit(1) - const documents = await cursor.toArray(); + + const documents = await aggregatePipeline( + dataService, + signal, + namespace, + [...stages, { $count: 'count' }], + options, + 0, + 1, + ); dispatch({ type: ActionTypes.CountFinished, count: documents[0]?.count ?? 0, }); } catch (e) { - dispatch({ type: ActionTypes.CountFailed }); + dispatch({ + type: ActionTypes.CountFailed, + }); } } } From 35e37acc36d35751cbafa09bdfadfb28eae7fe35 Mon Sep 17 00:00:00 2001 From: Basit Date: Sat, 23 Apr 2022 01:48:51 +0200 Subject: [PATCH 06/11] feat(aggregation-count): cancel when user switches to builder view --- .../src/modules/workspace.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/compass-aggregations/src/modules/workspace.ts b/packages/compass-aggregations/src/modules/workspace.ts index 26486d6fcf0..a397c7656ee 100644 --- a/packages/compass-aggregations/src/modules/workspace.ts +++ b/packages/compass-aggregations/src/modules/workspace.ts @@ -1,7 +1,10 @@ import type { Reducer } from 'redux'; -import { ActionTypes as AggregationActionTypes } from './aggregation'; +import { ActionTypes as AggregationActionTypes, cancelAggregation } from './aggregation'; import type { Actions as AggregationActions } from './aggregation'; +import type { ThunkAction } from 'redux-thunk'; +import type { RootState } from '.'; +import { cancelCount } from './count-documents'; export type Workspace = 'builder' | 'results'; @@ -30,9 +33,17 @@ const reducer: Reducer = (state = INITIAL_S } }; -export const changeWorkspace = (view: Workspace): WorkspaceChangedAction => ({ - type: ActionTypes.WorkspaceChanged, - view, -}); - +export const changeWorkspace = (view: Workspace): ThunkAction => { + return (dispatch) => { + // As user switches to builder view, we cancel running ops + if (view === 'builder') { + dispatch(cancelAggregation()); + dispatch(cancelCount()); + } + dispatch({ + type: ActionTypes.WorkspaceChanged, + view, + }); + }; +}; export default reducer; From 916586b732aed0a13d7cc57fd810ca4d8588995d Mon Sep 17 00:00:00 2001 From: Basit Date: Tue, 26 Apr 2022 00:02:07 +0200 Subject: [PATCH 07/11] feat(aggregation-count): cancel aggregation --- .../src/modules/aggregation.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/compass-aggregations/src/modules/aggregation.ts b/packages/compass-aggregations/src/modules/aggregation.ts index b4a62752aef..f3d1c5189ff 100644 --- a/packages/compass-aggregations/src/modules/aggregation.ts +++ b/packages/compass-aggregations/src/modules/aggregation.ts @@ -187,10 +187,14 @@ export const cancelAggregation = (): ThunkAction< const { aggregation: { abortController } } = getState(); - abortController?.abort(); + _abortAggregation(abortController); }; }; +const _abortAggregation = (controller?: AbortController): void => { + controller?.abort(); +}; + const fetchAggregationData = (page: number): ThunkAction< Promise, RootState, @@ -204,13 +208,22 @@ const fetchAggregationData = (page: number): ThunkAction< maxTimeMS, collation, dataService: { dataService }, - aggregation: { limit, documents: _documents, page: _page, isLast: _isLast }, + aggregation: { + limit, + page: _page, + isLast: _isLast, + documents: _documents, + abortController: _abortController, + }, } = getState(); if (!dataService) { return; } + // Cancel the existing aggregate + _abortAggregation(_abortController); + try { const abortController = new AbortController(); const signal = abortController.signal; From 95aaa29ef0db0aea2c9d047194c4cd2f4de09355 Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Apr 2022 04:12:19 +0200 Subject: [PATCH 08/11] feat(aggregation-count): handle run race condition --- .../src/modules/aggregation.spec.ts | 4 ++ .../src/modules/aggregation.ts | 56 ++++++++++++++----- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/packages/compass-aggregations/src/modules/aggregation.spec.ts b/packages/compass-aggregations/src/modules/aggregation.spec.ts index 9faaa4bad31..715e095762e 100644 --- a/packages/compass-aggregations/src/modules/aggregation.spec.ts +++ b/packages/compass-aggregations/src/modules/aggregation.spec.ts @@ -97,6 +97,7 @@ describe('aggregation module', function () { loading: false, error: undefined, abortController: undefined, + previousPageData: undefined, }); }); @@ -152,6 +153,7 @@ describe('aggregation module', function () { loading: false, error: undefined, abortController: undefined, + previousPageData: undefined, }); }); @@ -205,6 +207,7 @@ describe('aggregation module', function () { loading: false, error: undefined, abortController: undefined, + previousPageData: undefined, }); }); it('nextPage -> on last page', async function () { @@ -277,6 +280,7 @@ describe('aggregation module', function () { loading: false, error: undefined, abortController: undefined, + previousPageData: undefined, }); }); it('prevPage -> on first page', async function () { diff --git a/packages/compass-aggregations/src/modules/aggregation.ts b/packages/compass-aggregations/src/modules/aggregation.ts index f3d1c5189ff..f028adf788b 100644 --- a/packages/compass-aggregations/src/modules/aggregation.ts +++ b/packages/compass-aggregations/src/modules/aggregation.ts @@ -19,12 +19,20 @@ export enum ActionTypes { AggregationStarted = 'compass-aggregations/aggregationStarted', AggregationFinished = 'compass-aggregations/aggregationFinished', AggregationFailed = 'compass-aggregations/aggregationFailed', + AggregationCancelled = 'compass-aggregations/aggregationCancelled', LastPageReached = 'compass-aggregations/lastPageReached', } +type PreviousPageData = { + page: number; + isLast: boolean; + documents: Document[]; +}; + type AggregationStartedAction = { type: ActionTypes.AggregationStarted; abortController: AbortController; + previousPageData: PreviousPageData; }; type AggregationFinishedAction = { @@ -40,6 +48,10 @@ type AggregationFailedAction = { page: number; }; +type AggregationCancelledAction = { + type: ActionTypes.AggregationCancelled; +}; + type LastPageReachedAction = { type: ActionTypes.LastPageReached; page: number; @@ -49,6 +61,7 @@ export type Actions = | AggregationStartedAction | AggregationFinishedAction | AggregationFailedAction + | AggregationCancelledAction | LastPageReachedAction; export type State = { @@ -59,6 +72,7 @@ export type State = { loading: boolean; abortController?: AbortController; error?: string; + previousPageData?: PreviousPageData; }; export const INITIAL_STATE: State = { @@ -72,11 +86,7 @@ export const INITIAL_STATE: State = { const reducer: Reducer = (state = INITIAL_STATE, action) => { switch (action.type) { case WorkspaceActionTypes.WorkspaceChanged: - return { - ...INITIAL_STATE, - page: 1, - limit: 20, - }; + return INITIAL_STATE; case ActionTypes.AggregationStarted: return { ...state, @@ -84,6 +94,7 @@ const reducer: Reducer = (state = INITIAL_STA error: undefined, documents: [], abortController: action.abortController, + previousPageData: action.previousPageData, }; case ActionTypes.AggregationFinished: return { @@ -94,6 +105,7 @@ const reducer: Reducer = (state = INITIAL_STA loading: false, abortController: undefined, error: undefined, + previousPageData: undefined, }; case ActionTypes.AggregationFailed: return { @@ -103,6 +115,17 @@ const reducer: Reducer = (state = INITIAL_STA abortController: undefined, error: action.error, page: action.page, + previousPageData: undefined, + }; + case ActionTypes.AggregationCancelled: + return { + ...state, + loading: false, + abortController: undefined, + documents: state.previousPageData?.documents || [], + page: state.previousPageData?.page || 1, + isLast: state.previousPageData?.isLast || false, + previousPageData: undefined, }; case ActionTypes.LastPageReached: return { @@ -182,12 +205,17 @@ export const cancelAggregation = (): ThunkAction< void, Actions > => { - return (_dispatch, getState) => { + return (dispatch, getState) => { track('Aggregation Canceled'); const { aggregation: { abortController } } = getState(); _abortAggregation(abortController); + // In order to avoid the race condition between user cancel and cancel triggered + // in fetchAggregationData, we dispatch ActionTypes.AggregationCancelled here. + dispatch({ + type: ActionTypes.AggregationCancelled, + }); }; }; @@ -230,6 +258,11 @@ const fetchAggregationData = (page: number): ThunkAction< dispatch({ type: ActionTypes.AggregationStarted, abortController, + previousPageData: { + page: _page, + isLast: _isLast, + documents: _documents, + }, }); const stages = pipeline.map(generateStage).filter(x => Object.keys(x).length > 0); @@ -260,15 +293,8 @@ const fetchAggregationData = (page: number): ThunkAction< }); } } catch (e) { - // On cancel, we show the previous state - if ((e as Error).name === PROMISE_CANCELLED_ERROR) { - dispatch({ - type: ActionTypes.AggregationFinished, - documents: _documents, - page: _page, - isLast: _isLast, - }); - } else { + // User cancel is handled in cancelAggregation + if ((e as Error).name !== PROMISE_CANCELLED_ERROR) { dispatch({ type: ActionTypes.AggregationFailed, error: (e as Error).message, From 5ea676a4def6d062a27a5535e3ed6ce840a1807d Mon Sep 17 00:00:00 2001 From: Basit <1305718+mabaasit@users.noreply.github.com> Date: Wed, 27 Apr 2022 04:13:33 +0200 Subject: [PATCH 09/11] Update packages/compass-import-export/src/modules/export.js Co-authored-by: Sergey Petushkov --- packages/compass-import-export/src/modules/export.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/compass-import-export/src/modules/export.js b/packages/compass-import-export/src/modules/export.js index 54559bde37c..7de3fb96039 100644 --- a/packages/compass-import-export/src/modules/export.js +++ b/packages/compass-import-export/src/modules/export.js @@ -384,9 +384,10 @@ export const openExport = ({ } = getState(); try { const count = - !isAggregation && !maybeCount + maybeCount ?? + (!isAggregation ? await fetchDocumentCount(dataService, namespace, query) - : maybeCount; + : null); dispatch(nsChanged(namespace)); dispatch(onModalOpen({ namespace, query, count, aggregation })); From e4b21d5c428f5818edee02aa1cfc208cb44fab0a Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Apr 2022 14:51:54 +0200 Subject: [PATCH 10/11] feat(aggregation-count): clean up action --- .../src/modules/aggregation.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/compass-aggregations/src/modules/aggregation.ts b/packages/compass-aggregations/src/modules/aggregation.ts index f028adf788b..9cec40c3a47 100644 --- a/packages/compass-aggregations/src/modules/aggregation.ts +++ b/packages/compass-aggregations/src/modules/aggregation.ts @@ -32,7 +32,6 @@ type PreviousPageData = { type AggregationStartedAction = { type: ActionTypes.AggregationStarted; abortController: AbortController; - previousPageData: PreviousPageData; }; type AggregationFinishedAction = { @@ -94,7 +93,11 @@ const reducer: Reducer = (state = INITIAL_STA error: undefined, documents: [], abortController: action.abortController, - previousPageData: action.previousPageData, + previousPageData: { + page: state.page, + documents: state.documents, + isLast: state.isLast, + }, }; case ActionTypes.AggregationFinished: return { @@ -238,9 +241,6 @@ const fetchAggregationData = (page: number): ThunkAction< dataService: { dataService }, aggregation: { limit, - page: _page, - isLast: _isLast, - documents: _documents, abortController: _abortController, }, } = getState(); @@ -258,11 +258,6 @@ const fetchAggregationData = (page: number): ThunkAction< dispatch({ type: ActionTypes.AggregationStarted, abortController, - previousPageData: { - page: _page, - isLast: _isLast, - documents: _documents, - }, }); const stages = pipeline.map(generateStage).filter(x => Object.keys(x).length > 0); From 9e1c3a3b657c8abc43ed0d439d788ec236325785 Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Apr 2022 15:56:15 +0200 Subject: [PATCH 11/11] feat(aggregation-count): remove cancelled --- .../src/modules/count-documents.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/compass-aggregations/src/modules/count-documents.ts b/packages/compass-aggregations/src/modules/count-documents.ts index a1fb398e50b..3dfca997c1f 100644 --- a/packages/compass-aggregations/src/modules/count-documents.ts +++ b/packages/compass-aggregations/src/modules/count-documents.ts @@ -12,7 +12,6 @@ import { ActionTypes as WorkspaceActionTypes } from './workspace'; export enum ActionTypes { CountStarted = 'compass-aggregations/countStarted', CountFinished = 'compass-aggregations/countFinished', - CountCancelled = 'compass-aggregations/countCancelled', CountFailed = 'compass-aggregations/countFailed', }; @@ -26,10 +25,6 @@ type CountFinishedAction = { count: number; }; -type CountCancelledAction = { - type: ActionTypes.CountCancelled; -}; - type CountFailedAction = { type: ActionTypes.CountFailed; }; @@ -37,7 +32,6 @@ type CountFailedAction = { export type Actions = | CountStartedAction | CountFinishedAction - | CountCancelledAction | CountFailedAction; export type State = { @@ -65,11 +59,6 @@ const reducer: Reducer = (state = INITIAL_STA abortController: undefined, count: action.count, }; - case ActionTypes.CountCancelled: - return { - ...state, - abortController: undefined, - }; case ActionTypes.CountFailed: return { ...state,