Repository navigation
fix(admin): exclusion search drops plugins matching any searchable field (#7690) - #7691
Conversation
Aias00
left a comment
There was a problem hiding this comment.
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.
Fixes #7690.
Modifications
The excluded-search predicates in
plugin-sqlmap.xmlandnamespace-plugin-rel-sqlmap.xmlwere joined withOR(name NOT LIKE x OR role NOT LIKE x). A row therefore survived whenever either column missed the term, so!dividestill returned the divide plugin because its role"Proxy"does not match.The join is now
ANDin both mappers, excluding a plugin when any searchable field matches. This is symmetric with the positive keyword search, which already usesORacross the same two columns.Verifying this change
New
PluginMapperTest#excludedSearchDropsPluginWhenAnySearchableFieldMatchesandNamespacePluginRelMapperTest#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 previousORjoin and passes with this change.Targeted tests pass:
./mvnw -B -ntp -pl shenyu-admin test -Dtest=PluginMapperTest,NamespacePluginRelMapperTest20/20 green.
Notes
nameandroleare NOT NULL in every schema dialect, so nullable-field handling is unchanged.Checklist
./mvnw clean install -Dmaven.javadoc.skip=true(targeted reactor tests used instead).