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 f4c3f04c1 fix(web): reject writes on absent resources and double
submits (#6044)
f4c3f04c1 is described below
commit f4c3f04c1f53ada7829824867850fce21b4efaa2
Author: messere <[email protected]>
AuthorDate: Mon Oct 12 15:33:05 2026 +0800
fix(web): reject writes on absent resources and double submits (#6044)
Three fixes from the same author for operations that report success when
they should not —
two service-layer lookups that swallow a missing record and one destructive
dialog with no
submit lock — folded into a single commit.
- #6042 `web/src/services/alertRuleAssetService.ts:35` resolves an empty
YAML Blob from the mock
branch of `exportAlertRuleAsset` when the name is not in
`mockAlertRuleAssets`, while
`getAlertRuleAsset` in the same file already throws (`:24-26`) and the
backend raises
`BusinessException(404, "Alert rule asset not found: " + name)`
(`server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleAssetService.java:100`),
so
the demo data source silently exports nothing where the real one reports
404; throws the same
message and pulls the duplicated `find` out of get and export into one
`getMockAlertRuleAsset()` helper.
- #6043 `web/src/services/aclService.ts:168` and `:213` splice only when
the index is found and
otherwise return without a word, so `deleteAclRule` / `deleteAclUser`
report success for a
record that is not there while `updateAclRule` (`:154`) and
`updateAclUser` (`:199`) in the same
file already throw `ACL … not found` — and
`web/src/pages/instance/acl.tsx:349-356` holds no
in-flight lock and refreshes the list asynchronously, so confirming the
same row again before
the refresh lands shows `acl.ruleDeleted` for a no-op; throws on a
missing index with the text
the backend 404 uses, which routes that second confirm into the page's
existing
`common.operationFailed` branch.
- #6044 `web/src/pages/ops/audit.tsx:291-301` gives `handleCleanup` no
in-flight protection and
the cleanup Modal (`:561-568`) no `confirmLoading`, so a destructive
retention delete can be
confirmed twice into two concurrent backend calls and can still be
dismissed with the X, Esc,
the mask or Cancel while the request runs, leaving the UI looking
cancelled while the delete
continues — the export button on the same page is already locked
(`:303-312`, `:510`), so this
was the one write entry left unguarded; adds a synchronous
`cleanupInFlightRef` gate plus a
`cleaning` state, wires `confirmLoading`, disables close / keyboard /
mask / cancel while the
request is in flight, makes `onCancel` a no-op in that window, and resets
both in `finally`.
Three new cases, one per member, all mutation-sensitive: restoring `found ?
found.yaml : ''`
makes `exportAlertRuleAsset('does-not-exist')` resolve instead of rejecting
with the 404 text,
removing either ACL throw fails the matching `rejects.toThrow`, and
removing the cleanup gate lets
both clicks of a double-click into `cleanupAuditLogs`, failing
`toHaveBeenCalledTimes(1)` together
with the `ant-btn-loading` class assertion. Two of them assert more than
the throw or the lock:
the ACL case snapshots `listAclRules()` / `listAclUsers()` before and
after, so a
splice-then-throw implementation also fails, and the audit case checks that
the filter options
still refresh once a cleanup succeeds, so the new gate cannot pass by
swallowing the reload.
Folded: #6042, #6043 were closed as folded into this commit (same author).
---
web/src/pages/ops/__tests__/AuditPage.test.tsx | 24 ++++++++++++++++++++++++
web/src/pages/ops/audit.tsx | 17 ++++++++++++++++-
web/src/services/aclService.test.ts | 13 +++++++++++++
web/src/services/aclService.ts | 6 ++++--
web/src/services/alertRuleAssetService.test.ts | 8 ++++++++
web/src/services/alertRuleAssetService.ts | 18 ++++++++++--------
6 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/web/src/pages/ops/__tests__/AuditPage.test.tsx
b/web/src/pages/ops/__tests__/AuditPage.test.tsx
index 4aa7c1d85..fc5fd5108 100644
--- a/web/src/pages/ops/__tests__/AuditPage.test.tsx
+++ b/web/src/pages/ops/__tests__/AuditPage.test.tsx
@@ -389,4 +389,28 @@ describe('Audit page', () => {
await waitFor(() =>
expect(opsService.getAuditFilterOptions).toHaveBeenCalledTimes(2));
});
+
+ it('submits audit cleanup only once while the request is pending', async ()
=> {
+ const user = userEvent.setup();
+ let resolveCleanup: (deleted: number) => void = () => undefined;
+ vi.mocked(opsService.cleanupAuditLogs).mockImplementation(
+ () =>
+ new Promise<number>((resolve) => {
+ resolveCleanup = resolve;
+ }),
+ );
+
+ renderWithProviders(<AuditPage />);
+
+ expect(await screen.findByText('topic-a')).toBeInTheDocument();
+ await user.click(screen.getByRole('button', { name: '清理日志' }));
+ const confirmButton = await screen.findByRole('button', { name: '确认清理' });
+ await user.dblClick(confirmButton);
+
+ expect(opsService.cleanupAuditLogs).toHaveBeenCalledTimes(1);
+ expect(confirmButton).toHaveClass('ant-btn-loading');
+
+ resolveCleanup(3);
+ await waitFor(() =>
expect(opsService.getAuditFilterOptions).toHaveBeenCalledTimes(2));
+ });
});
diff --git a/web/src/pages/ops/audit.tsx b/web/src/pages/ops/audit.tsx
index d33fd79ca..498e70402 100644
--- a/web/src/pages/ops/audit.tsx
+++ b/web/src/pages/ops/audit.tsx
@@ -106,12 +106,14 @@ const AuditPage: React.FC = () => {
const [filterOptions, setFilterOptions] =
useState<AuditFilterOptions>(emptyFilterOptions);
const [cleanupModalOpen, setCleanupModalOpen] = useState(false);
const [cleanupDays, setCleanupDays] = useState(30);
+ const [cleaning, setCleaning] = useState(false);
const [exporting, setExporting] = useState(false);
const [summary, setSummary] = useState<AuditSummary | null>(null);
const [summaryLoading, setSummaryLoading] = useState(true);
const [timelineResource, setTimelineResource] =
useState<AuditTimelineResource | null>(null);
const recordsRequestRef = useRef(0);
const filterOptionsRequestRef = useRef(0);
+ const cleanupInFlightRef = useRef(false);
useEffect(() => {
const requestId = ++filterOptionsRequestRef.current;
@@ -289,6 +291,9 @@ const AuditPage: React.FC = () => {
};
const handleCleanup = async () => {
+ if (cleanupInFlightRef.current) return;
+ cleanupInFlightRef.current = true;
+ setCleaning(true);
try {
await cleanupAuditLogs(cleanupDays);
setPage(1);
@@ -297,6 +302,9 @@ const AuditPage: React.FC = () => {
setCleanupModalOpen(false);
} catch {
message.error('清理审计日志失败，请稍后重试');
+ } finally {
+ cleanupInFlightRef.current = false;
+ setCleaning(false);
}
};
@@ -562,9 +570,16 @@ const AuditPage: React.FC = () => {
title={t('audit.cleanupTitle')}
open={cleanupModalOpen}
onOk={handleCleanup}
- onCancel={() => setCleanupModalOpen(false)}
+ onCancel={() => {
+ if (!cleanupInFlightRef.current) setCleanupModalOpen(false);
+ }}
okText={t('audit.cleanupConfirm')}
cancelText={t('common.cancel')}
+ confirmLoading={cleaning}
+ closable={!cleaning}
+ keyboard={!cleaning}
+ maskClosable={!cleaning}
+ cancelButtonProps={{ disabled: cleaning }}
okButtonProps={{ danger: true }}
>
<Flex vertical gap={12}>
diff --git a/web/src/services/aclService.test.ts
b/web/src/services/aclService.test.ts
index 6aae08c4a..5a40e1487 100644
--- a/web/src/services/aclService.test.ts
+++ b/web/src/services/aclService.test.ts
@@ -20,6 +20,8 @@ import {
createAclRule,
createAclUser,
createAndUpdatePlainAccessConfig,
+ deleteAclRule,
+ deleteAclUser,
examineBrokerClusterAclConfig,
listAclRules,
listAclUsers,
@@ -153,4 +155,15 @@ describe('ACL service mock data', () => {
'secretKey is required',
);
});
+
+ it('rejects deletion of missing ACL records', async () => {
+ const rulesBefore = await listAclRules();
+ const usersBefore = await listAclUsers();
+
+ await expect(deleteAclRule('missing-rule')).rejects.toThrow('ACL rule not
found: missing-rule');
+ await expect(deleteAclUser('missing-user')).rejects.toThrow('ACL user not
found: missing-user');
+
+ await expect(listAclRules()).resolves.toEqual(rulesBefore);
+ await expect(listAclUsers()).resolves.toEqual(usersBefore);
+ });
});
diff --git a/web/src/services/aclService.ts b/web/src/services/aclService.ts
index 877f6c182..2b5084376 100644
--- a/web/src/services/aclService.ts
+++ b/web/src/services/aclService.ts
@@ -165,7 +165,8 @@ export async function updateAclRule(
export async function deleteAclRule(id: AclEntityId, instanceId?: string):
Promise<void> {
if (isMockMode()) {
const idx = aclRulesState.findIndex((rule) => rule.id === id);
- if (idx >= 0) aclRulesState.splice(idx, 1);
+ if (idx < 0) throw new Error(`ACL rule not found: ${id}`);
+ aclRulesState.splice(idx, 1);
return;
}
return aclApi.deleteAclRule(id, instanceId);
@@ -210,7 +211,8 @@ export async function updateAclUser(
export async function deleteAclUser(id: AclEntityId, instanceId?: string):
Promise<void> {
if (isMockMode()) {
const idx = aclUsersState.findIndex((user) => user.id === id);
- if (idx >= 0) aclUsersState.splice(idx, 1);
+ if (idx < 0) throw new Error(`ACL user not found: ${id}`);
+ aclUsersState.splice(idx, 1);
return;
}
return aclApi.deleteAclUser(id, instanceId);
diff --git a/web/src/services/alertRuleAssetService.test.ts
b/web/src/services/alertRuleAssetService.test.ts
index 2c545f713..34d545d77 100644
--- a/web/src/services/alertRuleAssetService.test.ts
+++ b/web/src/services/alertRuleAssetService.test.ts
@@ -73,6 +73,14 @@ describe('alertRuleAssetService (mock mode)', () => {
expect(blob).toBeInstanceOf(Blob);
expect(blob.type).toBe('text/yaml');
});
+
+ it('rejects an export for an unknown asset', async () => {
+ mode.mock = true;
+
+ await expect(exportAlertRuleAsset('does-not-exist')).rejects.toThrow(
+ 'Alert rule asset not found: does-not-exist',
+ );
+ });
});
describe('alertRuleAssetService (real mode)', () => {
diff --git a/web/src/services/alertRuleAssetService.ts
b/web/src/services/alertRuleAssetService.ts
index cd82389fa..db8d296dc 100644
--- a/web/src/services/alertRuleAssetService.ts
+++ b/web/src/services/alertRuleAssetService.ts
@@ -6,6 +6,14 @@ import * as alertRuleAssetsApi from '../api/alertRuleAssets';
import type { AlertRuleAssetInfo } from '../api/alertRuleAssets';
import { mockAlertRuleAssets } from '../mock/alertRuleAssets';
+function getMockAlertRuleAsset(name: string) {
+ const found = mockAlertRuleAssets.find((asset) => asset.name === name);
+ if (!found) {
+ throw new Error(`Alert rule asset not found: ${name}`);
+ }
+ return found;
+}
+
export async function listAlertRuleAssets(): Promise<AlertRuleAssetInfo[]> {
if (isMockMode()) {
return mockAlertRuleAssets.map(({ name, group, ruleCount, severities }) =>
({
@@ -20,20 +28,14 @@ export async function listAlertRuleAssets():
Promise<AlertRuleAssetInfo[]> {
export async function getAlertRuleAsset(name: string): Promise<string> {
if (isMockMode()) {
- const found = mockAlertRuleAssets.find((asset) => asset.name === name);
- if (!found) {
- throw new Error(`Alert rule asset not found: ${name}`);
- }
- return found.yaml;
+ return getMockAlertRuleAsset(name).yaml;
}
return alertRuleAssetsApi.getAlertRuleAsset(name);
}
export async function exportAlertRuleAsset(name: string): Promise<Blob> {
if (isMockMode()) {
- const found = mockAlertRuleAssets.find((asset) => asset.name === name);
- const yaml = found ? found.yaml : '';
- return new Blob([yaml], { type: 'text/yaml' });
+ return new Blob([getMockAlertRuleAsset(name).yaml], { type: 'text/yaml' });
}
return alertRuleAssetsApi.exportAlertRuleAsset(name);
}
