Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3397 . #3415

Open
wants to merge 2 commits into
base: 1.8
Choose a base branch
from
Open

Fix #3397 . #3415

wants to merge 2 commits into from

Conversation

dowenliu-xyz
Copy link
Contributor

Describe what this PR does / why we need it

This PR try to fix #3397

Does this pull request fix one issue?

Yes, #3397

Describe how you did it

Like the solution that fix #3386, the issue is caused by trying locate method with incomplete signature. So I rewrite the register map, with a key type that holds all method signature parts.

Describe how to verify it

Unit tests are provided and passed.

Also, an spring-aop example that fits #3397 case is also provided:

Run module sentinel-demo-annotation-spring-aop, and run following requests (in curl format) to verify that handlers won't conflict with each other. Once the module started, you can call requests in any sequence.

curl -XGET localhost:19966/bar?t=
curl -XGET localhost:19966/foo?t=-1
curl -XGET localhost:19966/count

Special notes for reviews

An spring-aop example that fits alibaba#3397 case is also provided:

Run module sentinel-demo-annotation-spring-aop, and run following
requests (in curl format) to verify that handlers won't conflict with
each other. Once the module started, you can call requests in any
sequence.

curl -XGET localhost:19966/bar?t=
curl -XGET localhost:19966/foo?t=-1
curl -XGET localhost:19966/count
Copy link

codecov bot commented Jun 8, 2024

Codecov Report

Attention: Patch coverage is 82.97872% with 8 lines in your changes missing coverage. Please review.

Project coverage is 45.88%. Comparing base (3892514) to head (3902153).
Report is 2 commits behind head on 1.8.

Files Patch % Lines
...a/csp/sentinel/annotation/aspectj/HandlerMeta.java 60.00% 2 Missing and 6 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                1.8    #3415      +/-   ##
============================================
+ Coverage     45.85%   45.88%   +0.02%     
- Complexity     2148     2151       +3     
============================================
  Files           431      432       +1     
  Lines         12932    12972      +40     
  Branches       1728     1733       +5     
============================================
+ Hits           5930     5952      +22     
- Misses         6301     6316      +15     
- Partials        701      704       +3     
Components Coverage Δ
sentinel-adapter 43.22% <ø> (ø)
sentinel-cluster 23.71% <ø> (ø)
sentinel-core 59.62% <ø> (-0.02%) ⬇️
sentinel-extension 46.08% <82.97%> (+0.20%) ⬆️
sentinel-logging 54.54% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LearningGp LearningGp added kind/bug Category issues or prs related to bug. area/annotation Issues or PRs related to annotation support to-review To review labels Aug 23, 2024
@LearningGp LearningGp self-requested a review August 29, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/annotation Issues or PRs related to annotation support kind/bug Category issues or prs related to bug. to-review To review
Projects
None yet
2 participants