zhang-arvin commented on PR #7001: URL: https://github.com/apache/shenyu/pull/7001#issuecomment-5886657683
Thanks @Aias00 — you were right, and the compiler confirmed it. On your head I ran `mvn -pl shenyu-admin -am -DskipTests test-compile` and got exactly the three production sites you listed: ``` AiProxyRealKeyResolver.java:[226,66] selectByIdSet — required: (Set<String>, String) found: (Set<String>) RuleServiceImpl.java:[478,77] selectByIdSet — required: (Set<String>, String) found: (HashSet<String>) RuleServiceImpl.java:[525,77] selectByIdSet — required: (Set<String>, String) found: (HashSet<String>) ``` All sites updated, item by item: **Production** - `RuleServiceImpl.java:478` / `:525` — **updated**. Both are `buildRuleDataList` / `buildRuleVOList`, reached from `listAll*()` (all namespaces) as well as `listAllByNamespaceId()`, so no single namespace is in scope there. `RuleDO` carries its own `namespaceId`, so I now group the rules by `NamespaceUtils.normalizeNamespace(ruleDO.getNamespaceId())` and issue one `selectByIdSet(ids, namespaceId)` per namespace. No `null` is passed, and all-namespace listings keep working instead of silently returning empty. - `AiProxyRealKeyResolver.java:226` — **updated** with `Constants.SYS_DEFAULT_NAMESPACE_ID`, not `null`. This resolver is a selectorId-keyed cache shared across callers that have no namespace in scope (`findById` / `findByIds` / `listByPage` / `listAll` / `syncData`), and the ai-proxy keys/selectors are registered and synced in the shared default namespace, so that is the namespace it is actually querying. Documented with inline comments. Happy to thread an explicit `namespaceId` through the resolver API instead if you'd prefer that shape — just say so. **Tests** (all updated so `test-compile` passes) - `SelectorMapperTest:75` — updated. - `SelectorServiceTest:182`, `:198` — updated; the test now stubs and asserts `SYS_DEFAULT_NAMESPACE_ID` explicitly instead of an unbound `any()`. - `RuleServiceTest:392` — updated. - `AiProxyRealKeyResolverTest` — all `selectByIdSet` stubs and verifications updated, including the `never()` ones (now `selectByIdSet(any(), any())`). **Also** - Added the missing `@param namespaceId` Javadoc to `SelectorMapper#selectByIdSet` and `#deleteByIds`. That is the `JavadocMethod` checkstyle failure you flagged on 2026-09-22 — it was failing `pr_build` at the `validate` phase, before compilation even ran, which is why the compile errors were hidden behind it. - Agreed on the unrelated mappers: the other `deleteByIds(List)` hits (`PluginMapper`, `RuleMapper`, `TagMapper`, …) are different methods and left untouched. Head: `7fc1e75` (verified with `git ls-remote`). Local `mvn -pl shenyu-admin -am -DskipTests test-compile` with checkstyle enabled → **BUILD SUCCESS**. One thing I did not add: the cross-namespace assertion test you suggested (a selector in another namespace neither returned nor deleted). Do you want it in this PR or as a follow-up? --- *This comment was generated by an AI agent.* -- 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]
