Fix: RuleServiceImpl#searchByPage() Selectors now populated from namespace only when condition selectors are null or empty.

[bruce121]

This commit optimizes the selector loading logic within the RuleServiceImpl#searchByPage() method.

Fixes #6304

Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

hi, pls add my wechat: aias00

[Copilot]

Pull request overview

Fixes a bug in RuleServiceImpl#searchByPage() where a caller-provided selector filter could be unintentionally broadened to all selectors in the namespace, leading to incorrect rule results (e.g., KeyAuth page returning Divide rules).

Changes:

• Update searchByPage() to populate selectors from namespace only when condition.getSelectors() is null/empty.
• Update existing testSearchByPage expectations to reflect the corrected selector handling.
• Add targeted test coverage for “specified selectors preserved” and “empty selectors expanded from namespace” scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java	Only loads namespace selectors when the incoming selector filter is null/empty, preventing unintended query broadening.
shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java	Adjusts selector expectations and adds tests to validate the corrected searchByPage() behavior.

Comments suppressed due to low confidence (1)

shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java:342

• The helper name testSelectorMergeScenario is now misleading: the updated searchByPage behavior no longer merges selector lists when a selector filter is provided. Renaming this helper (and/or its parameters like namespaceSelectorDOs) to reflect the current “populate selectors when empty” behavior would make the test intent clearer.

    private void testSelectorMergeScenario(final List<String> userSelectors,
                                           final List<SelectorDO> namespaceSelectorDOs,
                                           final int expectedSize,

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In testSearchByPageWithSpecifiedSelectors, the test stubs selectorMapper.selectAllByNamespaceId(...) even though the new behavior/optimization is that this call should not happen when selectors are already provided. Consider removing the unused stubbing and adding an explicit verify(..., never()) to ensure the namespace selector query is not triggered in this scenario (otherwise a performance regression could slip through while still passing assertions).

[Aias00]

LGTM

[Aias00]

hi, pls add my wechat: aias00