Aias00 commented on PR #7990: URL: https://github.com/apache/shenyu/pull/7990#issuecomment-6356303289
Thanks for completing the guards — `matchRestful`, the `CUSTOM_FLOW` scope and the rule path are all in now, which is exactly what was missing in the first revision. However, **#7984 has converged to the same change**. It gates positive and negative entries for both the selector and the rule cache, and additionally requires the selector set to be path-determined for the rule cache. The two PRs touch the same four call sites in `AbstractShenyuPlugin` and are functionally equivalent, so they conflict textually and only one should land. I have approved #7984 as the one to take: it has the stronger positive/negative control coverage (`...StillAppliesToPathOnlyCandidates` for all four branches) and uses `Objects.equals` rather than an `Integer` comparison. Before it is merged, please consider porting your two genuinely better tests over to #7984: - `restfulSelectorCandidateShouldPreventResultCaching` / `restfulRuleCandidateShouldPreventResultCaching` — the only coverage of the `matchRestful` branch that #7984 added but did not test. - the `verify(candidate, never()).getConditionList()` assertion in `uriOnlySelectorResultsShouldBeCachedAndReused` — it proves the cache is actually *hit*, where #7984 only asserts that the entry exists. With those two ported, #7984 becomes a superset of this PR, so closing this one as a duplicate is the cleanest path. Ping me if you would rather it were the other way round — happy to re-review either direction. -- 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]
