lizhimins commented on PR #4132: URL: https://github.com/apache/rocketmq-dashboard/pull/4132#issuecomment-5602329614
Closing as a duplicate of #4049, which was merged into `rocketmq-studio` as c6b546a7. The production fixes are byte-identical, not merely equivalent. Both PRs take `SettingsService.java` from blob `7847297d45` to blob `001d5541f4` — the same single-line change, dropping the explicit SpEL `key` from `@Cacheable` on the paged `listDataSources` so Spring's `SimpleKeyGenerator` handles it and `null` is no longer conflated with the literal string `"null"`. #4049 is the one that landed because it was opened roughly 25 hours earlier and had already been marked ready for review, so it is first past the post. Your diagnosis was independently correct — an identical production change is the strongest possible evidence of that. On the tests: yours boots the full application context (`@SpringBootTest` with `spring.cache.type=simple`) while #4049's uses a sliced `@ContextConfiguration` over a nested `@EnableCaching` config with an autowired `SettingsService`. We looked at whether merging #4049 lost anything by preferring the lighter test, and it did not — both exercise a real caching proxy rather than a hand-constructed service, and we verified #4049's case is sensitive to the fix by restoring the old SpEL key and confirming it fails. Yours asserts the same two properties (distinct returned objects, `times(1)` per argument shape). The sliced version is the one we would write for this, since it pins the cache key without paying for a full context startup, but that is a preference rather than a deficiency in your approach. One process note for future PRs: this one declared `Fixes #4048`, but that issue was filed by the author of #4049 to describe their own finding. Linking `Fixes` to someone else's issue attaches your PR to their report and, on a repository whose default branch is not the merge target, can leave the issue in an ambiguous state. If you independently find the same bug, opening your own issue or simply describing the defect in the PR body works better. Thank you for the fix — the defect was real and is now closed on the branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
