You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by li...@apache.org on 2026/10/12 08:13:26 UTC
(rocketmq-dashboard) branch rocketmq-studio updated: fix(settings): align the data-source, model and ACL contracts (#6011)
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 d2f0f762c fix(settings): align the data-source, model and ACL contracts (#6011)
d2f0f762c is described below
commit d2f0f762cc8e503fe314168e7cb79ef6bf40fab1
Author: 烤化の初雪 <ya...@outlook.com>
AuthorDate: Mon Oct 12 16:13:20 2026 +0800
fix(settings): align the data-source, model and ACL contracts (#6011)
Three implementations that disagreed with a contract the repository already publishes — the API spec, the server's curated model list, and the duplicate-key handling used by every sibling path in the same ACL repository.
- #6011 `SettingsController.java:83` bound only `@RequestParam(required = false) String key` for `POST /api/settings/datasources/delete`, while `docs/api-spec.md` §14.7 (`:2362-2366`) documents `key` as a required **body** field, so a spec-conformant client sent `key = null` and `SettingsService.deleteDataSource` (`:276-283`) answered 400 "Data source key is required" having deleted nothing → the handler also accepts `@RequestBody(required = false) DataSourceKeyDTO`, a new `@Data` DTO [...]
- #6013 `web/src/pages/studio/llmModelOptions.ts:31-40` had drifted from the server's curated `LlmConfigService.PROVIDER_MODELS` — `tongyi` listed 7 of the 12 ids at `LlmConfigService.java:69-81` and `ollama` listed `gemma2`, which `:82-85` does not serve → the fallback mirrors both lists in server order with a comment naming `PROVIDER_MODELS` as the single source of truth, so `AiAssistantTab.tsx:130` (provider switch) and `:189-190` (first load, or a failed `GET /llm/models`) stop hi [...]
- #6015 `MybatisPlusAclRepository.createAndUpdatePlainAccessConfig` always executes `entity.setUsername(config.getAccessKey())` at `:257` and then a bare `userMapper.updateById(entity)` at `:270`, so when a stored row's username had been renamed away from its access key (`replaceUser` supports renaming, `:164-176`) and another row already owned `username = accessKey`, the `uk_username` violation escaped as a 500 "Internal Server Error" and the `@Transactional` method (`:235-236`) roll [...]
Tests: one case each, all mutation-sensitive — `SettingsControllerTest.deleteDataSourceShouldAcceptTheDocumentedKeyBodyTest` (23 → 24) pins `isOk()` plus `verify(settingsService).deleteDataSource("ds-1")` alongside the existing query-parameter and missing-key cases, `LlmSettings.test.ts` (4 → 5) pins both providers' ordered lists with `toEqual`, and `MybatisPlusAclRepositoryTest` (25 → 26) pins the 409, its message and `verifyNoInteractions(ruleMapper)`, filling the one duplicate-key [...]
Folded: #6013, #6015 were closed as folded into this commit (same author).
---
.../instance/acl/MybatisPlusAclRepository.java | 10 ++++++++-
.../rocketmq/studio/settings/DataSourceKeyDTO.java | 26 ++++++++++++++++++++++
.../studio/settings/SettingsController.java | 8 +++++--
.../instance/acl/MybatisPlusAclRepositoryTest.java | 19 ++++++++++++++++
.../studio/settings/SettingsControllerTest.java | 15 +++++++++++++
web/src/pages/studio/__tests__/LlmSettings.test.ts | 25 +++++++++++++++++++++
web/src/pages/studio/llmModelOptions.ts | 14 +++++++++++-
7 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
index a848bdbcf..5f751e992 100644
--- a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
+++ b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
@@ -267,7 +267,15 @@ public class MybatisPlusAclRepository implements AclRepository {
entity.setWhiteRemoteAddress(normalizeWhiteRemoteAddress(config.getWhiteRemoteAddress()));
entity.setGmtModified(LocalDateTime.now());
if (existing != null) {
- userMapper.updateById(entity);
+ try {
+ userMapper.updateById(entity);
+ } catch (DuplicateKeyException exception) {
+ // The update rewrites username to the accessKey, so a row that was renamed through
+ // the users form (or created with that name concurrently) hits uk_username here -
+ // the same collision the sibling write paths already translate, and without this the
+ // caller saw a generic 500 "Internal Server Error" for a client mistake.
+ throw aclUserConflict(config.getAccessKey());
+ }
if (entity.getWhiteRemoteAddress() == null) {
// Clearing the whitelist must persist as a null column; updateById skips null fields.
clearColumn(userMapper, entity.getId(), "white_remote_address");
diff --git a/server/src/main/java/org/apache/rocketmq/studio/settings/DataSourceKeyDTO.java b/server/src/main/java/org/apache/rocketmq/studio/settings/DataSourceKeyDTO.java
new file mode 100644
index 000000000..2c35a060f
--- /dev/null
+++ b/server/src/main/java/org/apache/rocketmq/studio/settings/DataSourceKeyDTO.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.settings;
+
+import lombok.Data;
+
+/** The documented request body of the data-source deletion, which also accepts the query parameter. */
+@Data
+public class DataSourceKeyDTO {
+
+ private String key;
+}
diff --git a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
index d80d588aa..f8f01c9b3 100644
--- a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
+++ b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
@@ -80,8 +80,12 @@ public class SettingsController {
}
@PostMapping("/datasources/delete")
- public Result<Void> deleteDataSource(@RequestParam(required = false) String key) {
- settingsService.deleteDataSource(key);
+ public Result<Void> deleteDataSource(@RequestParam(required = false) String key,
+ @RequestBody(required = false) DataSourceKeyDTO request) {
+ // The API spec documents a {"key": ...} body while the console sends the query parameter;
+ // binding only the parameter made the documented request a 400 "Data source key is
+ // required" and deleted nothing.
+ settingsService.deleteDataSource(key != null ? key : request == null ? null : request.getKey());
return Result.ok();
}
diff --git a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
index b78d4e340..6b5b984d9 100644
--- a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
+++ b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
@@ -418,6 +418,25 @@ class MybatisPlusAclRepositoryTest {
verifyNoInteractions(ruleMapper);
}
+ @Test
+ void updateShouldReportARenameOntoAnExistingUsernameAsAConflictTest() {
+ RmqAclUser existing = userEntity(1L, "svc-c", CredentialUtils.encodeBase64("kept-secret"));
+ existing.setAccessKey("K");
+ when(userMapper.selectList(any(QueryWrapper.class))).thenReturn(List.of(existing));
+ // Another row already owns username=K (uk_username), e.g. a user created after this account
+ // was renamed away from it.
+ when(userMapper.updateById(any(RmqAclUser.class)))
+ .thenThrow(new org.springframework.dao.DuplicateKeyException("duplicate"));
+
+ PlainAccessConfigVO config = PlainAccessConfigVO.builder().accessKey("K").build();
+
+ assertThatThrownBy(() -> repository.createAndUpdatePlainAccessConfig(config))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("ACL user already exists: K")
+ .satisfies(error -> assertThat(((BusinessException) error).getCode()).isEqualTo(409));
+ org.mockito.Mockito.verifyNoInteractions(ruleMapper);
+ }
+
@Test
void updateWithBlankSecretShouldKeepStoredSecret() {
RmqAclUser existing = userEntity(1L, "svc-x",
diff --git a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
index 4067d4f2d..74258f82b 100644
--- a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
+++ b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
@@ -444,6 +444,21 @@ class SettingsControllerTest extends WebMvcAuthTestSupport {
verify(settingsService).deleteDataSource("ds-1");
}
+ @Test
+ void deleteDataSourceShouldAcceptTheDocumentedKeyBodyTest() throws Exception {
+ doNothing().when(settingsService).deleteDataSource("ds-1");
+
+ // The API spec (§14.7) documents a {"key": ...} body; binding only the query parameter made
+ // a doc-conformant client's request a 400 that deleted nothing.
+ mockMvc.perform(post("/api/settings/datasources/delete")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("{\"key\":\"ds-1\"}"))
+ .andExpect(status().isOk())
+ .andExpect(jsonPath("$.code", is(200)));
+
+ verify(settingsService).deleteDataSource("ds-1");
+ }
+
@Test
void deleteDataSourceShouldRejectMissingKeyTest() throws Exception {
doThrow(new BusinessException(400, "Data source key is required"))
diff --git a/web/src/pages/studio/__tests__/LlmSettings.test.ts b/web/src/pages/studio/__tests__/LlmSettings.test.ts
index a00b44867..490d0b660 100644
--- a/web/src/pages/studio/__tests__/LlmSettings.test.ts
+++ b/web/src/pages/studio/__tests__/LlmSettings.test.ts
@@ -56,6 +56,31 @@ describe('LlmSettingsPage', () => {
]);
});
+ it('mirrors the server curated model list per provider', () => {
+ // LlmConfigService.PROVIDER_MODELS is the source of truth (GET /llm/models and the home page
+ // picker). The fallback is what the settings dialog shows when that endpoint fails or the
+ // provider is re-selected, so a shorter list hides models the gateway serves.
+ expect(fallbackModelOptions('tongyi').map((option) => option.value)).toEqual([
+ 'qwen3.8-max',
+ 'qwen3.7-max',
+ 'qwen3.7-plus',
+ 'gpt-5',
+ 'gpt-5.1',
+ 'claude-fable-5',
+ 'claude-opus-5',
+ 'claude-sonnet-5',
+ 'deepseek-v4-pro',
+ 'deepseek-v4-flash',
+ 'MiniMax-M2.5',
+ 'glm-5.2',
+ ]);
+ expect(fallbackModelOptions('ollama').map((option) => option.value)).toEqual([
+ 'llama3',
+ 'mistral',
+ 'qwen2.5',
+ ]);
+ });
+
it('falls back to the current model for unknown providers', () => {
expect(fallbackModelOptions('custom', 'custom-model')).toEqual([
{ value: 'custom-model', label: 'custom-model' },
diff --git a/web/src/pages/studio/llmModelOptions.ts b/web/src/pages/studio/llmModelOptions.ts
index ba94e032f..b81c8d428 100644
--- a/web/src/pages/studio/llmModelOptions.ts
+++ b/web/src/pages/studio/llmModelOptions.ts
@@ -28,16 +28,28 @@ export const FALLBACK_MODELS: Record<string, string[]> = {
'claude-haiku-4-5',
],
deepseek: ['deepseek-chat', 'deepseek-reasoner'],
+ /**
+ * Mirrors `LlmConfigService.PROVIDER_MODELS["tongyi"]` (what `GET /llm/models` answers and what
+ * the home page offers). The list is used whenever the endpoint fails or the provider is
+ * re-selected, so a shorter copy hid five of the twelve models the gateway serves.
+ */
tongyi: [
'qwen3.8-max',
'qwen3.7-max',
'qwen3.7-plus',
+ 'gpt-5',
+ 'gpt-5.1',
+ 'claude-fable-5',
+ 'claude-opus-5',
+ 'claude-sonnet-5',
'deepseek-v4-pro',
'deepseek-v4-flash',
'MiniMax-M2.5',
'glm-5.2',
],
- ollama: ['llama3', 'mistral', 'gemma2', 'qwen2.5'],
+ // Mirrors the server's list for the same reason (it offered gemma2, which the gateway does not
+ // serve).
+ ollama: ['llama3', 'mistral', 'qwen2.5'],
bedrock: [
'anthropic.claude-fable-5',
'anthropic.claude-opus-5',