Skip to content

Commit 85413f2

Browse files
authored
fix: fix crossfilter persisting after removal (apache#35998)
1 parent 9605a4a commit 85413f2

File tree

2 files changed

+75
-1
lines changed

2 files changed

+75
-1
lines changed

superset-frontend/src/components/Chart/chartAction.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,10 +407,12 @@ export function exploreJSON(
407407
ownState,
408408
) {
409409
return async (dispatch, getState) => {
410+
const state = getState();
410411
const logStart = Logger.getTimestamp();
411412
const controller = new AbortController();
413+
const prevController = state.charts?.[key]?.queryController;
412414
const queryTimeout =
413-
timeout || getState().common.conf.SUPERSET_WEBSERVER_TIMEOUT;
415+
timeout || state.common.conf.SUPERSET_WEBSERVER_TIMEOUT;
414416

415417
const requestParams = {
416418
signal: controller.signal,
@@ -422,6 +424,14 @@ export function exploreJSON(
422424
dispatch(updateDataMask(formData.slice_id, dataMask));
423425
};
424426
dispatch(chartUpdateStarted(controller, formData, key));
427+
/**
428+
* Abort in-flight requests after the new controller has been stored in
429+
* state. Delaying ensures we do not mutate the Redux state between
430+
* dispatches while still cancelling the previous request promptly.
431+
*/
432+
if (prevController) {
433+
setTimeout(() => prevController.abort(), 0);
434+
}
425435

426436
const chartDataRequest = getChartDataRequest({
427437
setDataMask,

superset-frontend/src/components/Chart/chartActions.test.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import * as exploreUtils from 'src/explore/exploreUtils';
3131
import * as actions from 'src/components/Chart/chartAction';
3232
import * as asyncEvent from 'src/middleware/asyncEvent';
3333
import { handleChartDataResponse } from 'src/components/Chart/chartAction';
34+
import * as dataMaskActions from 'src/dataMask/actions';
3435

3536
import configureMockStore from 'redux-mock-store';
3637
import thunk from 'redux-thunk';
@@ -110,6 +111,69 @@ describe('chart actions', () => {
110111
.callsFake(data => Promise.resolve(data));
111112
});
112113

114+
test('should defer abort of previous controller to avoid Redux state mutation', async () => {
115+
jest.useFakeTimers();
116+
const chartKey = 'defer_abort_test';
117+
const formData = {
118+
slice_id: 123,
119+
datasource: 'table__1',
120+
viz_type: 'table',
121+
};
122+
const oldController = new AbortController();
123+
const abortSpy = jest.spyOn(oldController, 'abort');
124+
const state = {
125+
charts: {
126+
[chartKey]: {
127+
queryController: oldController,
128+
},
129+
},
130+
common: {
131+
conf: {
132+
SUPERSET_WEBSERVER_TIMEOUT: 60,
133+
},
134+
},
135+
};
136+
const getState = jest.fn(() => state);
137+
const dispatchMock = jest.fn();
138+
const getChartDataRequestSpy = jest
139+
.spyOn(actions, 'getChartDataRequest')
140+
.mockResolvedValue({
141+
response: { status: 200 },
142+
json: { result: [] },
143+
});
144+
const handleChartDataResponseSpy = jest
145+
.spyOn(actions, 'handleChartDataResponse')
146+
.mockResolvedValue([]);
147+
const updateDataMaskSpy = jest
148+
.spyOn(dataMaskActions, 'updateDataMask')
149+
.mockReturnValue({ type: 'UPDATE_DATA_MASK' });
150+
const getQuerySettingsStub = sinon
151+
.stub(exploreUtils, 'getQuerySettings')
152+
.returns([false, () => {}]);
153+
154+
try {
155+
const thunk = actions.exploreJSON(formData, false, undefined, chartKey);
156+
const promise = thunk(dispatchMock, getState);
157+
158+
expect(abortSpy).not.toHaveBeenCalled();
159+
expect(oldController.signal.aborted).toBe(false);
160+
161+
jest.runOnlyPendingTimers();
162+
163+
expect(abortSpy).toHaveBeenCalledTimes(1);
164+
expect(oldController.signal.aborted).toBe(true);
165+
166+
await promise;
167+
} finally {
168+
getChartDataRequestSpy.mockRestore();
169+
handleChartDataResponseSpy.mockRestore();
170+
updateDataMaskSpy.mockRestore();
171+
getQuerySettingsStub.restore();
172+
abortSpy.mockRestore();
173+
jest.useRealTimers();
174+
}
175+
});
176+
113177
afterEach(() => {
114178
getExploreUrlStub.restore();
115179
getChartDataUriStub.restore();

0 commit comments

Comments
 (0)