This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 8cafab339 fix(web): lock repeat confirmations on broker config and
data source writes (#7199)
8cafab339 is described below
commit 8cafab339f57d3cc5b99f808241c4e0f26aee04c
Author: messere <[email protected]>
AuthorDate: Mon Oct 26 20:32:02 2026 +0800
fix(web): lock repeat confirmations on broker config and data source writes
(#7199)
Integration batch of two same-template fixes by messere1: the broker config
submit (#7199) and the data source create/update submit (#7200) both raced a
second confirmation click fired before React re-rendered the submitting state,
putting the same write on the wire twice. Each now takes a synchronous ref
lock, following the nsSubmittingRef convention. The cluster half is adapted to
the submit/applyConfigUpdate split that landed on trunk after the PR was
written: the lock is taken in [...]
---
.../pages/cluster/__tests__/ClusterPage.test.tsx | 73 ++++++++++++++++++++++
web/src/pages/cluster/index.tsx | 25 +++++++-
web/src/pages/settings/DataSourceTab.tsx | 9 ++-
.../settings/__tests__/DataSourceTab.test.tsx | 46 ++++++++++++++
4 files changed, 151 insertions(+), 2 deletions(-)
diff --git a/web/src/pages/cluster/__tests__/ClusterPage.test.tsx
b/web/src/pages/cluster/__tests__/ClusterPage.test.tsx
index 951325d8a..286a90af6 100644
--- a/web/src/pages/cluster/__tests__/ClusterPage.test.tsx
+++ b/web/src/pages/cluster/__tests__/ClusterPage.test.tsx
@@ -24,6 +24,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect,
it, vi } from 'vite
import type {
BrokerConfigDiffResult,
ClusterConfigPreviewResult,
+ ClusterConfigUpdateResult,
ClusterInfo,
ClusterProbeResult,
NameServerConfigDiffResult,
@@ -446,6 +447,78 @@ describe('Cluster page', () => {
expect(within(dialog).getByRole('row', { name: /写队列数/
})).toHaveTextContent('16');
});
+ it('submits a broker config update once per confirmation', async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ const pendingUpdate = deferred<ClusterConfigUpdateResult>();
+
clusterServiceMocks.updateClusterConfig.mockReset().mockReturnValue(pendingUpdate.promise);
+ renderWithProviders(<ClusterPage />);
+
+ const brokerRow = await screen.findByRole('row', { name:
/10\.101\.2\.11:10911/ });
+ await user.click(within(brokerRow).getByRole('button', { name: /^配\s*置$/
}));
+ const dialog = await screen.findByRole('dialog', { name: /配置 -
rocketmq-prod/ });
+ const retentionInput = within(dialog).getByLabelText('文件保留时长 (小时)');
+ await user.clear(retentionInput);
+ await user.type(retentionInput, '96');
+
+ const confirmButton = within(dialog).getByRole('button', { name: /^OK$/ });
+ fireEvent.click(confirmButton);
+ // The dialog must lock with the first click: `configSubmitting` alone
flips only after
+ // validateFields resolves and React re-renders, so a second confirmation
in that window
+ // would put the same broker-config write on the wire twice.
+ expect(confirmButton).toHaveClass('ant-btn-loading');
+ fireEvent.click(confirmButton);
+
+ await waitFor(() =>
expect(clusterServiceMocks.updateClusterConfig).toHaveBeenCalledTimes(1));
+ expect(clusterServiceMocks.updateClusterConfig).toHaveBeenCalledTimes(1);
+
+ await act(async () => {
+ pendingUpdate.resolve({
+ cluster: buildCluster(),
+ status: 'SUCCESS',
+ successfulBrokers: buildCluster().brokers.map((broker) => broker.addr),
+ failedBrokers: [],
+ });
+ await pendingUpdate.promise;
+ });
+ });
+
+ it('keeps the config preview button inert while an update is in flight',
async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ const pendingUpdate = deferred<ClusterConfigUpdateResult>();
+
clusterServiceMocks.updateClusterConfig.mockReset().mockReturnValue(pendingUpdate.promise);
+ renderWithProviders(<ClusterPage />);
+
+ const brokerRow = await screen.findByRole('row', { name:
/10\.101\.2\.11:10911/ });
+ await user.click(within(brokerRow).getByRole('button', { name: /^配\s*置$/
}));
+ const dialog = await screen.findByRole('dialog', { name: /配置 -
rocketmq-prod/ });
+ const retentionInput = within(dialog).getByLabelText('文件保留时长 (小时)');
+ await user.clear(retentionInput);
+ await user.type(retentionInput, '96');
+ const previewButton = within(dialog).getByRole('button', { name: /预\s*览/
});
+
+ // Positive control: the preview runs while no write is in flight.
+ await user.click(previewButton);
+ await waitFor(() =>
expect(clusterServiceMocks.previewClusterConfig).toHaveBeenCalledTimes(1));
+
+ // A write in flight means a preview response would describe the cluster
mid-write, so the
+ // preview button must not start another read while the update is on the
wire.
+ fireEvent.click(within(dialog).getByRole('button', { name: /^OK$/ }));
+ await waitFor(() =>
expect(clusterServiceMocks.updateClusterConfig).toHaveBeenCalledTimes(1));
+ fireEvent.click(previewButton);
+ await act(async () => {});
+ expect(clusterServiceMocks.previewClusterConfig).toHaveBeenCalledTimes(1);
+
+ await act(async () => {
+ pendingUpdate.resolve({
+ cluster: buildCluster(),
+ status: 'SUCCESS',
+ successfulBrokers: buildCluster().brokers.map((broker) => broker.addr),
+ failedBrokers: [],
+ });
+ await pendingUpdate.promise;
+ });
+ });
+
it('preservesUntouchedMessageSizeBytesInPreviewAndSaveTest', async () => {
clusterServiceMocks.listRegistryClusters.mockResolvedValue([
buildCluster({ maxMessageSize: 5000000 }),
diff --git a/web/src/pages/cluster/index.tsx b/web/src/pages/cluster/index.tsx
index 2dfa24696..babd276db 100644
--- a/web/src/pages/cluster/index.tsx
+++ b/web/src/pages/cluster/index.tsx
@@ -171,6 +171,9 @@ const ClusterPage = () => {
const [initialFlushDisk, setInitialFlushDisk] = useState<string |
null>(null);
const [configPreviewLoading, setConfigPreviewLoading] = useState(false);
const [configSubmitting, setConfigSubmitting] = useState(false);
+ // Synchronous lock: the OK button's spinner only renders after React
re-renders, so a second
+ // confirmation click inside the validateFields await would start a
duplicate write.
+ const configSubmittingRef = useRef(false);
const [nsRegistry, setNsRegistry] = useState<NameserverRegistryEntry[]>([]);
const [selectedProxy, setSelectedProxy] = useState<ProxyDetail | null>(null);
const [nsConfigDiffState, setNsConfigDiffState] = useState<{
@@ -772,15 +775,32 @@ const ClusterPage = () => {
}
};
+ const releaseConfigSubmitLock = () => {
+ configSubmittingRef.current = false;
+ setConfigSubmitting(false);
+ };
+
const handleConfigSubmit = async () => {
+ if (configSubmittingRef.current) return;
+ configSubmittingRef.current = true;
+ // Following the nsSubmittingRef convention from the nameserver dialog:
the spinner on the
+ // OK button does not stop a click fired before React re-renders, so the
lock has to be taken
+ // synchronously here, before any await. The write's finally releases it;
the paths that never
+ // reach the write release it themselves, and cancelling the flush-disk
confirmation releases
+ // it too so the dialog is not left dead.
+ setConfigSubmitting(true);
let values: ClusterConfigFormValues;
try {
values = await configForm.validateFields();
} catch {
+ releaseConfigSubmitLock();
return;
}
const request = buildConfigUpdateRequest(values);
- if (!request) return;
+ if (!request) {
+ releaseConfigSubmitLock();
+ return;
+ }
// The flush disk type is the one knob here that a running Broker ignores
until it restarts,
// so applying it gets an explicit second confirmation naming the change —
a silent success
@@ -793,6 +813,7 @@ const ClusterPage = () => {
to: String(values.flushDiskType),
}),
onOk: () => applyConfigUpdate(request),
+ onCancel: releaseConfigSubmitLock,
});
return;
}
@@ -829,6 +850,7 @@ const ClusterPage = () => {
} catch {
message.error(t('cluster.configUpdateFailed', { brokers: '' }));
} finally {
+ configSubmittingRef.current = false;
setConfigSubmitting(false);
}
};
@@ -1446,6 +1468,7 @@ const ClusterPage = () => {
size="small"
icon={<EyeOutlined />}
loading={configPreviewLoading}
+ disabled={configSubmitting}
onClick={() => void handleConfigPreview()}
>
{t('cluster.configPreview')}
diff --git a/web/src/pages/settings/DataSourceTab.tsx
b/web/src/pages/settings/DataSourceTab.tsx
index 9d164be74..1c2755a26 100644
--- a/web/src/pages/settings/DataSourceTab.tsx
+++ b/web/src/pages/settings/DataSourceTab.tsx
@@ -134,6 +134,10 @@ export const DataSourceTab = () => {
const [testingKeys, setTestingKeys] = useState<Set<string>>(() => new Set());
const modalTestSessionRef = useRef(0);
const [submitting, setSubmitting] = useState(false);
+ // `submitting` only renders after validateFields resolves and React
re-renders, so a second
+ // OK click inside that window would put the same create/update write on the
wire twice.
+ // The ref locks the modal synchronously, before any await.
+ const submittingRef = useRef(false);
const [exporting, setExporting] = useState(false);
const requestSeqRef = useRef(0);
const [deleteRefreshVersion, setDeleteRefreshVersion] = useState(0);
@@ -278,12 +282,14 @@ export const DataSourceTab = () => {
};
const handleSubmit = async () => {
+ if (submittingRef.current) return;
+ submittingRef.current = true;
+ setSubmitting(true);
try {
// Credentials are supplied per metrics query and are only used here by
the
// connection test. Saving metadata must not require re-entering them.
const values = await dsForm.validateFields(saveFieldNames);
const dataSourceValues = withoutSecrets(values);
- setSubmitting(true);
const saved = editingDataSource
? await updateDataSource({ ...editingDataSource, ...dataSourceValues })
: await createDataSource(dataSourceValues);
@@ -305,6 +311,7 @@ export const DataSourceTab = () => {
}
message.error(t('settings.dataSourceSaveFailed'));
} finally {
+ submittingRef.current = false;
setSubmitting(false);
}
};
diff --git a/web/src/pages/settings/__tests__/DataSourceTab.test.tsx
b/web/src/pages/settings/__tests__/DataSourceTab.test.tsx
index 0295b103a..599a32f9d 100644
--- a/web/src/pages/settings/__tests__/DataSourceTab.test.tsx
+++ b/web/src/pages/settings/__tests__/DataSourceTab.test.tsx
@@ -772,6 +772,52 @@ describe('DataSourceTab', () => {
});
});
+ it('creates each data source once per modal confirmation', async () => {
+ const pendingCreate = deferred<DataSource>();
+ vi.mocked(createDataSource).mockReturnValueOnce(pendingCreate.promise);
+
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ render(
+ <App>
+ <DataSourceTab />
+ </App>,
+ );
+
+ await screen.findByText('Prometheus prod');
+ await user.click(screen.getByRole('button', { name: /添加数据源/ }));
+ await user.type(screen.getByLabelText('名称'), 'Prometheus dupe');
+ await selectAntdOption(user, '类型', 'Prometheus');
+ await user.type(screen.getByLabelText('URL'), 'http://prometheus:9090');
+
+ const confirmButton = screen.getByRole('button', { name: 'OK' });
+ fireEvent.click(confirmButton);
+ // The modal must lock with the first click: `submitting` alone flips only
after
+ // validateFields resolves and React re-renders, so a second OK in that
window would
+ // create the same data source twice.
+ expect(confirmButton).toHaveClass('ant-btn-loading');
+ fireEvent.click(confirmButton);
+
+ await waitFor(() => expect(createDataSource).toHaveBeenCalledTimes(1));
+ expect(createDataSource).toHaveBeenCalledTimes(1);
+ expect(createDataSource).toHaveBeenCalledWith({
+ name: 'Prometheus dupe',
+ type: 'Prometheus',
+ url: 'http://prometheus:9090',
+ auth: 'None',
+ });
+
+ await act(async () => {
+ pendingCreate.resolve({
+ key: 'prom-dupe',
+ name: 'Prometheus dupe',
+ type: 'Prometheus',
+ url: 'http://prometheus:9090',
+ auth: 'None',
+ });
+ await pendingCreate.promise;
+ });
+ });
+
it('offers Cortex and ARMS alongside the other backend types', async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 });
render(
