From 43fa2261a9ddd7929c98212e73678e86ad345446 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 11 Nov 2024 13:51:56 +0100 Subject: [PATCH 1/7] refactor: remove in-progress states --- .../src/components/create-shard-key-form.tsx | 19 +- .../src/components/index.spec.tsx | 7 - .../src/components/index.tsx | 31 +--- .../states/incomplete-sharding-setup.tsx | 4 +- .../components/states/shard-key-correct.tsx | 4 +- .../components/states/shard-key-mismatch.tsx | 4 +- .../src/components/states/sharding-error.tsx | 12 +- .../src/components/states/sharding.tsx | 8 +- .../src/store/index.spec.ts | 41 ++-- .../src/store/reducer.ts | 175 +++++------------- 10 files changed, 88 insertions(+), 217 deletions(-) diff --git a/packages/compass-global-writes/src/components/create-shard-key-form.tsx b/packages/compass-global-writes/src/components/create-shard-key-form.tsx index 19c801e7b1d..555e849a65d 100644 --- a/packages/compass-global-writes/src/components/create-shard-key-form.tsx +++ b/packages/compass-global-writes/src/components/create-shard-key-form.tsx @@ -21,7 +21,6 @@ import { import { createShardKey, type RootState, - ShardingStatuses, type CreateShardKeyData, } from '../store/reducer'; import { useAutocompleteFields } from '@mongodb-js/compass-field-store'; @@ -317,19 +316,11 @@ export function CreateShardKeyForm({ } export default connect( - (state: RootState) => { - return { - namespace: state.namespace, - isSubmittingForSharding: [ - ShardingStatuses.SUBMITTING_FOR_SHARDING, - ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, - ].includes(state.status), - isCancellingSharding: [ - ShardingStatuses.CANCELLING_SHARDING, - ShardingStatuses.CANCELLING_SHARDING_ERROR, - ].includes(state.status), - }; - }, + (state: RootState) => ({ + namespace: state.namespace, + isSubmittingForSharding: !!state.isSubmittingForSharding, + isCancellingSharding: !!state.isCancellingSharding, + }), { onCreateShardKey: createShardKey, } diff --git a/packages/compass-global-writes/src/components/index.spec.tsx b/packages/compass-global-writes/src/components/index.spec.tsx index 4aff0336345..c5ab3f89791 100644 --- a/packages/compass-global-writes/src/components/index.spec.tsx +++ b/packages/compass-global-writes/src/components/index.spec.tsx @@ -15,13 +15,6 @@ describe('Compass GlobalWrites Plugin', function () { expect(screen.getByTestId('shard-collection-button')).to.exist; }); - it('renders plugin in SUBMITTING_FOR_SHARDING state', async function () { - await renderWithStore( - - ); - expect(screen.getByTestId('shard-collection-button')).to.exist; - }); - it('renders plugin in SHARDING state', async function () { await renderWithStore(); expect(screen.getByText(/sharding your collection/i)).to.exist; diff --git a/packages/compass-global-writes/src/components/index.tsx b/packages/compass-global-writes/src/components/index.tsx index d7331022a17..7ccdf375590 100644 --- a/packages/compass-global-writes/src/components/index.tsx +++ b/packages/compass-global-writes/src/components/index.tsx @@ -44,32 +44,19 @@ function ShardingStateView({ }: { shardingStatus: Exclude; }) { - if ( - shardingStatus === ShardingStatuses.UNSHARDED || - shardingStatus === ShardingStatuses.SUBMITTING_FOR_SHARDING - ) { + if (shardingStatus === ShardingStatuses.UNSHARDED) { return ; } - if ( - shardingStatus === ShardingStatuses.SHARDING || - shardingStatus === ShardingStatuses.CANCELLING_SHARDING - ) { + if (shardingStatus === ShardingStatuses.SHARDING) { return ; } - if ( - shardingStatus === ShardingStatuses.SHARDING_ERROR || - shardingStatus === ShardingStatuses.CANCELLING_SHARDING_ERROR || - shardingStatus === ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR - ) { + if (shardingStatus === ShardingStatuses.SHARDING_ERROR) { return ; } - if ( - shardingStatus === ShardingStatuses.SHARD_KEY_CORRECT || - shardingStatus === ShardingStatuses.UNMANAGING_NAMESPACE - ) { + if (shardingStatus === ShardingStatuses.SHARD_KEY_CORRECT) { return ; } @@ -77,17 +64,11 @@ function ShardingStateView({ return ; } - if ( - shardingStatus === ShardingStatuses.SHARD_KEY_MISMATCH || - shardingStatus === ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH - ) { + if (shardingStatus === ShardingStatuses.SHARD_KEY_MISMATCH) { return ; } - if ( - shardingStatus === ShardingStatuses.INCOMPLETE_SHARDING_SETUP || - shardingStatus === ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE - ) { + if (shardingStatus === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) { return ; } diff --git a/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx b/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx index 8f5b4e0ba4c..bce9e9bdeef 100644 --- a/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx +++ b/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx @@ -12,7 +12,6 @@ import React from 'react'; import ShardKeyMarkup from '../shard-key-markup'; import { resumeManagedNamespace, - ShardingStatuses, type ShardZoneData, type RootState, type ShardKey, @@ -92,8 +91,7 @@ export default connect( namespace: state.namespace, shardKey: state.shardKey, shardZones: state.shardZones, - isSubmittingForSharding: - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE, + isSubmittingForSharding: !!state.isSubmittingForSharding, }; }, { diff --git a/packages/compass-global-writes/src/components/states/shard-key-correct.tsx b/packages/compass-global-writes/src/components/states/shard-key-correct.tsx index 356a24ca4e9..7c7b82fb64e 100644 --- a/packages/compass-global-writes/src/components/states/shard-key-correct.tsx +++ b/packages/compass-global-writes/src/components/states/shard-key-correct.tsx @@ -10,7 +10,6 @@ import { } from '@mongodb-js/compass-components'; import { connect } from 'react-redux'; import { - ShardingStatuses, unmanageNamespace, type RootState, type ShardKey, @@ -86,8 +85,7 @@ export default connect( namespace: state.namespace, shardKey: state.shardKey, shardZones: state.shardZones, - isUnmanagingNamespace: - state.status === ShardingStatuses.UNMANAGING_NAMESPACE, + isUnmanagingNamespace: !!state.isUnmanagingNamespace, }; }, { diff --git a/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx b/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx index 33d3b87be2d..883770d4fb0 100644 --- a/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx +++ b/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx @@ -10,7 +10,6 @@ import { import React from 'react'; import ShardKeyMarkup from '../shard-key-markup'; import { - ShardingStatuses, unmanageNamespace, type RootState, type ShardKey, @@ -104,8 +103,7 @@ export default connect( shardKey: state.shardKey, requestedShardKey: state.managedNamespace && getRequestedShardKey(state.managedNamespace), - isUnmanagingNamespace: - state.status === ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH, + isUnmanagingNamespace: !!state.isUnmanagingNamespace, }; }, { diff --git a/packages/compass-global-writes/src/components/states/sharding-error.tsx b/packages/compass-global-writes/src/components/states/sharding-error.tsx index a71c2d583b6..35607131295 100644 --- a/packages/compass-global-writes/src/components/states/sharding-error.tsx +++ b/packages/compass-global-writes/src/components/states/sharding-error.tsx @@ -8,11 +8,7 @@ import { SpinLoader, } from '@mongodb-js/compass-components'; import { connect } from 'react-redux'; -import { - cancelSharding, - type RootState, - ShardingStatuses, -} from '../../store/reducer'; +import { cancelSharding, type RootState } from '../../store/reducer'; import CreateShardKeyForm from '../create-shard-key-form'; import { containerStyles, bannerStyles } from '../common-styles'; @@ -69,10 +65,8 @@ export default connect( } return { shardingError: state.shardingError, - isCancellingSharding: - state.status === ShardingStatuses.CANCELLING_SHARDING_ERROR, - isSubmittingForSharding: - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, + isCancellingSharding: !!state.isCancellingSharding, + isSubmittingForSharding: !!state.isSubmittingForSharding, }; }, { diff --git a/packages/compass-global-writes/src/components/states/sharding.tsx b/packages/compass-global-writes/src/components/states/sharding.tsx index 914e3197080..3522df6830f 100644 --- a/packages/compass-global-writes/src/components/states/sharding.tsx +++ b/packages/compass-global-writes/src/components/states/sharding.tsx @@ -10,11 +10,7 @@ import { SpinLoader, } from '@mongodb-js/compass-components'; import { connect } from 'react-redux'; -import { - cancelSharding, - type RootState, - ShardingStatuses, -} from '../../store/reducer'; +import { cancelSharding, type RootState } from '../../store/reducer'; import { containerStyles, bannerStyles } from '../common-styles'; const nbsp = '\u00a0'; @@ -65,7 +61,7 @@ export function ShardingState({ export default connect( (state: RootState) => ({ - isCancellingSharding: state.status === ShardingStatuses.CANCELLING_SHARDING, + isCancellingSharding: !!state.isCancellingSharding, }), { onCancelSharding: cancelSharding, diff --git a/packages/compass-global-writes/src/store/index.spec.ts b/packages/compass-global-writes/src/store/index.spec.ts index 19fb3bf21a6..4f932d77989 100644 --- a/packages/compass-global-writes/src/store/index.spec.ts +++ b/packages/compass-global-writes/src/store/index.spec.ts @@ -254,7 +254,7 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING'); + expect(store.getState().isSubmittingForSharding).to.equal(true); mockManagedNamespace = true; await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -264,6 +264,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; }); }); @@ -283,7 +284,7 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING'); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -292,6 +293,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARDING_ERROR'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; expect(store.getState().shardingError).to.equal( `Failed to shard ${NS}` ); // the original error text was: `before timestamp[01:02:03.456]Failed to shard ${NS}` @@ -310,9 +312,10 @@ describe('GlobalWritesStore Store', function () { // user tries to submit for sharding, but the request fails const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING'); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; expect(store.getState().status).to.equal('UNSHARDED'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; }); it('sharding -> valid shard key', async function () { @@ -448,9 +451,7 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); mockManagedNamespace = true; - expect(store.getState().status).to.equal( - 'SUBMITTING_FOR_SHARDING_INCOMPLETE' - ); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; // sharding @@ -460,10 +461,11 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; }); }); - it('incomplete setup -> sharding -> incomplete setup (request was cancelled)', async function () { + it.only('incomplete setup -> sharding -> incomplete setup (request was cancelled)', async function () { // initial state -> incomplete shardingSetup clock = sinon.useFakeTimers({ shouldAdvanceTime: true, @@ -479,9 +481,7 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); - expect(store.getState().status).to.equal( - 'SUBMITTING_FOR_SHARDING_INCOMPLETE' - ); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; // sharding @@ -493,6 +493,8 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().isCancellingSharding).to.be.undefined; }); }); @@ -513,14 +515,13 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); - expect(store.getState().status).to.equal( - 'SUBMITTING_FOR_SHARDING_INCOMPLETE' - ); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; // it failed await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; }); }); @@ -537,9 +538,10 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().status).to.equal('UNMANAGING_NAMESPACE'); + expect(store.getState().isUnmanagingNamespace).to.equal(true); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().isUnmanagingNamespace).to.be.undefined; }); it('valid shard key -> valid shard key (failed unmanage attempt)', async function () { @@ -559,9 +561,10 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage mockFailure = true; const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().status).to.equal('UNMANAGING_NAMESPACE'); + expect(store.getState().isUnmanagingNamespace).to.equal(true); await promise; expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().isUnmanagingNamespace).to.be.undefined; }); context('invalid and mismatching shard keys', function () { @@ -654,11 +657,10 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().status).to.equal( - 'UNMANAGING_NAMESPACE_MISMATCH' - ); + expect(store.getState().isUnmanagingNamespace).to.equal(true); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().isUnmanagingNamespace).to.be.undefined; }); }); @@ -707,7 +709,7 @@ describe('GlobalWritesStore Store', function () { // user submits the form const promise = store.dispatch(createShardKey(shardKeyData)); mockShardingError = false; - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING_ERROR'); + expect(store.getState().isSubmittingForSharding).to.equal(true); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -716,6 +718,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().isSubmittingForSharding).to.be.undefined; }); }); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index aa6904b0eee..02dbe0ed5e9 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -149,26 +149,11 @@ export enum ShardingStatuses { */ INCOMPLETE_SHARDING_SETUP = 'INCOMPLETE_SHARDING_SETUP', - /** - * State when user submits namespace to be sharded and - * we are waiting for server to accept the request. - */ - SUBMITTING_FOR_SHARDING = 'SUBMITTING_FOR_SHARDING', - SUBMITTING_FOR_SHARDING_ERROR = 'SUBMITTING_FOR_SHARDING_ERROR', - SUBMITTING_FOR_SHARDING_INCOMPLETE = 'SUBMITTING_FOR_SHARDING_INCOMPLETE', - /** * Namespace is being sharded. */ SHARDING = 'SHARDING', - /** - * State when user cancels the sharding and - * we are waiting for server to accept the request. - */ - CANCELLING_SHARDING = 'CANCELLING_SHARDING', - CANCELLING_SHARDING_ERROR = 'CANCELLING_SHARDING_ERROR', - /** * Sharding failed. */ @@ -191,12 +176,6 @@ export enum ShardingStatuses { * location key and second key is valid custom key. */ SHARD_KEY_CORRECT = 'SHARD_KEY_CORRECT', - - /** - * Namespace is being unmanaged. - */ - UNMANAGING_NAMESPACE = 'UNMANAGING_NAMESPACE', - UNMANAGING_NAMESPACE_MISMATCH = 'UNMANAGING_NAMESPACE_MISMATCH', } export type ShardingStatus = keyof typeof ShardingStatuses; @@ -220,6 +199,9 @@ export type RootState = { namespace: string; managedNamespace?: ManagedNamespace; shardZones: ShardZoneData[]; + isSubmittingForSharding?: boolean; + isCancellingSharding?: boolean; + isUnmanagingNamespace?: boolean; } & ( | { status: ShardingStatuses.LOADING_ERROR; @@ -236,10 +218,7 @@ export type RootState = { loadingError?: never; } | { - status: - | ShardingStatuses.UNSHARDED - | ShardingStatuses.SUBMITTING_FOR_SHARDING - | ShardingStatuses.CANCELLING_SHARDING; + status: ShardingStatuses.UNSHARDED; shardKey?: ShardKey; // shardKey might exist if the collection was sharded before // and then unmanaged @@ -259,10 +238,7 @@ export type RootState = { loadingError?: never; } | { - status: - | ShardingStatuses.SHARDING_ERROR - | ShardingStatuses.CANCELLING_SHARDING_ERROR - | ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR; + status: ShardingStatuses.SHARDING_ERROR; shardKey?: never; shardingError: string; pollingTimeout?: never; @@ -273,10 +249,7 @@ export type RootState = { | ShardingStatuses.SHARD_KEY_CORRECT | ShardingStatuses.SHARD_KEY_INVALID | ShardingStatuses.SHARD_KEY_MISMATCH - | ShardingStatuses.UNMANAGING_NAMESPACE - | ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH - | ShardingStatuses.INCOMPLETE_SHARDING_SETUP - | ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE; + | ShardingStatuses.INCOMPLETE_SHARDING_SETUP; shardKey: ShardKey; shardingError?: never; pollingTimeout?: never; @@ -396,37 +369,13 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.SubmittingForShardingStarted ) && - state.status === ShardingStatuses.UNSHARDED - ) { - return { - ...state, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.SubmittingForShardingStarted - ) && - state.status === ShardingStatuses.SHARDING_ERROR - ) { - return { - ...state, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.SubmittingForShardingStarted - ) && - state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP + (state.status === ShardingStatuses.UNSHARDED || + state.status === ShardingStatuses.SHARDING_ERROR || + state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) ) { return { ...state, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE, + isSubmittingForSharding: true, }; } @@ -435,13 +384,14 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.SubmittingForShardingFinished ) && - (state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING || - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR || - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE || + (state.status === ShardingStatuses.UNSHARDED || + state.status === ShardingStatuses.SHARDING_ERROR || + state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP || state.status === ShardingStatuses.NOT_READY) ) { return { ...state, + isSubmittingForSharding: undefined, shardingError: undefined, managedNamespace: action.managedNamespace || state.managedNamespace, status: ShardingStatuses.SHARDING, @@ -479,43 +429,32 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.CancellingShardingStarted ) && - state.status === ShardingStatuses.SHARDING + (state.status === ShardingStatuses.SHARDING || + state.status === ShardingStatuses.SHARDING_ERROR || + state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) ) { if (state.pollingTimeout) { throw new Error('Polling was not stopped'); } return { ...state, - status: ShardingStatuses.CANCELLING_SHARDING, + isCancellingSharding: true, pollingTimeout: state.pollingTimeout, }; } - if ( - isAction( - action, - GlobalWritesActionTypes.CancellingShardingStarted - ) && - state.status === ShardingStatuses.SHARDING_ERROR - ) { - return { - ...state, - status: ShardingStatuses.CANCELLING_SHARDING_ERROR, - }; - } - if ( isAction( action, GlobalWritesActionTypes.CancellingShardingErrored ) && - (state.status === ShardingStatuses.CANCELLING_SHARDING || - state.status === ShardingStatuses.CANCELLING_SHARDING_ERROR) + (state.status === ShardingStatuses.SHARDING || + state.status === ShardingStatuses.SHARDING_ERROR || + state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) ) { return { ...state, - shardingError: undefined, - status: ShardingStatuses.SHARDING, + isCancellingSharding: undefined, }; } @@ -524,16 +463,20 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.CancellingShardingFinished ) && - (state.status === ShardingStatuses.CANCELLING_SHARDING || - state.status === ShardingStatuses.SHARDING_ERROR || - state.status === ShardingStatuses.CANCELLING_SHARDING_ERROR) && - // the error might come before the cancel request was processed + (state.status === ShardingStatuses.SHARDING || + state.status === ShardingStatuses.SHARDING_ERROR) && !state.shardKey ) { + if (state.pollingTimeout) { + throw new Error('Polling has not been stopped'); + } return { ...state, - status: ShardingStatuses.UNSHARDED, + managedNamespace: undefined, shardingError: undefined, + isCancellingSharding: undefined, + pollingTimeout: state.pollingTimeout, + status: ShardingStatuses.UNSHARDED, }; } @@ -542,14 +485,16 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.CancellingShardingFinished ) && - state.status === ShardingStatuses.CANCELLING_SHARDING && + state.status === ShardingStatuses.SHARDING && state.shardKey ) { return { ...state, + managedNamespace: undefined, shardKey: state.shardKey, - status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, shardingError: undefined, + isCancellingSharding: undefined, + status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, }; } @@ -558,40 +503,14 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.SubmittingForShardingErrored ) && - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING - ) { - return { - ...state, - managedNamespace: undefined, - status: ShardingStatuses.UNSHARDED, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.SubmittingForShardingErrored - ) && - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR - ) { - return { - ...state, - managedNamespace: undefined, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.SubmittingForShardingErrored - ) && - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE + (state.status === ShardingStatuses.UNSHARDED || + state.status === ShardingStatuses.SHARDING_ERROR || + state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) ) { return { ...state, - managedNamespace: undefined, - status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, + managedNamepsace: undefined, + isSubmittingForSharding: undefined, }; } @@ -605,10 +524,7 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, - status: - state.status === ShardingStatuses.SHARD_KEY_CORRECT - ? ShardingStatuses.UNMANAGING_NAMESPACE - : ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH, + isUnmanagingNamespace: true, }; } @@ -617,13 +533,14 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.UnmanagingNamespaceFinished ) && - (state.status === ShardingStatuses.UNMANAGING_NAMESPACE || - state.status === ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH) + (state.status === ShardingStatuses.SHARD_KEY_CORRECT || + state.status === ShardingStatuses.SHARD_KEY_MISMATCH) ) { return { ...state, managedNamespace: undefined, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, + isUnmanagingNamespace: undefined, }; } @@ -632,11 +549,13 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.UnmanagingNamespaceErrored ) && - state.status === ShardingStatuses.UNMANAGING_NAMESPACE + (state.status === ShardingStatuses.SHARD_KEY_CORRECT || + state.status === ShardingStatuses.SHARD_KEY_MISMATCH) ) { return { ...state, status: ShardingStatuses.SHARD_KEY_CORRECT, + isUnmanagingNamespace: undefined, }; } From a2a85b67d2a687e5a2a2c2e7561286e0d4292cde Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 11 Nov 2024 14:26:34 +0100 Subject: [PATCH 2/7] improve test --- packages/compass-global-writes/src/store/index.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/compass-global-writes/src/store/index.spec.ts b/packages/compass-global-writes/src/store/index.spec.ts index 4f932d77989..f23d3019cf9 100644 --- a/packages/compass-global-writes/src/store/index.spec.ts +++ b/packages/compass-global-writes/src/store/index.spec.ts @@ -236,7 +236,7 @@ describe('GlobalWritesStore Store', function () { }); }); - it('not managed -> sharding -> valid shard key', async function () { + it('not managed -> sharding -> shard key correct', async function () { let mockShardKey = false; let mockManagedNamespace = false; // initial state === unsharded @@ -259,6 +259,10 @@ describe('GlobalWritesStore Store', function () { await promise; expect(store.getState().status).to.equal('SHARDING'); + // polling continues for a while + clock.tick(POLLING_INTERVAL); + clock.tick(POLLING_INTERVAL); + // sharding ends with a shardKey mockShardKey = true; clock.tick(POLLING_INTERVAL); @@ -465,7 +469,7 @@ describe('GlobalWritesStore Store', function () { }); }); - it.only('incomplete setup -> sharding -> incomplete setup (request was cancelled)', async function () { + it('incomplete setup -> sharding -> incomplete setup (request was cancelled)', async function () { // initial state -> incomplete shardingSetup clock = sinon.useFakeTimers({ shouldAdvanceTime: true, From 6047bdcdd3574e08b3d0817ac33ecf1576c53774 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 11 Nov 2024 15:13:42 +0100 Subject: [PATCH 3/7] single userAction --- .../src/components/create-shard-key-form.tsx | 5 +- .../states/incomplete-sharding-setup.tsx | 3 +- .../components/states/shard-key-correct.tsx | 2 +- .../components/states/shard-key-mismatch.tsx | 2 +- .../src/components/states/sharding-error.tsx | 5 +- .../src/components/states/sharding.tsx | 2 +- .../src/store/index.spec.ts | 62 ++++++++---- .../src/store/reducer.ts | 95 ++++++++++++------- 8 files changed, 112 insertions(+), 64 deletions(-) diff --git a/packages/compass-global-writes/src/components/create-shard-key-form.tsx b/packages/compass-global-writes/src/components/create-shard-key-form.tsx index 555e849a65d..a129d4b6ee6 100644 --- a/packages/compass-global-writes/src/components/create-shard-key-form.tsx +++ b/packages/compass-global-writes/src/components/create-shard-key-form.tsx @@ -318,8 +318,9 @@ export function CreateShardKeyForm({ export default connect( (state: RootState) => ({ namespace: state.namespace, - isSubmittingForSharding: !!state.isSubmittingForSharding, - isCancellingSharding: !!state.isCancellingSharding, + isSubmittingForSharding: + !!state.userActionInProgress === 'submitForSharding', + isCancellingSharding: !!state.userActionInProgress === 'cancelSharding', }), { onCreateShardKey: createShardKey, diff --git a/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx b/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx index bce9e9bdeef..a75f2f63c8a 100644 --- a/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx +++ b/packages/compass-global-writes/src/components/states/incomplete-sharding-setup.tsx @@ -91,7 +91,8 @@ export default connect( namespace: state.namespace, shardKey: state.shardKey, shardZones: state.shardZones, - isSubmittingForSharding: !!state.isSubmittingForSharding, + isSubmittingForSharding: + state.userActionInProgress === 'submitForSharding', }; }, { diff --git a/packages/compass-global-writes/src/components/states/shard-key-correct.tsx b/packages/compass-global-writes/src/components/states/shard-key-correct.tsx index 7c7b82fb64e..3608de29d5e 100644 --- a/packages/compass-global-writes/src/components/states/shard-key-correct.tsx +++ b/packages/compass-global-writes/src/components/states/shard-key-correct.tsx @@ -85,7 +85,7 @@ export default connect( namespace: state.namespace, shardKey: state.shardKey, shardZones: state.shardZones, - isUnmanagingNamespace: !!state.isUnmanagingNamespace, + isUnmanagingNamespace: state.userActionInProgress === 'unmanageNamespace', }; }, { diff --git a/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx b/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx index 883770d4fb0..5dbed84469e 100644 --- a/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx +++ b/packages/compass-global-writes/src/components/states/shard-key-mismatch.tsx @@ -103,7 +103,7 @@ export default connect( shardKey: state.shardKey, requestedShardKey: state.managedNamespace && getRequestedShardKey(state.managedNamespace), - isUnmanagingNamespace: !!state.isUnmanagingNamespace, + isUnmanagingNamespace: state.userActionInProgress === 'unmanageNamespace', }; }, { diff --git a/packages/compass-global-writes/src/components/states/sharding-error.tsx b/packages/compass-global-writes/src/components/states/sharding-error.tsx index 35607131295..6c4df570932 100644 --- a/packages/compass-global-writes/src/components/states/sharding-error.tsx +++ b/packages/compass-global-writes/src/components/states/sharding-error.tsx @@ -65,8 +65,9 @@ export default connect( } return { shardingError: state.shardingError, - isCancellingSharding: !!state.isCancellingSharding, - isSubmittingForSharding: !!state.isSubmittingForSharding, + isCancellingSharding: state.userActionInProgress === 'cancelSharding', + isSubmittingForSharding: + state.userActionInProgress === 'submitForSharding', }; }, { diff --git a/packages/compass-global-writes/src/components/states/sharding.tsx b/packages/compass-global-writes/src/components/states/sharding.tsx index 3522df6830f..f30852bb563 100644 --- a/packages/compass-global-writes/src/components/states/sharding.tsx +++ b/packages/compass-global-writes/src/components/states/sharding.tsx @@ -61,7 +61,7 @@ export function ShardingState({ export default connect( (state: RootState) => ({ - isCancellingSharding: !!state.isCancellingSharding, + isCancellingSharding: state.userActionInProgress === 'cancelSharding', }), { onCancelSharding: cancelSharding, diff --git a/packages/compass-global-writes/src/store/index.spec.ts b/packages/compass-global-writes/src/store/index.spec.ts index f23d3019cf9..af3c9b1d8e1 100644 --- a/packages/compass-global-writes/src/store/index.spec.ts +++ b/packages/compass-global-writes/src/store/index.spec.ts @@ -254,7 +254,9 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); mockManagedNamespace = true; await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -268,7 +270,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -288,7 +290,9 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -297,7 +301,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARDING_ERROR'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; expect(store.getState().shardingError).to.equal( `Failed to shard ${NS}` ); // the original error text was: `before timestamp[01:02:03.456]Failed to shard ${NS}` @@ -316,10 +320,12 @@ describe('GlobalWritesStore Store', function () { // user tries to submit for sharding, but the request fails const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('UNSHARDED'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); it('sharding -> valid shard key', async function () { @@ -455,7 +461,9 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); mockManagedNamespace = true; - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; // sharding @@ -465,7 +473,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -485,7 +493,9 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; // sharding @@ -497,8 +507,8 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; - expect(store.getState().isCancellingSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -519,13 +529,15 @@ describe('GlobalWritesStore Store', function () { // user asks to resume geosharding const promise = store.dispatch(resumeManagedNamespace()); - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; // it failed await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -542,10 +554,12 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().isUnmanagingNamespace).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'unmanageNamespace' + ); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); - expect(store.getState().isUnmanagingNamespace).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); it('valid shard key -> valid shard key (failed unmanage attempt)', async function () { @@ -565,10 +579,12 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage mockFailure = true; const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().isUnmanagingNamespace).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'unmanageNamespace' + ); await promise; expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); - expect(store.getState().isUnmanagingNamespace).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); context('invalid and mismatching shard keys', function () { @@ -661,10 +677,12 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().isUnmanagingNamespace).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'unmanageNamespace' + ); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); - expect(store.getState().isUnmanagingNamespace).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -713,7 +731,9 @@ describe('GlobalWritesStore Store', function () { // user submits the form const promise = store.dispatch(createShardKey(shardKeyData)); mockShardingError = false; - expect(store.getState().isSubmittingForSharding).to.equal(true); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -722,7 +742,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); - expect(store.getState().isSubmittingForSharding).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 02dbe0ed5e9..5ce4d38ffa8 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -199,12 +199,14 @@ export type RootState = { namespace: string; managedNamespace?: ManagedNamespace; shardZones: ShardZoneData[]; - isSubmittingForSharding?: boolean; - isCancellingSharding?: boolean; - isUnmanagingNamespace?: boolean; + userActionInProgress?: + | 'submitForSharding' + | 'cancelSharding' + | 'unmanageNamespace'; } & ( | { status: ShardingStatuses.LOADING_ERROR; + userActionInProgress?: never; shardKey?: ShardKey; shardingError?: never; pollingTimeout?: never; @@ -212,6 +214,7 @@ export type RootState = { } | { status: ShardingStatuses.NOT_READY; + userActionInProgress?: never; shardKey?: never; shardingError?: never; pollingTimeout?: never; @@ -219,6 +222,7 @@ export type RootState = { } | { status: ShardingStatuses.UNSHARDED; + userActionInProgress?: 'submitForSharding'; shardKey?: ShardKey; // shardKey might exist if the collection was sharded before // and then unmanaged @@ -228,6 +232,7 @@ export type RootState = { } | { status: ShardingStatuses.SHARDING; + userActionInProgress?: 'cancelSharding'; /** * note: shardKey might exist * if the collection was sharded previously and then unmanaged @@ -239,6 +244,7 @@ export type RootState = { } | { status: ShardingStatuses.SHARDING_ERROR; + userActionInProgress?: 'cancelSharding' | 'submitForSharding'; shardKey?: never; shardingError: string; pollingTimeout?: never; @@ -375,7 +381,7 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, - isSubmittingForSharding: true, + userActionInProgress: 'submitForSharding', }; } @@ -391,7 +397,7 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, - isSubmittingForSharding: undefined, + userActionInProgress: undefined, shardingError: undefined, managedNamespace: action.managedNamespace || state.managedNamespace, status: ShardingStatuses.SHARDING, @@ -438,7 +444,7 @@ const reducer: Reducer = (state = initialState, action) => { } return { ...state, - isCancellingSharding: true, + userActionInProgress: 'cancelSharding', pollingTimeout: state.pollingTimeout, }; } @@ -447,14 +453,15 @@ const reducer: Reducer = (state = initialState, action) => { isAction( action, GlobalWritesActionTypes.CancellingShardingErrored - ) && - (state.status === ShardingStatuses.SHARDING || - state.status === ShardingStatuses.SHARDING_ERROR || - state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) + ) || + isAction( + action, + GlobalWritesActionTypes.UnmanagingNamespaceErrored + ) ) { return { ...state, - isCancellingSharding: undefined, + userActionInProgress: undefined, }; } @@ -472,9 +479,9 @@ const reducer: Reducer = (state = initialState, action) => { } return { ...state, + userActionInProgress: undefined, managedNamespace: undefined, shardingError: undefined, - isCancellingSharding: undefined, pollingTimeout: state.pollingTimeout, status: ShardingStatuses.UNSHARDED, }; @@ -488,12 +495,16 @@ const reducer: Reducer = (state = initialState, action) => { state.status === ShardingStatuses.SHARDING && state.shardKey ) { + if (state.pollingTimeout) { + throw new Error('Polling has not been stopped'); + } return { ...state, + userActionInProgress: undefined, managedNamespace: undefined, shardKey: state.shardKey, shardingError: undefined, - isCancellingSharding: undefined, + pollingTimeout: state.pollingTimeout, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, }; } @@ -510,7 +521,7 @@ const reducer: Reducer = (state = initialState, action) => { return { ...state, managedNamepsace: undefined, - isSubmittingForSharding: undefined, + userActionInProgress: undefined, }; } @@ -524,7 +535,7 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, - isUnmanagingNamespace: true, + userActionInProgress: 'unmanageNamespace', }; } @@ -538,24 +549,9 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, + userActionInProgress: undefined, managedNamespace: undefined, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, - isUnmanagingNamespace: undefined, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.UnmanagingNamespaceErrored - ) && - (state.status === ShardingStatuses.SHARD_KEY_CORRECT || - state.status === ShardingStatuses.SHARD_KEY_MISMATCH) - ) { - return { - ...state, - status: ShardingStatuses.SHARD_KEY_CORRECT, - isUnmanagingNamespace: undefined, }; } @@ -572,6 +568,7 @@ const reducer: Reducer = (state = initialState, action) => { } return { ...state, + userActionInProgress: undefined, status: ShardingStatuses.LOADING_ERROR, loadingError: action.error, pollingTimeout: state.pollingTimeout, @@ -646,7 +643,17 @@ export const createShardKey = ( getState, { atlasGlobalWritesService, logger, connectionInfoRef } ) => { - const { namespace } = getState(); + const { namespace, userActionInProgress } = getState(); + + if (userActionInProgress) { + logger.log.warn( + logger.mongoLogId(1_001_000_337), + 'Global writes duplicate action', + `SubmittingForSharding triggered while another action is in progress - ${userActionInProgress}` + ); + return; + } + dispatch({ type: GlobalWritesActionTypes.SubmittingForShardingStarted, }); @@ -703,7 +710,16 @@ export const cancelSharding = (): GlobalWritesThunkAction< return; } - const { namespace, status } = getState(); + const { namespace, status, userActionInProgress } = getState(); + + if (userActionInProgress) { + logger.log.warn( + logger.mongoLogId(1_001_000_335), + 'Global writes duplicate action', + `CancelSharding triggered while another action is in progress - ${userActionInProgress}` + ); + return; + } if (status === ShardingStatuses.SHARDING) { dispatch(stopPollingForShardKey()); @@ -929,9 +945,18 @@ export const unmanageNamespace = (): GlobalWritesThunkAction< return async ( dispatch, getState, - { atlasGlobalWritesService, connectionInfoRef } + { atlasGlobalWritesService, connectionInfoRef, logger } ) => { - const { namespace } = getState(); + const { namespace, userActionInProgress } = getState(); + + if (userActionInProgress) { + logger.log.warn( + logger.mongoLogId(1_001_000_336), + 'Global writes duplicate action', + `UnmanageNamespace triggered while another action is in progress - ${userActionInProgress}` + ); + return; + } dispatch({ type: GlobalWritesActionTypes.UnmanagingNamespaceStarted, From 747d4b02a40ab0a18a30d097b9cce4d71c491b8c Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 11 Nov 2024 16:54:34 +0100 Subject: [PATCH 4/7] . --- .../src/components/create-shard-key-form.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/compass-global-writes/src/components/create-shard-key-form.tsx b/packages/compass-global-writes/src/components/create-shard-key-form.tsx index a129d4b6ee6..77c47bc68f7 100644 --- a/packages/compass-global-writes/src/components/create-shard-key-form.tsx +++ b/packages/compass-global-writes/src/components/create-shard-key-form.tsx @@ -318,9 +318,8 @@ export function CreateShardKeyForm({ export default connect( (state: RootState) => ({ namespace: state.namespace, - isSubmittingForSharding: - !!state.userActionInProgress === 'submitForSharding', - isCancellingSharding: !!state.userActionInProgress === 'cancelSharding', + isSubmittingForSharding: state.userActionInProgress === 'submitForSharding', + isCancellingSharding: state.userActionInProgress === 'cancelSharding', }), { onCreateShardKey: createShardKey, From 46e6b3d5ab05fb0113104294f9e1e7ea27f81bec Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Tue, 12 Nov 2024 11:14:49 +0100 Subject: [PATCH 5/7] moving pollingTimeout out of the state --- .../src/store/index.spec.ts | 73 +++++++++- .../compass-global-writes/src/store/index.ts | 7 + .../src/store/reducer.ts | 136 ++++-------------- 3 files changed, 99 insertions(+), 117 deletions(-) diff --git a/packages/compass-global-writes/src/store/index.spec.ts b/packages/compass-global-writes/src/store/index.spec.ts index af3c9b1d8e1..4fb17b07b21 100644 --- a/packages/compass-global-writes/src/store/index.spec.ts +++ b/packages/compass-global-writes/src/store/index.spec.ts @@ -67,6 +67,34 @@ function createAuthFetchResponse< }; } +function wait(ms: number) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +const expectPolling = async ({ + spy, + clock, + interval, + reverse, +}: { + spy: Sinon.SinonSpy; + clock: Sinon.SinonFakeTimers; + interval: number; + reverse?: boolean; +}) => { + spy.resetHistory(); + clock.tick(interval); + // leaving time for the poll to actually execute after the clock tick + await wait(1); + if (!reverse) { + expect(spy).to.have.been.called; + } else { + expect(spy).not.to.have.been.called; + } +}; + function createStore({ isNamespaceManaged = () => false, hasShardingError = () => false, @@ -239,9 +267,10 @@ describe('GlobalWritesStore Store', function () { it('not managed -> sharding -> shard key correct', async function () { let mockShardKey = false; let mockManagedNamespace = false; + const hasShardKey = Sinon.fake(() => mockShardKey); // initial state === unsharded const store = createStore({ - hasShardKey: Sinon.fake(() => mockShardKey), + hasShardKey, isNamespaceManaged: Sinon.fake(() => mockManagedNamespace), }); await waitFor(() => { @@ -261,13 +290,26 @@ describe('GlobalWritesStore Store', function () { await promise; expect(store.getState().status).to.equal('SHARDING'); - // polling continues for a while - clock.tick(POLLING_INTERVAL); - clock.tick(POLLING_INTERVAL); + // empty polling for a while + await expectPolling({ + clock, + interval: POLLING_INTERVAL, + spy: hasShardKey, + }); + await expectPolling({ + clock, + interval: POLLING_INTERVAL, + spy: hasShardKey, + }); // sharding ends with a shardKey mockShardKey = true; - clock.tick(POLLING_INTERVAL); + await expectPolling({ + clock, + interval: POLLING_INTERVAL, + spy: hasShardKey, + }); + await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); expect(store.getState().userActionInProgress).to.be.undefined; @@ -400,24 +442,41 @@ describe('GlobalWritesStore Store', function () { it('sharding -> cancelling request -> not managed', async function () { let mockManagedNamespace = true; + const hasShardKey = Sinon.fake(() => false); confirmationStub.resolves(true); // initial state === sharding + clock = sinon.useFakeTimers({ + shouldAdvanceTime: true, + }); const store = createStore({ isNamespaceManaged: Sinon.fake(() => mockManagedNamespace), + hasShardKey, }); await waitFor(() => { expect(store.getState().status).to.equal('SHARDING'); - expect(store.getState().pollingTimeout).not.to.be.undefined; expect(store.getState().managedNamespace).to.equal(managedNamespace); }); + await expectPolling({ + clock, + interval: POLLING_INTERVAL, + spy: hasShardKey, + }); + // user cancels the sharding request const promise = store.dispatch(cancelSharding()); mockManagedNamespace = false; await promise; expect(store.getState().status).to.equal('UNSHARDED'); - expect(store.getState().pollingTimeout).to.be.undefined; expect(confirmationStub).to.have.been.called; + + // is no longer polling + await expectPolling({ + reverse: true, + clock, + interval: POLLING_INTERVAL, + spy: hasShardKey, + }); }); it('valid shard key', async function () { diff --git a/packages/compass-global-writes/src/store/index.ts b/packages/compass-global-writes/src/store/index.ts index b00fad27e27..c37217bc185 100644 --- a/packages/compass-global-writes/src/store/index.ts +++ b/packages/compass-global-writes/src/store/index.ts @@ -20,6 +20,9 @@ type GlobalWritesExtraArgs = { track: TrackFunction; connectionInfoRef: ConnectionInfoRef; atlasGlobalWritesService: AtlasGlobalWritesService; + pollingTimeoutRef: { + current: ReturnType | null; + }; }; export type GlobalWritesThunkAction = ThunkAction< @@ -60,6 +63,9 @@ export function activateGlobalWritesPlugin( atlasService, connectionInfoRef ); + const pollingTimeoutRef = { + current: null, + }; const store: GlobalWritesStore = createStore( reducer, { @@ -73,6 +79,7 @@ export function activateGlobalWritesPlugin( track, connectionInfoRef, atlasGlobalWritesService, + pollingTimeoutRef, }) ) ); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 5ce4d38ffa8..6e32e19b180 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -40,9 +40,6 @@ enum GlobalWritesActionTypes { CancellingShardingFinished = 'global-writes/CancellingShardingFinished', CancellingShardingErrored = 'global-writes/CancellingShardingErrored', - NextPollingTimeoutSet = 'global-writes/NextPollingTimeoutSet', - NextPollingTimeoutCleared = 'global-writes/NextPollingTimeoutCleared', - UnmanagingNamespaceStarted = 'global-writes/UnmanagingNamespaceStarted', UnmanagingNamespaceFinished = 'global-writes/UnmanagingNamespaceFinished', UnmanagingNamespaceErrored = 'global-writes/UnmanagingNamespaceErrored', @@ -105,15 +102,6 @@ type CancellingShardingErroredAction = { type: GlobalWritesActionTypes.CancellingShardingErrored; }; -type NextPollingTimeoutSetAction = { - type: GlobalWritesActionTypes.NextPollingTimeoutSet; - timeout: NodeJS.Timeout; -}; - -type NextPollingTimeoutClearedAction = { - type: GlobalWritesActionTypes.NextPollingTimeoutCleared; -}; - type UnmanagingNamespaceStartedAction = { type: GlobalWritesActionTypes.UnmanagingNamespaceStarted; }; @@ -209,7 +197,6 @@ export type RootState = { userActionInProgress?: never; shardKey?: ShardKey; shardingError?: never; - pollingTimeout?: never; loadingError: string; } | { @@ -217,7 +204,6 @@ export type RootState = { userActionInProgress?: never; shardKey?: never; shardingError?: never; - pollingTimeout?: never; loadingError?: never; } | { @@ -227,7 +213,6 @@ export type RootState = { // shardKey might exist if the collection was sharded before // and then unmanaged shardingError?: never; - pollingTimeout?: never; loadingError?: never; } | { @@ -239,7 +224,6 @@ export type RootState = { */ shardKey?: ShardKey; shardingError?: never; - pollingTimeout?: NodeJS.Timeout; loadingError?: never; } | { @@ -247,7 +231,6 @@ export type RootState = { userActionInProgress?: 'cancelSharding' | 'submitForSharding'; shardKey?: never; shardingError: string; - pollingTimeout?: never; loadingError?: never; } | { @@ -258,7 +241,6 @@ export type RootState = { | ShardingStatuses.INCOMPLETE_SHARDING_SETUP; shardKey: ShardKey; shardingError?: never; - pollingTimeout?: never; loadingError?: never; } ); @@ -291,15 +273,11 @@ const reducer: Reducer = (state = initialState, action) => { (state.status === ShardingStatuses.NOT_READY || state.status === ShardingStatuses.SHARDING) ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } return { ...state, status: ShardingStatuses.SHARDING_ERROR, shardKey: undefined, shardingError: action.error, - pollingTimeout: state.pollingTimeout, }; } @@ -312,9 +290,6 @@ const reducer: Reducer = (state = initialState, action) => { state.status === ShardingStatuses.SHARDING) && action.shardKey ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } return { ...state, status: getStatusFromShardKeyAndManaged( @@ -323,7 +298,6 @@ const reducer: Reducer = (state = initialState, action) => { ), shardKey: action.shardKey, shardingError: undefined, - pollingTimeout: state.pollingTimeout, }; } @@ -336,13 +310,9 @@ const reducer: Reducer = (state = initialState, action) => { !action.shardKey && !state.managedNamespace ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } return { ...state, status: ShardingStatuses.UNSHARDED, - pollingTimeout: state.pollingTimeout, }; } @@ -404,32 +374,6 @@ const reducer: Reducer = (state = initialState, action) => { }; } - if ( - isAction( - action, - GlobalWritesActionTypes.NextPollingTimeoutSet - ) && - state.status === ShardingStatuses.SHARDING - ) { - return { - ...state, - pollingTimeout: action.timeout, - }; - } - - if ( - isAction( - action, - GlobalWritesActionTypes.NextPollingTimeoutCleared - ) && - state.status === ShardingStatuses.SHARDING - ) { - return { - ...state, - pollingTimeout: undefined, - }; - } - if ( isAction( action, @@ -439,13 +383,9 @@ const reducer: Reducer = (state = initialState, action) => { state.status === ShardingStatuses.SHARDING_ERROR || state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } return { ...state, userActionInProgress: 'cancelSharding', - pollingTimeout: state.pollingTimeout, }; } @@ -474,15 +414,11 @@ const reducer: Reducer = (state = initialState, action) => { state.status === ShardingStatuses.SHARDING_ERROR) && !state.shardKey ) { - if (state.pollingTimeout) { - throw new Error('Polling has not been stopped'); - } return { ...state, userActionInProgress: undefined, managedNamespace: undefined, shardingError: undefined, - pollingTimeout: state.pollingTimeout, status: ShardingStatuses.UNSHARDED, }; } @@ -495,16 +431,12 @@ const reducer: Reducer = (state = initialState, action) => { state.status === ShardingStatuses.SHARDING && state.shardKey ) { - if (state.pollingTimeout) { - throw new Error('Polling has not been stopped'); - } return { ...state, userActionInProgress: undefined, managedNamespace: undefined, shardKey: state.shardKey, shardingError: undefined, - pollingTimeout: state.pollingTimeout, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, }; } @@ -563,15 +495,11 @@ const reducer: Reducer = (state = initialState, action) => { (state.status === ShardingStatuses.NOT_READY || state.status === ShardingStatuses.SHARDING) ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } return { ...state, userActionInProgress: undefined, status: ShardingStatuses.LOADING_ERROR, loadingError: action.error, - pollingTimeout: state.pollingTimeout, }; } @@ -700,7 +628,11 @@ export const cancelSharding = (): GlobalWritesThunkAction< | CancellingShardingFinishedAction | CancellingShardingErroredAction > => { - return async (dispatch, getState, { atlasGlobalWritesService, logger }) => { + return async ( + dispatch, + getState, + { atlasGlobalWritesService, logger, pollingTimeoutRef } + ) => { const confirmed = await showConfirmation({ title: 'Confirmation', description: 'Are you sure you want to cancel the sharding request?', @@ -722,7 +654,7 @@ export const cancelSharding = (): GlobalWritesThunkAction< } if (status === ShardingStatuses.SHARDING) { - dispatch(stopPollingForShardKey()); + stopPollingForShardKey(pollingTimeoutRef); } dispatch({ type: GlobalWritesActionTypes.CancellingShardingStarted, @@ -771,36 +703,25 @@ const setNamespaceBeingSharded = ( const pollForShardKey = (): GlobalWritesThunkAction< void, - NextPollingTimeoutSetAction + FetchNamespaceShardKeyActions > => { - return (dispatch, getState) => { - const { pollingTimeout } = getState(); + return (dispatch, getState, { pollingTimeoutRef }) => { if ( - pollingTimeout // prevent double polling + pollingTimeoutRef.current // prevent double polling ) { return; } - const timeout = setTimeout(() => { + pollingTimeoutRef.current = setTimeout(() => { void dispatch(fetchNamespaceShardKey()); }, POLLING_INTERVAL); - - dispatch({ - type: GlobalWritesActionTypes.NextPollingTimeoutSet, - timeout, - }); }; }; -const stopPollingForShardKey = (): GlobalWritesThunkAction< - void, - NextPollingTimeoutClearedAction -> => { - return (dispatch, getState) => { - const { pollingTimeout } = getState(); - if (!pollingTimeout) return; - clearTimeout(pollingTimeout); - dispatch({ type: GlobalWritesActionTypes.NextPollingTimeoutCleared }); - }; +const stopPollingForShardKey = (pollingTimeoutRef: { + current: ReturnType | null; +}) => { + if (!pollingTimeoutRef.current) return; + clearTimeout(pollingTimeoutRef.current); }; const handleLoadingError = ({ @@ -833,19 +754,21 @@ const handleLoadingError = ({ }; }; +type FetchNamespaceShardKeyActions = + | NamespaceShardingErrorFetchedAction + | NamespaceShardKeyFetchedAction; + export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< Promise, - | NamespaceShardingErrorFetchedAction - | NamespaceShardKeyFetchedAction - | NextPollingTimeoutClearedAction + FetchNamespaceShardKeyActions > => { return async ( dispatch, getState, - { atlasGlobalWritesService, logger, connectionInfoRef } + { atlasGlobalWritesService, logger, connectionInfoRef, pollingTimeoutRef } ) => { - dispatch({ type: GlobalWritesActionTypes.NextPollingTimeoutCleared }); - const { namespace, status, managedNamespace } = getState(); + pollingTimeoutRef.current = null; + const { namespace, managedNamespace } = getState(); try { const [shardingError, shardKey] = await Promise.all([ @@ -853,20 +776,13 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< atlasGlobalWritesService.getShardingKeys(namespace), ]); - if (status === ShardingStatuses.SHARDING && (shardKey || shardingError)) { - dispatch(stopPollingForShardKey()); - } - if (managedNamespace && !shardKey) { - if (!shardingError) { - dispatch(setNamespaceBeingSharded()); - return; - } // if there is an existing shard key and an error both, // means we have a key mismatch // this will be handled in NamespaceShardKeyFetched - if (status === ShardingStatuses.SHARDING) { - dispatch(stopPollingForShardKey()); + if (!shardingError) { + dispatch(setNamespaceBeingSharded()); + return; } dispatch({ type: GlobalWritesActionTypes.NamespaceShardingErrorFetched, From 01bf7f9b8bcf53208a79d87487a915075974fbc3 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Tue, 12 Nov 2024 14:44:22 +0100 Subject: [PATCH 6/7] more cleanup --- .../src/store/reducer.ts | 103 ++++++++++-------- 1 file changed, 55 insertions(+), 48 deletions(-) diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 6e32e19b180..9d6fadbfc6b 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -187,20 +187,18 @@ export type RootState = { namespace: string; managedNamespace?: ManagedNamespace; shardZones: ShardZoneData[]; - userActionInProgress?: - | 'submitForSharding' - | 'cancelSharding' - | 'unmanageNamespace'; } & ( | { status: ShardingStatuses.LOADING_ERROR; - userActionInProgress?: never; shardKey?: ShardKey; - shardingError?: never; loadingError: string; + ////////////// + userActionInProgress?: never; + shardingError?: never; } | { status: ShardingStatuses.NOT_READY; + ////////////// userActionInProgress?: never; shardKey?: never; shardingError?: never; @@ -209,9 +207,8 @@ export type RootState = { | { status: ShardingStatuses.UNSHARDED; userActionInProgress?: 'submitForSharding'; - shardKey?: ShardKey; - // shardKey might exist if the collection was sharded before - // and then unmanaged + ////////////// + shardKey?: never; shardingError?: never; loadingError?: never; } @@ -223,23 +220,41 @@ export type RootState = { * if the collection was sharded previously and then unmanaged */ shardKey?: ShardKey; + ////////////// shardingError?: never; loadingError?: never; } | { status: ShardingStatuses.SHARDING_ERROR; userActionInProgress?: 'cancelSharding' | 'submitForSharding'; - shardKey?: never; shardingError: string; + ////////////// + shardKey?: never; loadingError?: never; } | { status: | ShardingStatuses.SHARD_KEY_CORRECT - | ShardingStatuses.SHARD_KEY_INVALID - | ShardingStatuses.SHARD_KEY_MISMATCH - | ShardingStatuses.INCOMPLETE_SHARDING_SETUP; + | ShardingStatuses.SHARD_KEY_MISMATCH; + userActionInProgress?: 'unmanageNamespace'; + shardKey: ShardKey; + ////////////// + shardingError?: never; + loadingError?: never; + } + | { + status: ShardingStatuses.SHARD_KEY_INVALID; + shardKey: ShardKey; + ////////////// + userActionInProgress?: never; + shardingError?: never; + loadingError?: never; + } + | { + status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP; + userActionInProgress?: 'cancelSharding' | 'submitForSharding'; shardKey: ShardKey; + ////////////// shardingError?: never; loadingError?: never; } @@ -296,6 +311,7 @@ const reducer: Reducer = (state = initialState, action) => { action.shardKey, state.managedNamespace ), + userActionInProgress: undefined, shardKey: action.shardKey, shardingError: undefined, }; @@ -340,6 +356,26 @@ const reducer: Reducer = (state = initialState, action) => { }; } + if ( + isAction( + action, + GlobalWritesActionTypes.CancellingShardingErrored + ) || + isAction( + action, + GlobalWritesActionTypes.UnmanagingNamespaceErrored + ) || + isAction( + action, + GlobalWritesActionTypes.SubmittingForShardingErrored + ) + ) { + return { + ...state, + userActionInProgress: undefined, + }; + } + if ( isAction( action, @@ -389,22 +425,6 @@ const reducer: Reducer = (state = initialState, action) => { }; } - if ( - isAction( - action, - GlobalWritesActionTypes.CancellingShardingErrored - ) || - isAction( - action, - GlobalWritesActionTypes.UnmanagingNamespaceErrored - ) - ) { - return { - ...state, - userActionInProgress: undefined, - }; - } - if ( isAction( action, @@ -418,6 +438,7 @@ const reducer: Reducer = (state = initialState, action) => { ...state, userActionInProgress: undefined, managedNamespace: undefined, + shardKey: state.shardKey, shardingError: undefined, status: ShardingStatuses.UNSHARDED, }; @@ -441,22 +462,6 @@ const reducer: Reducer = (state = initialState, action) => { }; } - if ( - isAction( - action, - GlobalWritesActionTypes.SubmittingForShardingErrored - ) && - (state.status === ShardingStatuses.UNSHARDED || - state.status === ShardingStatuses.SHARDING_ERROR || - state.status === ShardingStatuses.INCOMPLETE_SHARDING_SETUP) - ) { - return { - ...state, - managedNamepsace: undefined, - userActionInProgress: undefined, - }; - } - if ( isAction( action, @@ -777,13 +782,15 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< ]); if (managedNamespace && !shardKey) { - // if there is an existing shard key and an error both, - // means we have a key mismatch - // this will be handled in NamespaceShardKeyFetched if (!shardingError) { + // there is neither a shardKey nor shardingError + // means sharding is in progress dispatch(setNamespaceBeingSharded()); return; } + // if there is an existing shard key and an error both, + // means we have a key mismatch + // this will be handled in NamespaceShardKeyFetched dispatch({ type: GlobalWritesActionTypes.NamespaceShardingErrorFetched, error: shardingError, From becce3d9e1e980063c80d821d185acd951c7b0f6 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 14 Nov 2024 15:49:58 +0100 Subject: [PATCH 7/7] updated comment --- packages/compass-global-writes/src/store/reducer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 9d6fadbfc6b..8cdbece05b3 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -624,7 +624,7 @@ export const createShardKey = ( }; // Exporting this for test only to stub it and set -// its value. This enables to test cancelSharding action. +// its value. This enables to test cancelShardingaction. export const showConfirmation = showConfirmationModal; export const cancelSharding = (): GlobalWritesThunkAction< @@ -783,8 +783,8 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< if (managedNamespace && !shardKey) { if (!shardingError) { - // there is neither a shardKey nor shardingError - // means sharding is in progress + // Since the namespace is managed, Atlas has been instructed to shard this collection, + // and since there is no shard key and no sharding error, the shard must still be in progress dispatch(setNamespaceBeingSharded()); return; }