-
Notifications
You must be signed in to change notification settings - Fork 149
Update embedding Membrane Example #2521
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
Conversation
…s to align with latest library versions - Replaced `HttpRouter` with `DefaultRouter` and adjusted lifecycle methods (`start` instead of `init`). - Updated interceptor usage to `getFlow` for improved consistency. - Migrated dependencies to `service-proxy-core` version `7.0.6-SNAPSHOT` and switched logging to Log4j. - Adjusted Maven plugin configurations to use Java 21. - Added roadmap entry for Maven Central publish fix.
📝 WalkthroughWalkthroughThis pull request updates example code and documentation to align with API changes. It migrates two example projects from deprecated logging and router patterns to Log4j2/SLF4J and a new Router interface, updates Maven dependencies, and increases Java compiler target version to 21. A documentation roadmap entry is also added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/examples/extending-membrane/embedding-java/pom.xml (1)
15-16: Java version mismatch with PR objectives.The PR objectives state "Adjust Maven plugin configurations to use Java 21", but the compiler source and target are still set to 1.8. This inconsistency should be resolved.
🔎 Proposed fix to update Java version to 21
- <source>1.8</source> - <target>1.8</target> + <source>21</source> + <target>21</target>
🧹 Nitpick comments (4)
distribution/examples/websockets/custom-websocket-interceptor/embedded/src/main/java/com/predic8/application/MyApplication.java (1)
23-23: Consider using specific imports instead of wildcard.Wildcard imports can lead to namespace pollution and make it unclear which classes are actually being used. Consider replacing with explicit imports:
import com.predic8.membrane.core.Router; import com.predic8.membrane.core.DefaultRouter;distribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/AddMyHeaderInterceptor.java (1)
16-20: Consider using specific imports instead of wildcards.Multiple wildcard imports can lead to namespace conflicts and reduced code clarity. Consider replacing with explicit imports for the classes actually used:
import com.predic8.membrane.core.exchange.Exchange; import com.predic8.membrane.core.interceptor.AbstractInterceptor; import com.predic8.membrane.core.interceptor.Outcome; import org.slf4j.Logger; import org.slf4j.LoggerFactory;distribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/EmbeddingJava.java (1)
16-17: Consider using specific imports instead of wildcards.Wildcard imports reduce code clarity. Consider replacing with explicit imports:
import com.predic8.membrane.core.Router; import com.predic8.membrane.core.DefaultRouter;distribution/examples/extending-membrane/embedding-java/pom.xml (1)
13-13: Consider updating maven-compiler-plugin version.The maven-compiler-plugin version 3.1 is from 2013 and is quite outdated. Consider updating to a more recent version for better Java 21 support and bug fixes.
🔎 Proposed update to a recent version
- <version>3.1</version> + <version>3.13.0</version>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
distribution/examples/extending-membrane/embedding-java/pom.xmldistribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/AddMyHeaderInterceptor.javadistribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/EmbeddingJava.javadistribution/examples/websockets/custom-websocket-interceptor/embedded/pom.xmldistribution/examples/websockets/custom-websocket-interceptor/embedded/src/main/java/com/predic8/application/MyApplication.javadocs/ROADMAP.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-22T19:49:29.473Z
Learnt from: predic8
Repo: membrane/api-gateway PR: 2156
File: pom.xml:0-0
Timestamp: 2025-09-22T19:49:29.473Z
Learning: In the Membrane API Gateway project, jackson-annotations is pinned to version "2.20" instead of using ${jackson.version} (2.20.0) because version 2.20.0 is not available in Maven repository yet for jackson-annotations, while it is available for jackson-core and jackson-databind. This version inconsistency is intentional and necessary due to different release schedules between Jackson modules.
Applied to files:
distribution/examples/websockets/custom-websocket-interceptor/embedded/pom.xmldistribution/examples/extending-membrane/embedding-java/pom.xml
📚 Learning: 2025-12-19T09:01:49.730Z
Learnt from: rrayst
Repo: membrane/api-gateway PR: 2441
File: annot/pom.xml:0-0
Timestamp: 2025-12-19T09:01:49.730Z
Learning: In the annot module (annot/pom.xml), log4j dependencies (log4j-slf4j2-impl and log4j-core) should have <scope>test</scope>, while in the distribution module they should be normal compile-scope dependencies.
Applied to files:
distribution/examples/websockets/custom-websocket-interceptor/embedded/pom.xmldistribution/examples/extending-membrane/embedding-java/pom.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
🔇 Additional comments (9)
docs/ROADMAP.md (1)
17-17: LGTM!The roadmap entry appropriately documents the Maven Central publishing issue under the 7.X section.
distribution/examples/websockets/custom-websocket-interceptor/embedded/src/main/java/com/predic8/application/MyApplication.java (1)
35-35: LGTM! API migration looks correct.The migration from the legacy API to the new Flow-based API is properly implemented:
- Using
getFlow().add()instead ofgetInterceptors().add()- Using
Routerinterface withDefaultRouterimplementation- Using
start()lifecycle method instead ofinit()These changes align with the PR objectives.
Also applies to: 38-38, 41-43
distribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/AddMyHeaderInterceptor.java (1)
28-29: LGTM! Logging migration is correctly implemented.The migration from Lombok's
@Log4j2to SLF4J with parameterized logging is properly implemented and follows best practices. The use of parameterized logging ({}placeholders) improves performance by avoiding unnecessary string concatenation.Also applies to: 33-40
distribution/examples/extending-membrane/embedding-java/src/main/java/com/predic8/membrane/api/EmbeddingJava.java (2)
49-49: LGTM! API migration is correctly implemented.The migration to the new API is properly implemented:
- Using
getFlow().add()for interceptors- Using
Routerinterface withDefaultRouterimplementation- Using
start()lifecycle methodThese changes align with the PR objectives and modernize the example code.
Also applies to: 51-53
46-46: Remove the incorrect protocol change claim.The file is new and was created with HTTPS from the start—there is no protocol change from HTTP to HTTPS. The HTTPS endpoint (
https://api.predic8.de) is accessible and functional. ThegetTarget().setUrl()API is indeed more flexible than separate host/port methods, and Java handles SSL/TLS certificate validation automatically for HTTPS endpoints.Likely an incorrect or invalid review comment.
distribution/examples/websockets/custom-websocket-interceptor/embedded/pom.xml (3)
36-37: Add Java 21 requirement to the README documentation.The pom.xml specifies Java 21 as the compiler target, but this requirement is not mentioned in the embedded example's README.md. The PREREQUISITES section should document the minimum Java version needed to build and run this example.
15-17: Theservice-proxy-coreversion7.0.6-SNAPSHOTis not available in public Maven repositories.The artifact was not found in Sonatype OSS Snapshots or Maven Central. Public releases of
service-proxy-coreonly reach version 6.x. Either:
- Add the appropriate snapshot repository configuration (e.g., Sonatype OSS Snapshots) to the build, or
- Verify the correct version number (consider using a released version like 6.x if 7.0.6 hasn't been publicly released), or
- Ensure this SNAPSHOT is built and available in your internal repository.
Note: Log4j 2.25.3 is a valid release (published December 2025).
18-27: Log4j 2.25.3 is the correct version with CVE-2025-68161 patch applied.Version 2.25.3 fixed CVE-2025-68161 (improper TLS hostname verification in SocketAppender affecting versions 2.0-beta9 through 2.25.2). If SocketAppender or SSL/TLS features are used, ensure outbound logging destinations are verified as trusted.
distribution/examples/extending-membrane/embedding-java/pom.xml (1)
29-36: Log4j 2.25.3 is valid and addresses a known security issue.Version 2.25.3 is a stable release (December 15, 2025) and is the vendor-recommended fix for CVE-2025-68161 (Socket Appender TLS hostname verification vulnerability). No changes needed.
…s to align with latest library versions
HttpRouterwithDefaultRouterand adjusted lifecycle methods (startinstead ofinit).getFlowfor improved consistency.service-proxy-coreversion7.0.6-SNAPSHOTand switched logging to Log4j.Summary by CodeRabbit
Release Notes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.