From 203ce42a04e4f310cdaa4cf1bad564506b8e93df Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Fri, 15 Nov 2024 16:38:06 +0100 Subject: [PATCH] refactor(compass-global-writes): remove in-progress states (#6475) --- .../src/components/create-shard-key-form.tsx | 19 +- .../src/components/index.spec.tsx | 7 - .../src/components/index.tsx | 31 +- .../states/incomplete-sharding-setup.tsx | 3 +- .../components/states/shard-key-correct.tsx | 4 +- .../components/states/shard-key-mismatch.tsx | 4 +- .../src/components/states/sharding-error.tsx | 11 +- .../src/components/states/sharding.tsx | 8 +- .../src/store/index.spec.ts | 124 +++++- .../compass-global-writes/src/store/index.ts | 7 + .../src/store/reducer.ts | 393 ++++++------------ 11 files changed, 261 insertions(+), 350 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 34fe299ad69..98e504e6d8e 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'; @@ -320,19 +319,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.userActionInProgress === 'submitForSharding', + isCancellingSharding: state.userActionInProgress === 'cancelSharding', + }), { 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 6f4f3f55778..87396d02b50 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 3ead0059e21..e45e0ac135d 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, @@ -90,7 +89,7 @@ export default connect( shardKey: state.shardKey, shardZones: state.shardZones, isSubmittingForSharding: - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_INCOMPLETE, + 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 356a24ca4e9..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 @@ -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.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 bad54b20256..5f6cbde240d 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 @@ -8,7 +8,6 @@ import { import React from 'react'; import ShardKeyMarkup from '../shard-key-markup'; import { - ShardingStatuses, unmanageNamespace, type RootState, type ShardKey, @@ -102,8 +101,7 @@ export default connect( shardKey: state.shardKey, requestedShardKey: state.managedNamespace && getRequestedShardKey(state.managedNamespace), - isUnmanagingNamespace: - state.status === ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH, + 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 7988299616e..ead1e46c98f 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, @@ -68,10 +64,9 @@ export default connect( } return { shardingError: state.shardingError, - isCancellingSharding: - state.status === ShardingStatuses.CANCELLING_SHARDING_ERROR, + isCancellingSharding: state.userActionInProgress === 'cancelSharding', isSubmittingForSharding: - state.status === ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, + 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 0eebdf0f12e..a07a297bf10 100644 --- a/packages/compass-global-writes/src/components/states/sharding.tsx +++ b/packages/compass-global-writes/src/components/states/sharding.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 { containerStyles, bannerStyles, @@ -62,7 +58,7 @@ export function ShardingState({ export default connect( (state: RootState) => ({ - isCancellingSharding: state.status === ShardingStatuses.CANCELLING_SHARDING, + 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 19fb3bf21a6..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, @@ -236,12 +264,13 @@ 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; + const hasShardKey = Sinon.fake(() => mockShardKey); // initial state === unsharded const store = createStore({ - hasShardKey: Sinon.fake(() => mockShardKey), + hasShardKey, isNamespaceManaged: Sinon.fake(() => mockManagedNamespace), }); await waitFor(() => { @@ -254,16 +283,36 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING'); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); mockManagedNamespace = true; await promise; expect(store.getState().status).to.equal('SHARDING'); + // 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; }); }); @@ -283,7 +332,9 @@ describe('GlobalWritesStore Store', function () { shouldAdvanceTime: true, }); const promise = store.dispatch(createShardKey(shardKeyData)); - expect(store.getState().status).to.equal('SUBMITTING_FOR_SHARDING'); + expect(store.getState().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -292,6 +343,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARDING_ERROR'); + 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}` @@ -310,9 +362,12 @@ 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().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('UNSHARDED'); + expect(store.getState().userActionInProgress).to.be.undefined; }); it('sharding -> valid shard key', async function () { @@ -387,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 () { @@ -448,8 +520,8 @@ 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().userActionInProgress).to.equal( + 'submitForSharding' ); await promise; @@ -460,6 +532,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -479,8 +552,8 @@ 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().userActionInProgress).to.equal( + 'submitForSharding' ); await promise; @@ -493,6 +566,8 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().userActionInProgress).to.be.undefined; + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -513,14 +588,15 @@ 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().userActionInProgress).to.equal( + 'submitForSharding' ); await promise; // it failed await waitFor(() => { expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -537,9 +613,12 @@ describe('GlobalWritesStore Store', function () { // user asks to unmanage const promise = store.dispatch(unmanageNamespace()); - expect(store.getState().status).to.equal('UNMANAGING_NAMESPACE'); + expect(store.getState().userActionInProgress).to.equal( + 'unmanageNamespace' + ); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().userActionInProgress).to.be.undefined; }); it('valid shard key -> valid shard key (failed unmanage attempt)', async function () { @@ -559,9 +638,12 @@ 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().userActionInProgress).to.equal( + 'unmanageNamespace' + ); await promise; expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().userActionInProgress).to.be.undefined; }); context('invalid and mismatching shard keys', function () { @@ -654,11 +736,12 @@ 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().userActionInProgress).to.equal( + 'unmanageNamespace' ); await promise; expect(store.getState().status).to.equal('INCOMPLETE_SHARDING_SETUP'); + expect(store.getState().userActionInProgress).to.be.undefined; }); }); @@ -707,7 +790,9 @@ 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().userActionInProgress).to.equal( + 'submitForSharding' + ); await promise; expect(store.getState().status).to.equal('SHARDING'); @@ -716,6 +801,7 @@ describe('GlobalWritesStore Store', function () { clock.tick(POLLING_INTERVAL); await waitFor(() => { expect(store.getState().status).to.equal('SHARD_KEY_CORRECT'); + expect(store.getState().userActionInProgress).to.be.undefined; }); }); 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 aa6904b0eee..8cdbece05b3 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; }; @@ -149,26 +137,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 +164,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; @@ -224,62 +191,71 @@ export type RootState = { | { status: ShardingStatuses.LOADING_ERROR; shardKey?: ShardKey; - shardingError?: never; - pollingTimeout?: never; loadingError: string; + ////////////// + userActionInProgress?: never; + shardingError?: never; } | { status: ShardingStatuses.NOT_READY; + ////////////// + userActionInProgress?: never; shardKey?: never; shardingError?: never; - pollingTimeout?: never; loadingError?: never; } | { - status: - | ShardingStatuses.UNSHARDED - | ShardingStatuses.SUBMITTING_FOR_SHARDING - | ShardingStatuses.CANCELLING_SHARDING; - shardKey?: ShardKey; - // shardKey might exist if the collection was sharded before - // and then unmanaged + status: ShardingStatuses.UNSHARDED; + userActionInProgress?: 'submitForSharding'; + ////////////// + shardKey?: never; shardingError?: never; - pollingTimeout?: never; loadingError?: never; } | { status: ShardingStatuses.SHARDING; + userActionInProgress?: 'cancelSharding'; /** * note: shardKey might exist * if the collection was sharded previously and then unmanaged */ shardKey?: ShardKey; + ////////////// shardingError?: never; - pollingTimeout?: NodeJS.Timeout; loadingError?: never; } | { - status: - | ShardingStatuses.SHARDING_ERROR - | ShardingStatuses.CANCELLING_SHARDING_ERROR - | ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR; - shardKey?: never; + status: ShardingStatuses.SHARDING_ERROR; + userActionInProgress?: 'cancelSharding' | 'submitForSharding'; shardingError: string; - pollingTimeout?: never; + ////////////// + shardKey?: never; loadingError?: never; } | { status: | 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.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; - pollingTimeout?: never; loadingError?: never; } ); @@ -312,15 +288,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, }; } @@ -333,18 +305,15 @@ 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( action.shardKey, state.managedNamespace ), + userActionInProgress: undefined, shardKey: action.shardKey, shardingError: undefined, - pollingTimeout: state.pollingTimeout, }; } @@ -357,13 +326,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, }; } @@ -392,28 +357,22 @@ const reducer: Reducer = (state = initialState, action) => { } if ( - isAction( + isAction( action, - GlobalWritesActionTypes.SubmittingForShardingStarted - ) && - state.status === ShardingStatuses.UNSHARDED - ) { - return { - ...state, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING, - }; - } - - if ( - isAction( + GlobalWritesActionTypes.CancellingShardingErrored + ) || + isAction( action, - GlobalWritesActionTypes.SubmittingForShardingStarted - ) && - state.status === ShardingStatuses.SHARDING_ERROR + GlobalWritesActionTypes.UnmanagingNamespaceErrored + ) || + isAction( + action, + GlobalWritesActionTypes.SubmittingForShardingErrored + ) ) { return { ...state, - status: ShardingStatuses.SUBMITTING_FOR_SHARDING_ERROR, + userActionInProgress: undefined, }; } @@ -422,11 +381,13 @@ const reducer: Reducer = (state = initialState, action) => { 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, + userActionInProgress: 'submitForSharding', }; } @@ -435,87 +396,32 @@ 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, + userActionInProgress: undefined, shardingError: undefined, managedNamespace: action.managedNamespace || state.managedNamespace, status: ShardingStatuses.SHARDING, }; } - 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, - GlobalWritesActionTypes.CancellingShardingStarted - ) && - state.status === ShardingStatuses.SHARDING - ) { - if (state.pollingTimeout) { - throw new Error('Polling was not stopped'); - } - return { - ...state, - status: ShardingStatuses.CANCELLING_SHARDING, - 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, + userActionInProgress: 'cancelSharding', }; } @@ -524,16 +430,17 @@ 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 ) { return { ...state, - status: ShardingStatuses.UNSHARDED, + userActionInProgress: undefined, + managedNamespace: undefined, + shardKey: state.shardKey, shardingError: undefined, + status: ShardingStatuses.UNSHARDED, }; } @@ -542,55 +449,15 @@ const reducer: Reducer = (state = initialState, action) => { action, GlobalWritesActionTypes.CancellingShardingFinished ) && - state.status === ShardingStatuses.CANCELLING_SHARDING && + state.status === ShardingStatuses.SHARDING && state.shardKey ) { return { ...state, + userActionInProgress: undefined, + managedNamespace: undefined, shardKey: state.shardKey, - status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, shardingError: undefined, - }; - } - - if ( - isAction( - 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 - ) { - return { - ...state, - managedNamespace: undefined, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, }; } @@ -605,10 +472,7 @@ const reducer: Reducer = (state = initialState, action) => { ) { return { ...state, - status: - state.status === ShardingStatuses.SHARD_KEY_CORRECT - ? ShardingStatuses.UNMANAGING_NAMESPACE - : ShardingStatuses.UNMANAGING_NAMESPACE_MISMATCH, + userActionInProgress: 'unmanageNamespace', }; } @@ -617,29 +481,17 @@ 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, + userActionInProgress: undefined, managedNamespace: undefined, status: ShardingStatuses.INCOMPLETE_SHARDING_SETUP, }; } - if ( - isAction( - action, - GlobalWritesActionTypes.UnmanagingNamespaceErrored - ) && - state.status === ShardingStatuses.UNMANAGING_NAMESPACE - ) { - return { - ...state, - status: ShardingStatuses.SHARD_KEY_CORRECT, - }; - } - if ( isAction( action, @@ -648,14 +500,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, }; } @@ -727,7 +576,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, }); @@ -765,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< @@ -774,7 +633,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?', @@ -784,10 +647,19 @@ 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()); + stopPollingForShardKey(pollingTimeoutRef); } dispatch({ type: GlobalWritesActionTypes.CancellingShardingStarted, @@ -836,36 +708,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 = ({ @@ -898,19 +759,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([ @@ -918,21 +781,16 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< atlasGlobalWritesService.getShardingKeys(namespace), ]); - if (status === ShardingStatuses.SHARDING && (shardKey || shardingError)) { - dispatch(stopPollingForShardKey()); - } - if (managedNamespace && !shardKey) { if (!shardingError) { + // 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; } // 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()); - } dispatch({ type: GlobalWritesActionTypes.NamespaceShardingErrorFetched, error: shardingError, @@ -1010,9 +868,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,