Skip to content

fix(admin): exclusion search drops plugins matching any searchable field (#7690) - #7691

Merged
Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/excluded-search-and-predicate
Oct 20, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/excluded-search-and-predicate

Conversation

@konlue

@konlue konlue commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #7690.

Modifications

The excluded-search predicates in plugin-sqlmap.xml and namespace-plugin-rel-sqlmap.xml were joined with OR (name NOT LIKE x OR role NOT LIKE x). A row therefore survived whenever either column missed the term, so !divide still returned the divide plugin because its role "Proxy" does not match.

The join is now AND in both mappers, excluding a plugin when any searchable field matches. This is symmetric with the positive keyword search, which already uses OR across the same two columns.

Verifying this change

  • New PluginMapperTest#excludedSearchDropsPluginWhenAnySearchableFieldMatches and NamespacePluginRelMapperTest#excludedSearchDropsRelPluginWhenAnySearchableFieldMatches: seed one plugin whose name matches the exclusion and one whose name does not, then assert the excluded row is dropped and the other retained. The "name matches, role does not" case fails with the previous OR join and passes with this change.

  • Targeted tests pass:

    ./mvnw -B -ntp -pl shenyu-admin test -Dtest=PluginMapperTest,NamespacePluginRelMapperTest

    20/20 green.

Notes

  • Both name and role are NOT NULL in every schema dialect, so nullable-field handling is unchanged.

Checklist

  • I have read the contribution guidelines.
  • Tests back the changes.
  • Full ./mvnw clean install -Dmaven.javadoc.skip=true (targeted reactor tests used instead).

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — exclusion search drops plugins matching any searchable field

Approve. Small, correct, and complete. I verified the fix against the full set of affected mappers rather than just the two changed files.

✅ Verified

The bug and the fix. plugin-sqlmap.xml:255-261:

-- before
AND ( name NOT LIKE '%x%' OR  role NOT LIKE '%x%' )
-- after
AND ( name NOT LIKE '%x%' AND role NOT LIKE '%x%' )

With OR, a row survived whenever either column missed the term, so !divide still returned the divide plugin (name matches, role Proxy does not) — exactly the reported symptom. The AND form is the De Morgan dual of the positive search at lines 250-253 (name LIKE keyword OR role LIKE keyword), which is the right symmetry and confirms your reasoning.

The regression test genuinely fails on master. For keyword !mockExcludedPlugin, BaseExcludedSearchCondition.init() splits it into excluded = "mockExcludedPlugin" and keyword = "" (so the positive-search <if> is skipped). The seeded row (mockExcludedPlugin / Proxy) then evaluates name NOT LIKE ... = false OR role NOT LIKE ... = true → survives on master, so assertTrue(noneMatch(...)) fails; with AND it evaluates false AND true → dropped. Good test.

init() being called twice is safe. The test calls condition.init() explicitly and PluginServiceImpl.searchByCondition:93 calls it again. On the second pass getKeyword() is "", so StringUtils.isNotBlank short-circuits and excluded is preserved. Worth knowing this is deliberate — a future change to init() that isn't idempotent would silently break both tests.

No sibling bug left behind. I checked every mapper with an excluded bind. Only two have a multi-column NOT LIKE join — the two you fixed. selector-sqlmap.xml:242 and rule-sqlmap.xml:182 filter on a single column (selector_name / rule_name), so no OR/AND question arises; meta-data-sqlmap.xml and operation-record-log-sqlmap.xml contain no NOT LIKE at all. The fix is exhaustive.

Schema claim checks out. H2 plugin has name varchar(62) NOT NULL and role varchar(64) NOT NULL (sql-script/h2/schema.sql:34,36), so neither column can be null and no nullable-handling change is needed. The PluginDO builders in both tests populate every NOT NULL column without a default.

Test assertions are robust. List_Column_List (plugin-sqlmap.xml:38-47) includes name, so PluginListVO.getName() is populated. In the namespace test, searchByCondition LEFT JOINs plugin and maps p.NAME AS NAME (namespace-plugin-rel-sqlmap.xml:168-170), so NamespacePluginVO.getName() is valid too. Both assertions are scoped to the test's own rows, so they don't depend on what else is in the table.

Rollback works. AbstractSpringIntegrationTest is @ActiveProfiles("h2") + @SpringBootTest, and both new tests carry @Transactional, so TransactionalTestExecutionListener rolls the inserts back. No cross-test pollution.


📝 Non-blocking

1. The symmetric case isn't pinned. You test "name matches, role does not". The other half — "role matches, name does not" — is equally affected by the OR bug on master and equally fixed by AND, but nothing asserts it. A row like id=excluded-role-match, name=unrelated, role=mockExcludedPlugin would close that gap cheaply and make the test express the full "any searchable field" contract the title claims.

2. Cosmetic: namespace-plugin-rel-sqlmap.xml:183 now reads AND p.role with the original double space. It matches the file's existing style (line 176 has OR p.role), so this is fine — just noting it in case you want to normalize while you're in there.


CI

Still running — 10 passed, 4 pending (including Analyze (java)), 3 skipped, no failures. The targeted PluginMapperTest / NamespacePluginRelMapperTest runs are what matter for this change and are covered by the pending Analyze job.

Summary

Approve. The diagnosis is right, the OR→AND fix is the correct minimal change, the regression test fails on master and passes after, and I confirmed no other mapper carries the same defect. The one suggestion above is test coverage, not a code issue.

@Aias00
Aias00 merged commit 85cbda4 into apache:master Oct 20, 2026
73 of 76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Plugin exclusion search retains matches because negative predicates use OR

2 participants