imDwAaY opened a new pull request, #8031:
URL: https://github.com/apache/shenyu/pull/8031
Fixes #8024.
### Problem
Rule updates are delivered through `CommonPluginDataSubscriber.handlerRule`,
not `removeRule`. `handlerRule` overwrites the stored snapshot and then calls
`registerTool`, which returns immediately when `enabled=false`. A tool
registered while the rule was enabled therefore stays in the SDK server and in
`CACHED_TOOL` after the rule is disabled, and its retained callback keeps
invoking the backend. Since the previous snapshot is discarded before the new
one is stored, nothing later knows what had been registered.
### Fix
- `handlerRule` keeps the previous snapshot for the rule id and, on an
`enabled -> disabled` transition, revokes the registration from that previous
snapshot (old selector server path and the effective tool name derived from its
handle).
- Revocation removes the `CACHED_TOOL` entry and calls
`ShenyuMcpServerManager.removeTool` on the server, then invalidates
`MetaDataCache` like the other mutation paths do.
- A rule that was already disabled is not revoked again on duplicate
updates, so repeated sync events do not repeatedly hit the SDK.
- A rule that was never active has no previous snapshot and is unaffected.
Disabled snapshots stay in the handler state, so a later selector replay still
skips them and a re-enable registers the tool exactly once.
- `removeRule` now shares the extracted `unregisterTool`, and the
effective-name expression is shared through `effectiveToolName`.
Renaming a rule while it stays enabled is the separate issue #8025 and is
not changed here.
### Tests
Added to `McpServerPluginDataHandlerTest`:
- `disablingAnActiveRuleRevokesTheRegisteredTool` — an enabled rule is
registered, then disabled: `removeTool("/restart", "restart-tool")` is called
and the cached handle is gone.
- `reEnablingARuleRegistersTheToolExactlyOnce` — duplicate disabled updates
revoke only once, and re-enabling registers the tool exactly one more time with
the cache restored.
Both tests fail against the previous implementation and pass with the change.
Local verification (JDK 17, as required by the build):
```
./mvnw -pl shenyu-plugin/shenyu-plugin-mcp-server test # 149 tests, 0
failures
./mvnw -pl shenyu-plugin/shenyu-plugin-mcp-server checkstyle:check
```
--
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]
