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 c6b546a73 fix(settings): avoid data source cache key collisions (#4049)
c6b546a73 is described below
commit c6b546a7322a1700a9b2a143601b301901962438
Author: halaxy <[email protected]>
AuthorDate: Wed Sep 9 21:03:01 2026 +0800
fix(settings): avoid data source cache key collisions (#4049)
`SettingsService.listDataSources(search, type, page, pageSize)` built its
`@Cacheable` key by SpEL string concatenation:
```
key = "'page:' + #search + ':' + #type + ':' + #page + ':' + #pageSize"
```
Java renders a null `String` as the literal text `null` during
concatenation, so the unfiltered request `(null, null, 1, 20)` and a request
whose search term happens to be the four characters `null` produced the same
key `page:null:null:1:20`. Whichever ran first populated the `data-sources`
entry and the other was served its page, which for the literal-`null` search
means returning the entire unfiltered inventory instead of the (almost
certainly empty) match set.
The explicit key is dropped so the cache uses Spring's
`SimpleKeyGenerator`. `SimpleKey` holds the arguments and compares with
`Arrays.deepEquals`, so parameter boundaries and null-ness are preserved and
`null` is no longer conflated with `"null"`. The no-argument
`listDataSources()` shares the same cache name but resolves to
`SimpleKey.EMPTY`, which cannot collide with a four-element `SimpleKey`, and
the three `@CacheEvict(allEntries = true)` sites do not depend on the key at
all, so [...]
`SettingsServiceCachingTest` pins both halves: that the two argument shapes
each reach the repository once rather than one being served from the other's
entry, and that the paged result is not shared. The test method was renamed to
end in `Test` to match the convention used throughout the neighbouring
`SettingsServiceTest` and `SettingsControllerTest`.
Closes #4048.
---
.../rocketmq/studio/settings/SettingsService.java | 2 +-
.../settings/SettingsServiceCachingTest.java | 98 ++++++++++++++++++++++
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
index 7847297d4..001d5541f 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
@@ -218,7 +218,7 @@ public class SettingsService {
return settingsRepository.findAllDataSources();
}
- @Cacheable(value = "data-sources", key = "'page:' + #search + ':' + #type
+ ':' + #page + ':' + #pageSize")
+ @Cacheable("data-sources")
public PageResult<DataSourceVO> listDataSources(String search, String
type, int page, int pageSize) {
if (page < 1) {
throw new BusinessException(400, "page must be greater than zero");
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceCachingTest.java
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceCachingTest.java
new file mode 100644
index 000000000..5036057c3
--- /dev/null
+++
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceCachingTest.java
@@ -0,0 +1,98 @@
+/*
+ * 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 com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.rocketmq.studio.audit.OperationAuditService;
+import org.apache.rocketmq.studio.common.domain.PageResult;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.cache.CacheManager;
+import org.springframework.cache.annotation.EnableCaching;
+import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
+import org.springframework.web.client.RestClient;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(SpringExtension.class)
+@ContextConfiguration(classes = SettingsServiceCachingTest.Config.class)
+class SettingsServiceCachingTest {
+
+ @Autowired
+ private SettingsService settingsService;
+
+ @Autowired
+ private SettingsRepository settingsRepository;
+
+ @Autowired
+ private CacheManager cacheManager;
+
+ @BeforeEach
+ void resetState() {
+ reset(settingsRepository);
+ cacheManager.getCache("data-sources").clear();
+ }
+
+ @Test
+ void pagedInventoryShouldDistinguishNullFromLiteralNullSearchTest() {
+ PageResult<DataSourceVO> unfiltered = PageResult.of(
+
List.of(DataSourceVO.builder().key("all").name("All").build()), 1, 1, 20);
+ PageResult<DataSourceVO> literalNull = PageResult.of(
+
List.of(DataSourceVO.builder().key("literal-null").name("Literal
null").build()),
+ 1, 1, 20);
+ when(settingsRepository.findDataSources(null, null, 1,
20)).thenReturn(unfiltered);
+ when(settingsRepository.findDataSources("null", null, 1,
20)).thenReturn(literalNull);
+
+ assertThat(settingsService.listDataSources(null, null, 1,
20)).isSameAs(unfiltered);
+ assertThat(settingsService.listDataSources("null", null, 1,
20)).isSameAs(literalNull);
+ verify(settingsRepository).findDataSources(null, null, 1, 20);
+ verify(settingsRepository).findDataSources("null", null, 1, 20);
+ }
+
+ @Configuration
+ @EnableCaching
+ static class Config {
+
+ @Bean
+ CacheManager cacheManager() {
+ return new ConcurrentMapCacheManager("data-sources");
+ }
+
+ @Bean
+ SettingsRepository settingsRepository() {
+ return mock(SettingsRepository.class);
+ }
+
+ @Bean
+ SettingsService settingsService(SettingsRepository settingsRepository)
{
+ return new SettingsService(settingsRepository,
RestClient.create(), new ObjectMapper(),
+ mock(OperationAuditService.class));
+ }
+ }
+}
