refactor code #2033
refactor code #2033
Conversation
|
Warning Rate limit exceeded@predic8 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis change removes the utility methods for creating HTTP requests from Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase
participant RequestBuilder
participant Exchange
TestCase->>RequestBuilder: Request.get(uri).build()
RequestBuilder-->>TestCase: Request object
TestCase->>Exchange: setRequest(Request)
sequenceDiagram
participant CoreClass
participant RequestBuilder
CoreClass->>RequestBuilder: Request.post(uri).build()
RequestBuilder-->>CoreClass: Request object
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request needs "/ok-to-test" from an authorized committer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java (1)
82-88: Nitpick: Fix typo in method nameThe method name
getStandartRequestshould begetStandardRequest(missing 'd' in "Standard").-private static Request getStandartRequest(String method) { +private static Request getStandardRequest(String method) {core/src/test/java/com/predic8/membrane/core/interceptor/DispatchingInterceptorTest.java (1)
16-27: Consider updating to explicit imports for consistencyOther files in this PR were updated to use explicit imports instead of wildcard imports. Consider updating this file for consistency.
core/src/test/java/com/predic8/membrane/core/interceptor/rest/HTTP2XMLInterceptorTest.java (2)
43-43: LGTM! Correct implementation of Request.Builder patternThe replacement correctly implements the new builder pattern, though using the full package name is unusual.
Consider adding an explicit import for
Requestand using the shorter form for consistency with other files:+import com.predic8.membrane.core.http.Request; ... -exc.setRequest(new com.predic8.membrane.core.http.Request.Builder().get("http://localhost/axis2/services/BLZService?wsdl").build()); +exc.setRequest(new Request.Builder().get("http://localhost/axis2/services/BLZService?wsdl").build());
16-28: Consider updating to explicit imports for consistencyOther files in this PR were updated to use explicit imports instead of wildcard imports. Consider updating this file for consistency with the refactoring pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
core/src/main/java/com/predic8/membrane/balancer/client/LBNotificationClient.java(2 hunks)core/src/main/java/com/predic8/membrane/core/interceptor/WSDLInterceptor.java(1 hunks)core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java(1 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/DispatchingInterceptorTest.java(2 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/WADLInterceptorTest.java(2 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/WSDLInterceptorTest.java(2 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/formvalidation/FormValidationInterceptorTest.java(2 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/rest/HTTP2XMLInterceptorTest.java(1 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/rewrite/RewriteInterceptorTest.java(3 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptorTest.java(1 hunks)core/src/test/java/com/predic8/membrane/core/interceptor/xmlprotection/XMLProtectionInterceptorTest.java(2 hunks)core/src/test/java/com/predic8/membrane/interceptor/ws_addressing/WsaEndpointRewriterInterceptorTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rrayst
PR: membrane/api-gateway#1906
File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0
Timestamp: 2025-06-18T12:03:09.931Z
Learning: The mediaType method in the Request class throws jakarta.mail.internet.ParseException, not java.text.ParseException, making the jakarta.mail.internet.ParseException import necessary and correct in JsonSchemaTestSuiteTests.java.
Learnt from: rrayst
PR: membrane/api-gateway#1906
File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0
Timestamp: 2025-06-18T12:03:09.931Z
Learning: The mediaType method in the Message class (which Request extends) throws jakarta.mail.internet.ParseException because it creates a ContentType object for parsing MIME media types. The jakarta.mail.internet.ParseException import is correct and necessary in test files that call mediaType().
📚 Learning: the mediatype method in the request class throws jakarta.mail.internet.parseexception, not java.text...
Learnt from: rrayst
PR: membrane/api-gateway#1906
File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0
Timestamp: 2025-06-18T12:03:09.931Z
Learning: The mediaType method in the Request class throws jakarta.mail.internet.ParseException, not java.text.ParseException, making the jakarta.mail.internet.ParseException import necessary and correct in JsonSchemaTestSuiteTests.java.
Applied to files:
core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/DispatchingInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/xmlprotection/XMLProtectionInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/rest/HTTP2XMLInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/WSDLInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/formvalidation/FormValidationInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/rewrite/RewriteInterceptorTest.javacore/src/main/java/com/predic8/membrane/core/util/MessageUtil.javacore/src/main/java/com/predic8/membrane/core/interceptor/WSDLInterceptor.javacore/src/test/java/com/predic8/membrane/interceptor/ws_addressing/WsaEndpointRewriterInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/WADLInterceptorTest.java
📚 Learning: the mediatype method in the message class (which request extends) throws jakarta.mail.internet.parse...
Learnt from: rrayst
PR: membrane/api-gateway#1906
File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0
Timestamp: 2025-06-18T12:03:09.931Z
Learning: The mediaType method in the Message class (which Request extends) throws jakarta.mail.internet.ParseException because it creates a ContentType object for parsing MIME media types. The jakarta.mail.internet.ParseException import is correct and necessary in test files that call mediaType().
Applied to files:
core/src/test/java/com/predic8/membrane/core/interceptor/rest/HTTP2XMLInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/WSDLInterceptorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/formvalidation/FormValidationInterceptorTest.javacore/src/main/java/com/predic8/membrane/core/util/MessageUtil.javacore/src/test/java/com/predic8/membrane/core/interceptor/WADLInterceptorTest.java
🔇 Additional comments (26)
core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java (1)
16-32: LGTM! Import statements properly refinedThe changes correctly replace wildcard imports with explicit imports and reduce static imports to only what's needed. This improves code clarity and reduces namespace pollution.
core/src/test/java/com/predic8/membrane/interceptor/ws_addressing/WsaEndpointRewriterInterceptorTest.java (2)
16-26: LGTM! Import statements properly refinedThe explicit imports improve code clarity and maintainability compared to wildcard imports.
44-44: LGTM! Correct implementation of Request.Builder patternThe replacement of
MessageUtil.getPostRequest()withnew Request.Builder().post(...).build()is correctly implemented and maintains the same functionality.core/src/test/java/com/predic8/membrane/core/interceptor/DispatchingInterceptorTest.java (3)
47-47: LGTM! Correct implementation of Request.Builder patternThe replacement of
MessageUtil.getGetRequest()withnew Request.Builder().get(...).build()is correctly implemented.
60-60: LGTM! Correct implementation of Request.Builder patternThe replacement maintains the same functionality with the new builder pattern.
65-65: Clarify the purpose of this commentThe comment "//dont work!!!!" suggests there's a known issue with this test. Please clarify what doesn't work or remove the comment if it's no longer relevant.
core/src/test/java/com/predic8/membrane/core/interceptor/xmlprotection/XMLProtectionInterceptorTest.java (2)
17-28: LGTM! Import statements properly refinedThe explicit imports improve code maintainability and reduce namespace pollution.
37-37: LGTM! Correct implementation of Request.Builder patternThe replacement of
MessageUtil.getGetRequest()withnew Request.Builder().get(...).build()maintains the same functionality with the new builder pattern.core/src/test/java/com/predic8/membrane/core/interceptor/WSDLInterceptorTest.java (2)
17-17: LGTM: Proper import added for Request class.The import addition aligns with the refactoring to use Request.Builder directly.
41-43: Add URISyntaxException to setUp method signature.The Request.Builder().get() method can throw URISyntaxException, but the setUp method doesn't declare this exception.
@BeforeEach -public void setUp() throws Exception { +public void setUp() throws Exception {Actually, since setUp already throws Exception, URISyntaxException is already covered. The refactoring from MessageUtil.getGetRequest() to Request.Builder pattern is correctly implemented.
⛔ Skipped due to learnings
Learnt from: rrayst PR: membrane/api-gateway#1906 File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0 Timestamp: 2025-06-18T12:03:09.931Z Learning: The mediaType method in the Request class throws jakarta.mail.internet.ParseException, not java.text.ParseException, making the jakarta.mail.internet.ParseException import necessary and correct in JsonSchemaTestSuiteTests.java.core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptorTest.java (2)
17-35: LGTM: Import statements cleaned up properly.The refactoring from wildcard imports to explicit imports improves code clarity and follows best practices.
58-63: LGTM: Proper exception handling and Request.Builder usage.The method signature correctly declares URISyntaxException, and the replacement of MessageUtil.getPostRequest with Request.Builder().post().build() is implemented consistently.
core/src/main/java/com/predic8/membrane/balancer/client/LBNotificationClient.java (2)
23-37: LGTM: Import statements properly organized.The import reorganization improves code structure and readability.
81-81: LGTM: Correct refactoring to Request.Builder pattern.The replacement of MessageUtil.getPostRequest() with Request.Builder().post().build() maintains the same functionality while using the modern builder API.
core/src/main/java/com/predic8/membrane/core/interceptor/WSDLInterceptor.java (2)
116-116: LGTM: Proper exception declaration added.The method signature correctly declares URISyntaxException, which can be thrown by Request.Builder().get().
118-118: LGTM: Correct refactoring to Request.Builder pattern.The replacement of MessageUtil.getGetRequest() with Request.Builder().get().build() maintains the same functionality while using the modern builder API.
core/src/test/java/com/predic8/membrane/core/interceptor/formvalidation/FormValidationInterceptorTest.java (2)
18-18: LGTM: Proper imports added.The imports for URISyntaxException and Request are correctly added to support the refactoring.
Also applies to: 23-23
65-67: LGTM: Proper exception handling and Request.Builder usage.The method signature correctly declares URISyntaxException, and the replacement of MessageUtil.getGetRequest() with Request.Builder().get().build() is implemented correctly.
core/src/test/java/com/predic8/membrane/core/interceptor/WADLInterceptorTest.java (4)
20-20: LGTM! Import addition is correct.The
URISyntaxExceptionimport is necessary for the newRequest.Builderpattern and is properly placed.
27-27: LGTM! Import addition is correct.The
Requestimport is necessary for the newRequest.Builderpattern and is properly placed.
116-116: LGTM! Exception declaration is correct.The method signature correctly declares
URISyntaxExceptionwhich is thrown by theRequest.Builder().get().build()pattern.
118-118: LGTM! Request construction correctly updated.The change from
MessageUtil.getGetRequest()toRequest.Builder().get().build()is correct and maintains the same functionality while using the new builder pattern.core/src/test/java/com/predic8/membrane/core/interceptor/rewrite/RewriteInterceptorTest.java (4)
16-35: LGTM! Import reorganization is clean.The imports are properly organized and all necessary imports for the refactored code are included.
68-69: LGTM! Test method correctly updated.The method signature and request construction are properly updated to use the new
Request.Builderpattern.
76-77: LGTM! Test method correctly updated.The method signature and request construction are properly updated to use the new
Request.Builderpattern.
86-87: LGTM! Test method correctly updated.The method signature and request construction are properly updated to use the new
Request.Builderpattern.
core/src/test/java/com/predic8/membrane/core/interceptor/rewrite/RewriteInterceptorTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/com/predic8/membrane/core/interceptor/rewrite/RewriteInterceptorTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/predic8/membrane/balancer/client/LBNotificationClient.java
Outdated
Show resolved
Hide resolved
2 participants
Summary by CodeRabbit
Refactor
Tests