-
Notifications
You must be signed in to change notification settings - Fork 47
Feat/GCI604-ELAN-2025 #114
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
base: main
Are you sure you want to change the base?
Feat/GCI604-ELAN-2025 #114
Conversation
|
This PR has been automatically marked as stale because it has no activity for 30 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Sonar check to enforce optimized Spring @Retryable parameters and adds tests to verify compliant and noncompliant usages.
- Implement
SpringMaxRetryableCheckto flag excessivemaxAttemptsor total backoff timeout - Add test input file with compliant/noncompliant examples for
@Retryable - Add
SpringMaxRetryableTestJUnit class to verify the new rule
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/java/org/greencodeinitiative/creedengo/java/checks/SpringMaxRetryableCheck.java | New rule implementation enforcing maxAttempts ≤ 3 and total timeout ≤ 5000ms |
| src/test/files/SpringMaxRetryableCheck.java | Test file with examples of compliant and noncompliant @Retryable |
| src/test/java/org/greencodeinitiative/creedengo/java/checks/SpringMaxRetryableTest.java | JUnit verifier for the new rule |
Comments suppressed due to low confidence (1)
src/test/java/org/greencodeinitiative/creedengo/java/checks/SpringMaxRetryableTest.java:29
- [nitpick] The test method name
testis too generic. Rename it to something more descriptive, e.g.,verifyRetryableParametersOptimization.
void test() {
| if (isGreaterThanMax(maxAttempts, delay, multiplier)) { | ||
| reportIssue(params.get(0).getArgument(), MESSAGE_RULE); | ||
| } | ||
| } | ||
|
|
||
| public boolean isGreaterThanMax(Integer maxAttempts, Long delay, Double multiplier) { | ||
| return (calculateRetryTimeout(maxAttempts, delay, multiplier) > MAX_TIMEOUT) || maxAttempts > MAX_RETRY; |
Copilot
AI
Jul 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporting always uses the first parameter, which may not be the one exceeding the limit. Consider tracking which argument caused isGreaterThanMax and report the issue on that specific ExpressionTree.
| if (isGreaterThanMax(maxAttempts, delay, multiplier)) { | |
| reportIssue(params.get(0).getArgument(), MESSAGE_RULE); | |
| } | |
| } | |
| public boolean isGreaterThanMax(Integer maxAttempts, Long delay, Double multiplier) { | |
| return (calculateRetryTimeout(maxAttempts, delay, multiplier) > MAX_TIMEOUT) || maxAttempts > MAX_RETRY; | |
| List<String> violatingParams = isGreaterThanMax(maxAttempts, delay, multiplier); | |
| if (!violatingParams.isEmpty()) { | |
| for (String paramName : violatingParams) { | |
| params.stream() | |
| .filter(argumentDetails -> argumentDetails.paramName.equals(paramName)) | |
| .findFirst() | |
| .ifPresent(argumentDetails -> reportIssue(argumentDetails.getArgument(), MESSAGE_RULE)); | |
| } | |
| } | |
| } | |
| public List<String> isGreaterThanMax(Integer maxAttempts, Long delay, Double multiplier) { | |
| List<String> violatingParams = new ArrayList<>(); | |
| if (calculateRetryTimeout(maxAttempts, delay, multiplier) > MAX_TIMEOUT) { | |
| violatingParams.add("backoff.delay or backoff.multiplier"); | |
| } | |
| if (maxAttempts > MAX_RETRY) { | |
| violatingParams.add("maxAttempts"); | |
| } | |
| return violatingParams; |
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
|
|
Copilot
AI
Jul 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These imports (Arrays, Collection, Collections) are not used in the test file and can be removed to clean up unused dependencies.
| import java.util.Arrays; | |
| import java.util.Collection; | |
| import java.util.Collections; |
|
implementation for issue : green-code-initiative/creedengo-rules-specifications#397 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waiting for taking into account the review feedbacks of green-code-initiative/creedengo-rules-specifications#397
|
This PR has been automatically marked as stale because it has no activity for 30 days. |
No description provided.