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 c4c2d96e fix: harden web service contracts (#924)
c4c2d96e is described below
commit c4c2d96e348dd2a8b691a29f32e4cdb0f71d8045
Author: aias00 <[email protected]>
AuthorDate: Tue Aug 4 03:03:26 2026 -0700
fix: harden web service contracts (#924)
* [ISSUE #909] Guard stale cluster store refreshes
Signed-off-by: liuhy <[email protected]>
* [ISSUE #911] Trim consumer group search text
Signed-off-by: liuhy <[email protected]>
* [ISSUE #913] Reject missing alert rule updates
Signed-off-by: liuhy <[email protected]>
* fix(web): copy k8s cert sans before storing
Signed-off-by: liuhy <[email protected]>
* fix(web): reject duplicate nameserver edits
Signed-off-by: liuhy <[email protected]>
* fix(web): reject duplicate topic creates
Signed-off-by: liuhy <[email protected]>
* fix(web): reject missing mock instance deletes
Signed-off-by: liuhy <[email protected]>
* fix(web): filter mock messages by time range
Signed-off-by: liuhy <[email protected]>
---------
Signed-off-by: liuhy <[email protected]>
---
web/src/services/clusterService.test.ts | 70 +++++++++++++++++++++++++++++++-
web/src/services/clusterService.ts | 13 +++---
web/src/services/consumerService.test.ts | 13 ++++++
web/src/services/consumerService.ts | 4 +-
web/src/services/instanceService.test.ts | 12 +++++-
web/src/services/instanceService.ts | 3 +-
web/src/services/messageService.test.ts | 12 ++++++
web/src/services/messageService.ts | 13 ++++++
web/src/services/opsService.test.ts | 17 ++++++++
web/src/services/opsService.ts | 3 +-
web/src/services/topicService.test.ts | 22 +++++++++-
web/src/services/topicService.ts | 5 +++
web/src/stores/clusterStore.test.ts | 44 ++++++++++++++++++++
web/src/stores/clusterStore.ts | 11 ++++-
14 files changed, 228 insertions(+), 14 deletions(-)
diff --git a/web/src/services/clusterService.test.ts
b/web/src/services/clusterService.test.ts
index 555c45f3..1d9e0ed6 100644
--- a/web/src/services/clusterService.test.ts
+++ b/web/src/services/clusterService.test.ts
@@ -22,7 +22,16 @@ vi.mock('../config', () => ({
API_BASE_URL: '/api',
}));
-import { getCluster, listClusters, updateClusterConfig } from
'./clusterService';
+import {
+ createK8sCert,
+ deleteK8sCert,
+ getCluster,
+ listClusters,
+ listK8sCerts,
+ updateClusterConfig,
+ updateK8sCert,
+ updateNameServer,
+} from './clusterService';
describe('clusterService mock clusters', () => {
it('returns defensive copies from cluster detail reads', async () => {
@@ -85,4 +94,63 @@ describe('clusterService mock clusters', () => {
});
}
});
+
+ it('rejects NameServer edits that would duplicate an address in the same
cluster', async () => {
+ const clusterId = 'cluster-prod';
+ const original = await getCluster(clusterId);
+ const [first, second] = original.nameServers;
+
+ try {
+ await expect(
+ updateNameServer({
+ clusterId,
+ addr: first.addr,
+ newAddr: second.addr,
+ }),
+ ).rejects.toThrow(`NameServer already exists: ${second.addr}`);
+
+ const fresh = await getCluster(clusterId);
+ expect(fresh.nameServers.map((item) => item.addr)).toEqual(
+ original.nameServers.map((item) => item.addr),
+ );
+ } finally {
+ const current = await getCluster(clusterId);
+ for (let index = 0; index < original.nameServers.length; index += 1) {
+ const currentAddr = current.nameServers[index]?.addr;
+ const originalAddr = original.nameServers[index].addr;
+ if (currentAddr && currentAddr !== originalAddr) {
+ await updateNameServer({ clusterId, addr: currentAddr, newAddr:
originalAddr });
+ }
+ }
+ }
+ });
+
+ it('copies certificate SAN arrays before writing them into the mock store',
async () => {
+ const san = ['proxy.example.com'];
+ const created = await createK8sCert({
+ name: 'cert-copy-test',
+ namespace: 'rocketmq',
+ cluster: 'cluster-prod',
+ san,
+ });
+
+ try {
+ san.push('mutated-create.example.com');
+
+ let stored = (await listK8sCerts()).find((cert) => cert.id ===
created.id);
+ expect(stored?.san).toEqual(['proxy.example.com']);
+
+ const nextSan = ['proxy-next.example.com'];
+ await updateK8sCert({
+ id: created.id,
+ san: nextSan,
+ });
+ nextSan.push('mutated-update.example.com');
+
+ stored = (await listK8sCerts()).find((cert) => cert.id === created.id);
+ expect(stored?.san).toEqual(['proxy-next.example.com']);
+ } finally {
+ await deleteK8sCert(created.id);
+ }
+ });
});
diff --git a/web/src/services/clusterService.ts
b/web/src/services/clusterService.ts
index de6677dd..601f9f22 100644
--- a/web/src/services/clusterService.ts
+++ b/web/src/services/clusterService.ts
@@ -82,7 +82,7 @@ export async function createK8sCert(data:
Partial<K8sCertInfo>): Promise<K8sCert
notAfter: notAfter.toISOString(),
status: 'valid',
daysRemaining: 365,
- san: data.san ?? [],
+ san: [...(data.san ?? [])],
};
mockCertStore.push(cert);
return { ...cert, san: [...cert.san] };
@@ -94,7 +94,7 @@ export async function updateK8sCert(data:
Partial<K8sCertInfo>): Promise<K8sCert
if (isMockMode()) {
const existing = mockCertStore.find((cert) => cert.id === data.id);
if (!existing) throw new Error(`Certificate not found: ${data.id}`);
- Object.assign(existing, data, { san: data.san ?? existing.san });
+ Object.assign(existing, data, { san: data.san ? [...data.san] :
existing.san });
return { ...existing, san: [...existing.san] };
}
return clusterApi.updateK8sCert(data);
@@ -204,10 +204,13 @@ export async function updateNameServer(data: {
newAddr?: string;
}): Promise<void> {
if (isMockMode()) {
- const nameServer = getMockCluster(data.clusterId).nameServers.find(
- (item) => item.addr === data.addr,
- );
+ const nameServers = getMockCluster(data.clusterId).nameServers;
+ const nameServer = nameServers.find((item) => item.addr === data.addr);
if (!nameServer) throw new Error(`NameServer not found: ${data.addr}`);
+ if (data.newAddr && data.newAddr !== data.addr) {
+ const duplicate = nameServers.some((item) => item !== nameServer &&
item.addr === data.newAddr);
+ if (duplicate) throw new Error(`NameServer already exists:
${data.newAddr}`);
+ }
if (data.newAddr) nameServer.addr = data.newAddr;
return;
}
diff --git a/web/src/services/consumerService.test.ts
b/web/src/services/consumerService.test.ts
index d5d637cf..4d61b45d 100644
--- a/web/src/services/consumerService.test.ts
+++ b/web/src/services/consumerService.test.ts
@@ -55,6 +55,19 @@ describe('consumer service mock data', () => {
expect(second.instances[0]).not.toBe(first.instances[0]);
});
+ it('trims search text before filtering consumer group names', async () => {
+ const groups = await listConsumerGroups({ search: ' CG-ORDER-NOTIFY ' });
+
+ expect(groups.map((group) => group.name)).toEqual(['cg-order-notify']);
+ });
+
+ it('ignores blank search text', async () => {
+ const allGroups = await listConsumerGroups();
+ const blankSearchGroups = await listConsumerGroups({ search: ' ' });
+
+ expect(blankSearchGroups).toHaveLength(allGroups.length);
+ });
+
it('returns copied progress and subscription rows', async () => {
const firstProgress = await getConsumerProgress('cg-order-notify');
const firstSubscriptions = await
getConsumerSubscriptions('cg-order-notify');
diff --git a/web/src/services/consumerService.ts
b/web/src/services/consumerService.ts
index 6cfc0678..256c1b0c 100644
--- a/web/src/services/consumerService.ts
+++ b/web/src/services/consumerService.ts
@@ -43,8 +43,8 @@ export async function listConsumerGroups(params?:
ConsumerGroupQuery): Promise<C
let result = [...consumerGroupsState];
if (params?.clusterId) result = result.filter((group) => group.clusterId
=== params.clusterId);
if (params?.search) {
- const kw = params.search.toLowerCase();
- result = result.filter((g) => g.name.toLowerCase().includes(kw));
+ const kw = params.search.trim().toLowerCase();
+ if (kw) result = result.filter((g) => g.name.toLowerCase().includes(kw));
}
return result.map(copyConsumerGroup);
}
diff --git a/web/src/services/instanceService.test.ts
b/web/src/services/instanceService.test.ts
index 5d285f05..5b8edb95 100644
--- a/web/src/services/instanceService.test.ts
+++ b/web/src/services/instanceService.test.ts
@@ -22,7 +22,7 @@ vi.mock('../config', () => ({
API_BASE_URL: '/api',
}));
-import { createInstance, listInstances, updateInstance } from
'./instanceService';
+import { createInstance, deleteInstance, listInstances, updateInstance } from
'./instanceService';
describe('instanceService mock instances', () => {
it('returns defensive copies from list reads', async () => {
@@ -82,4 +82,14 @@ describe('instanceService mock instances', () => {
const storedUpdated = afterUpdate.find((instance) => instance.id ===
created.id);
expect(storedUpdated?.remark).toBe('updated');
});
+
+ it('rejects deleting missing mock instances', async () => {
+ const before = await listInstances();
+
+ await expect(deleteInstance('missing-instance')).rejects.toThrow(
+ 'Instance not found: missing-instance',
+ );
+
+ await expect(listInstances()).resolves.toEqual(before);
+ });
});
diff --git a/web/src/services/instanceService.ts
b/web/src/services/instanceService.ts
index d162c160..54121765 100644
--- a/web/src/services/instanceService.ts
+++ b/web/src/services/instanceService.ts
@@ -65,7 +65,8 @@ export async function updateInstance(data:
UpdateInstanceRequest): Promise<Insta
export async function deleteInstance(id: string): Promise<void> {
if (isMockMode()) {
const idx = mockInstances.findIndex((i) => i.id === id);
- if (idx >= 0) mockInstances.splice(idx, 1);
+ if (idx < 0) throw new Error(`Instance not found: ${id}`);
+ mockInstances.splice(idx, 1);
return;
}
return instanceApi.deleteInstance(id);
diff --git a/web/src/services/messageService.test.ts
b/web/src/services/messageService.test.ts
index 3e010f01..377eb5d5 100644
--- a/web/src/services/messageService.test.ts
+++ b/web/src/services/messageService.test.ts
@@ -39,6 +39,18 @@ describe('message service mock data', () => {
expect(second[0].properties).not.toBe(first[0].properties);
});
+ it('filters mock topic queries by the selected store-time range', async ()
=> {
+ const messages = await queryMessages({
+ topic: 'order-create',
+ startTime: Date.parse('2026-07-01T10:24:00.000Z'),
+ endTime: Date.parse('2026-07-01T10:26:00.000Z'),
+ });
+
+ expect(messages.map((message) => message.msgId)).toEqual([
+ 'AC1E0A6400002A9F0000000001A3F7C2',
+ ]);
+ });
+
it('returns copied message trace rows', async () => {
const first = await getMessageTrace('AC1E0A6400002A9F0000000001A3F2B1');
expect(first?.nodes[0].title).toBe('Producer 发送');
diff --git a/web/src/services/messageService.ts
b/web/src/services/messageService.ts
index 44d74c27..80decb21 100644
--- a/web/src/services/messageService.ts
+++ b/web/src/services/messageService.ts
@@ -17,6 +17,13 @@ const cloneTrace = (trace: TraceRecord): TraceRecord => ({
const cloneDLQGroup = (group: DLQGroup): DLQGroup => ({ ...group });
+const toStoreTimestamp = (storeTime: MessageRecord['storeTime']): number => {
+ if (typeof storeTime === 'number') return storeTime;
+
+ const parsed = Date.parse(storeTime);
+ return Number.isNaN(parsed) ? 0 : parsed;
+};
+
export async function queryMessages(params: MessageQuery):
Promise<MessageRecord[]> {
if (isMockMode()) {
let result = [...mockMessages];
@@ -24,6 +31,12 @@ export async function queryMessages(params: MessageQuery):
Promise<MessageRecord
if (params.tag) result = result.filter((m) => m.tag === params.tag);
if (params.key) result = result.filter((m) => m.key.includes(params.key!));
if (params.msgId) result = result.filter((m) => m.msgId === params.msgId);
+ if (params.startTime !== undefined) {
+ result = result.filter((m) => toStoreTimestamp(m.storeTime) >=
params.startTime!);
+ }
+ if (params.endTime !== undefined) {
+ result = result.filter((m) => toStoreTimestamp(m.storeTime) <=
params.endTime!);
+ }
return sortMessagesByStoreTimeDesc((result as unknown as
MessageRecord[]).map(cloneMessage));
}
return messageApi.queryMessages(params);
diff --git a/web/src/services/opsService.test.ts
b/web/src/services/opsService.test.ts
index ae02a2dc..e06f1448 100644
--- a/web/src/services/opsService.test.ts
+++ b/web/src/services/opsService.test.ts
@@ -82,6 +82,23 @@ describe('ops service mock data', () => {
expect(afterUpdate?.channels).toEqual(['webhook']);
});
+ it('rejects updates for unknown alert rule IDs', async () => {
+ const before = await listAlertRules();
+ const missingRule = {
+ ...before[0],
+ id: 'missing-alert-rule',
+ name: 'missing rule',
+ };
+
+ await expect(updateAlertRule(missingRule)).rejects.toThrow(
+ 'Alert rule not found: missing-alert-rule',
+ );
+
+ const after = await listAlertRules();
+ expect(after.map((rule) => rule.id)).toEqual(before.map((rule) =>
rule.id));
+ expect(after.find((rule) => rule.id ===
'missing-alert-rule')).toBeUndefined();
+ });
+
it('returns copied system alert rows', async () => {
const first = await listSystemAlerts();
const originalTitle = first[0].title;
diff --git a/web/src/services/opsService.ts b/web/src/services/opsService.ts
index b27819ad..0bcde5f0 100644
--- a/web/src/services/opsService.ts
+++ b/web/src/services/opsService.ts
@@ -101,8 +101,9 @@ export async function createAlertRule(data:
Partial<AlertRule>): Promise<AlertRu
export async function updateAlertRule(data: AlertRule): Promise<AlertRule> {
if (isMockMode()) {
const index = alertRulesState.findIndex((rule) => rule.id === data.id);
+ if (index < 0) throw new Error(`Alert rule not found: ${data.id}`);
const rule = copyAlertRule(data);
- if (index >= 0) alertRulesState[index] = rule;
+ alertRulesState[index] = rule;
return copyAlertRule(rule);
}
return opsApi.updateAlertRule(data);
diff --git a/web/src/services/topicService.test.ts
b/web/src/services/topicService.test.ts
index 04580f07..4fe13bf8 100644
--- a/web/src/services/topicService.test.ts
+++ b/web/src/services/topicService.test.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-import { getTopicConsumers, getTopicRoutes, listTopics } from './topicService';
+import { createTopic, getTopicConsumers, getTopicRoutes, listTopics } from
'./topicService';
vi.mock('./dataMode', () => ({ isMockMode: () => true }));
vi.mock('../config', () => ({
@@ -68,4 +68,24 @@ describe('topic service mock data', () => {
expect(blankSearchTopics).toHaveLength(allTopics.length);
});
+
+ it('rejects duplicate topic creates in the same cluster', async () => {
+ const existing = (await listTopics({ search: 'order-create' }))[0];
+ const before = await listTopics({ clusterId: existing.clusterId });
+
+ await expect(
+ createTopic({
+ name: existing.name,
+ clusterId: existing.clusterId,
+ namespace: existing.namespace,
+ type: existing.type,
+ writeQueues: existing.writeQueues,
+ readQueues: existing.readQueues,
+ perm: existing.perm,
+ }),
+ ).rejects.toThrow(`Topic already exists: ${existing.name}`);
+
+ const after = await listTopics({ clusterId: existing.clusterId });
+ expect(after).toEqual(before);
+ });
});
diff --git a/web/src/services/topicService.ts b/web/src/services/topicService.ts
index ca1e945a..90617928 100644
--- a/web/src/services/topicService.ts
+++ b/web/src/services/topicService.ts
@@ -31,6 +31,11 @@ export async function listTopics(params?: TopicQuery):
Promise<Topic[]> {
export async function createTopic(data: Partial<Topic>): Promise<Topic> {
if (isMockMode()) {
+ const duplicate = mockTopics.some(
+ (topic) => topic.name === data.name && topic.clusterId ===
data.clusterId,
+ );
+ if (duplicate) throw new Error(`Topic already exists: ${data.name}`);
+
const topic = {
...data,
createdAt: new Date().toISOString(),
diff --git a/web/src/stores/clusterStore.test.ts
b/web/src/stores/clusterStore.test.ts
index 3084b22c..8463c33f 100644
--- a/web/src/stores/clusterStore.test.ts
+++ b/web/src/stores/clusterStore.test.ts
@@ -52,6 +52,23 @@ const cluster: ClusterInfo = {
tpsHistory: [100, 120],
};
+const newerCluster: ClusterInfo = {
+ ...cluster,
+ id: 'cluster-staging',
+ name: 'rocketmq-staging',
+};
+
+function deferred<T>() {
+ let resolve!: (value: T) => void;
+ let reject!: (reason?: unknown) => void;
+ const promise = new Promise<T>((resolvePromise, rejectPromise) => {
+ resolve = resolvePromise;
+ reject = rejectPromise;
+ });
+
+ return { promise, resolve, reject };
+}
+
describe('clusterStore', () => {
afterEach(() => {
vi.mocked(listClusters).mockReset();
@@ -70,6 +87,33 @@ describe('clusterStore', () => {
});
});
+ it('keeps the newest cluster list when overlapping loads finish out of
order', async () => {
+ const firstLoad = deferred<ClusterInfo[]>();
+ const secondLoad = deferred<ClusterInfo[]>();
+ vi.mocked(listClusters)
+ .mockReturnValueOnce(firstLoad.promise)
+ .mockReturnValueOnce(secondLoad.promise);
+
+ const firstFetch = useClusterStore.getState().fetchClusters();
+ const secondFetch = useClusterStore.getState().fetchClusters();
+
+ secondLoad.resolve([newerCluster]);
+ await secondFetch;
+
+ expect(useClusterStore.getState()).toMatchObject({
+ clusters: [newerCluster],
+ loading: false,
+ });
+
+ firstLoad.resolve([cluster]);
+ await firstFetch;
+
+ expect(useClusterStore.getState()).toMatchObject({
+ clusters: [newerCluster],
+ loading: false,
+ });
+ });
+
it('resets loading when loading clusters fails', async () => {
const error = new Error('failed to load clusters');
vi.mocked(listClusters).mockRejectedValue(error);
diff --git a/web/src/stores/clusterStore.ts b/web/src/stores/clusterStore.ts
index 48795048..ea6c349f 100644
--- a/web/src/stores/clusterStore.ts
+++ b/web/src/stores/clusterStore.ts
@@ -25,16 +25,23 @@ interface ClusterState {
fetchClusters: () => Promise<void>;
}
+let latestFetchRequestId = 0;
+
const useClusterStore = create<ClusterState>((set) => ({
clusters: [],
loading: false,
fetchClusters: async () => {
+ const requestId = ++latestFetchRequestId;
set({ loading: true });
try {
const clusters = await listClusters();
- set({ clusters });
+ if (requestId === latestFetchRequestId) {
+ set({ clusters });
+ }
} finally {
- set({ loading: false });
+ if (requestId === latestFetchRequestId) {
+ set({ loading: false });
+ }
}
},
}));
