Setting up Activemq #128
Setting up Activemq #128
Conversation
WalkthroughThis pull request integrates ActiveMQ messaging into the project. It adds several dependencies for ActiveMQ and Spring JMS to the Maven configuration, establishes a new JMS configuration class with necessary beans, and introduces a serializable report message class. Additionally, job classes have been updated to publish JMS messages rather than performing direct report generation or file operations. A new JMS listener handles incoming messages for report generation and notifications, and a utility method is added for centralized property retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant Job as Job Class
participant JMS as JMS Broker
participant Listener as ReportGenerationListener
participant Service as Report/Target Service
Job->>JMS: Publish JMS message (report event or target file notification)
JMS->>Listener: Deliver JMS message
Listener->>Service: Invoke report generation or file processing
Service-->>Listener: Return processing result
Listener-->>JMS: Log acknowledgement via listener
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
dinesh-beehyv
requested review from
Kantas2601,
Mandar-Beehyv and
atishbeehyv123
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/utils/Global.java (1)
23-23: 🛠️ Refactor suggestionReplace hardcoded paths with configurable property paths.
Multiple methods use hardcoded paths to property files which could cause deployment issues in different environments.
Consider:
- Moving property file paths to a configuration file or environment variables
- Creating constants for commonly used paths
- Using Spring's resource loading mechanisms
private static final String APP_PROPERTIES_PATH = "../webapps/NMSReportingSuite/WEB-INF/classes/app.properties"; private static final String SMS_PROPERTIES_PATH = "../webapps/NMSReportingSuite/WEB-INF/classes/sms.properties";Also applies to: 48-48, 90-90, 162-162, 185-185
🧹 Nitpick comments (11)
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java (3)
18-25: Consider using constructor injection instead of field injection.Field injection using
@Autowiredis discouraged as it makes the class harder to test and hides dependencies. Constructor injection is the recommended approach as it makes dependencies explicit and ensures they are provided at instantiation time.Apply this diff to switch to constructor injection:
- @Autowired - private AdminService adminService; - - @Autowired - SmsNotificationService smsNotificationService; - - @Autowired - AshaTargetFileService ashaTargetFileService; + private final AdminService adminService; + private final SmsNotificationService smsNotificationService; + private final AshaTargetFileService ashaTargetFileService; + + @Autowired + public ReportGenerationListener(AdminService adminService, + SmsNotificationService smsNotificationService, + AshaTargetFileService ashaTargetFileService) { + this.adminService = adminService; + this.smsNotificationService = smsNotificationService; + this.ashaTargetFileService = ashaTargetFileService; + }
88-88: Fix typo in method name.The method name
porcessKilkariSixWeekNoAnswerFilescontains a typo.Apply this diff to fix the typo:
- adminService.porcessKilkariSixWeekNoAnswerFiles(message.getFromDate(), message.getToDate()); + adminService.processKilkariSixWeekNoAnswerFiles(message.getFromDate(), message.getToDate());
1-144: Consider implementing a dead letter queue for failed messages.To improve reliability and error handling, consider implementing a dead letter queue (DLQ) pattern. This would allow failed messages to be moved to a separate queue for later analysis and potential reprocessing, rather than being lost when exceptions occur.
Key benefits:
- Prevents message loss during processing failures
- Enables monitoring of failed messages
- Allows for message replay after fixing issues
- Provides better visibility into system health
This can be configured in your ActiveMQ configuration:
@Bean public ActiveMQQueue dlq() { return new ActiveMQQueue("DLQ"); } @Bean public RedeliveryPolicy redeliveryPolicy() { RedeliveryPolicy policy = new RedeliveryPolicy(); policy.setMaximumRedeliveries(3); policy.setInitialRedeliveryDelay(1000L); policy.setBackOffMultiplier(2); return policy; }NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java (2)
17-21: Consider removing unused service dependencies.The
smsNotificationServiceandashaTargetFileServiceappear to be unused after migrating to JMS messaging. Consider removing these dependencies if they are no longer needed.- @Autowired - SmsNotificationService smsNotificationService; - - @Autowired - AshaTargetFileService ashaTargetFileService;
34-36: Enhance error handling with specific exception types.The current error handling catches all exceptions. Consider catching and handling specific JMS-related exceptions for better error handling and recovery.
- } catch (Exception e) { - LOGGER.error("Failed to publish target file processing event: {}", e.getMessage(), e); + } catch (JmsException e) { + LOGGER.error("JMS error while publishing target file event: {}", e.getMessage(), e); + // Consider implementing retry logic or fallback mechanism + } catch (Exception e) { + LOGGER.error("Unexpected error while publishing target file event: {}", e.getMessage(), e);NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AutoReportEmailGeneration.java (4)
13-17: Remove unused imports for cleaner code.The imports for
Destination(line 13) andQueue(line 17) appear unused. Eliminating them cleans the codebase and reduces mental overhead.-import javax.jms.Destination; ... -import java.util.Queue;
76-85: Reduce repetitive event publishing calls.All calls to
publishReportEvent(...)are structurally similar. A loop or mapping table could reduce repetition and centralize maintenance.- publishReportEvent("Mother_Rejected", "mother-rejected", fromDate, toDate, false); - publishReportEvent("Child_Rejected", "child-rejected", fromDate, toDate, false); ... + String[][] events = { + {"Mother_Rejected", "mother-rejected"}, + {"Child_Rejected", "child-rejected"}, + ... + }; + for(String[] e : events){ + publishReportEvent(e[0], e[1], fromDate, toDate, false); + }
95-103: Consider handling message publish failures.Currently, failures are only logged. If guaranteed delivery is crucial, you might consider a retry mechanism or error queue to ensure unprocessed messages are not lost.
107-134: Consolidate repeated logic for weekly vs. monthly generation.The weekly routine largely duplicates the process from
executeInternal(). A shared method could handle date calculations and event publishing to minimize code duplication.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/entity/ReportMessage.java (1)
1-54: Favor newer time/date APIs for clarity.Using
java.util.Datecan lead to confusion with time-zone or day boundaries. If feasible, considerjava.time.LocalDateorZonedDateTimeto better express date/time intent and reduce edge cases.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java (1)
58-66: Review concurrency settings for production load.Current concurrency range (
1-5) might be sufficient for development but may need tuning in production. Ensure the broker and system resources can handle the desired throughput.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
NMSReportingSuite/pom.xml(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/entity/ReportMessage.java(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java(2 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AutoReportEmailGeneration.java(3 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/utils/Global.java(1 hunks)
🔇 Additional comments (6)
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java (2)
9-9: LGTM!The JmsTemplate import is correctly added for the new messaging functionality.
26-38: Verify JMS configuration and queue naming consistency.Let's verify that:
- The ActiveMQ configuration is properly set up
- The queue name "target-file-queue" is consistently used across the application
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for queue name usage and JMS configuration echo "Checking queue name usage:" rg -l "target-file-queue" echo -e "\nChecking ActiveMQ configuration:" fd -e java -e xml -e properties -e yaml | xargs rg -l "activemq|jms"Length of output: 826
JMS Configuration and Queue Naming Verified
The search results confirm that the "target-file-queue" is consistently used in both
AshaSmsAutoTargetFileGeneration.javaandReportGenerationListener.java, and the ActiveMQ/JMS configurations are present inActiveMQConfig.java(as well as referenced in related files). This indicates that the JMS configuration is properly set up and the queue naming is consistent.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AutoReportEmailGeneration.java (1)
42-45: Returning early when auto-generation is disabled is a solid safeguard.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java (2)
25-25: Good practice enabling JMS and scanning for listeners.
31-43: Validate security of trusted packages.Narrowing the trusted package list is a key security measure. Ensure that these specific packages reflect all classes actually needed for deserialization, preventing potential RCE attacks.
NMSReportingSuite/pom.xml (1)
11-14: Consider upgrading Spring Framework version.The project is using Spring Framework 4.2.5.RELEASE which is quite old. Consider upgrading to a newer version for better security and features.
✅ Verification successful
🌐 Web query:
What is the latest stable version of Spring Framework 4.x and what are the key security improvements since 4.2.5?💡 Result:
The latest stable version of Spring Framework 4.x is 4.3.x, specifically 4.3.30 (the final release before its end-of-life on December 31, 2020)[1][4][7]. However, this version is no longer officially supported by the Spring team, and no further security patches are provided[1][6].
Key Security Improvements Since 4.2.5
While explicit security bulletins for older 4.x versions are not detailed in the search results, several critical dependency and configuration changes were introduced in later 4.x releases:
Version Range Security-Related Changes 4.1+ - Enforced minimum dependencies:
• Quartz ≥2.1.4 (replacing vulnerable Quartz ≤1.8)[1]
• Apache HttpClient ≥4.3 (addressing CVE-2012-6153 and other HTTP client vulnerabilities)[1]
- Servlet API validation improvements4.3+ - Extended security support for Tomcat ≥8.5+ and WebSphere ≥8.5+[1]
- Stronger validation of serialization/deserialization processes
- Deprecated insecure legacy XML configurationsThese changes mitigated risks like:
- Remote code execution via outdated Quartz schedulers
- HTTP request smuggling vulnerabilities in older HttpClient versions
- Session fixation attacks through servlet container alignment[1][10].
Critical Considerations
Spring Framework 4.x has been unsupported since December 2020[1][6], leaving applications vulnerable to unpatched CVEs like:
- CVE-2022-22965 ("SpringShell" RCE) - affects only Spring Framework ≥5.x[2]
- Potential XSS and CSRF vulnerabilities addressed in later framework generations.
Modern Spring applications should migrate to Spring Framework ≥6.x (requiring JDK17+), which includes enhanced security features like OAuth2 integration and virtual thread hardening[3][7][9]. For legacy systems stuck on Spring 4.x, third-party vendors like HeroDevs offer extended support options[6].
Citations:
- 1: https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-4.x
- 2: https://www.darkreading.com/threat-intelligence/upgrades-for-spring-framework-have-stalled
- 3: https://www.codejava.net/frameworks/spring-boot/spring-boot-version-history
- 4: https://github.com/spring-projects/spring-framework/wiki/Spring-Framework-Versions
- 5: https://docs.spring.io/spring-security/site/docs/4.0.1.RELEASE/reference/htmlsingle/
- 6: https://www.herodevs.com/blog-posts/navigating-the-end-of-life-for-spring-framework-5-3-and-6-what-you-need-to-know
- 7: https://www.codejava.net/frameworks/spring/spring-framework-version-history
- 8: https://docs.spring.io/spring-security/site/docs/4.0.x/reference/html/index.html
- 9: https://www.marcobehler.com/guides/spring-and-spring-boot-versions
- 10: https://www.dineshonjava.com/whats-new-in-spring-framework-4x/
Spring Framework Version Upgrade Recommendation Validated
The web research confirms that the project’s current Spring Framework version (4.2.5.RELEASE) is indeed outdated. The latest stable release in the 4.x series is 4.3.30, which incorporates several security-related improvements (e.g., updated dependencies and configuration enhancements). However, since the entire 4.x line is no longer supported as of December 2020, it's highly advisable to plan a migration to a fully supported version (such as Spring Framework 6) to ensure continued security and access to critical updates.
- Location:
NMSReportingSuite/pom.xml(lines 11-14)- Action: Consider updating at least to Spring 4.3.30 if immediate changes are needed; however, a more robust long-term solution would be migrating to a currently supported Spring Framework version.
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java
Show resolved
Hide resolved
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/utils/Global.java
Outdated
Show resolved
Hide resolved
….9 to 5.15.16.Updated target file handler method.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/business/impl/EtlNotificationServiceImpl.java (2)
37-38: Prefer constructor injection over field injection
Field injection can limit testability and discourage clear construction of dependencies. Implementing constructor injection often leads to more maintainable and testable code.
217-231: Use constants for queue names and verify JMS transaction boundaries
Using a string literal for"etl-notification"makes it prone to typos or usage in multiple places. Encouraging a constant or configuration property would improve readability and maintainability. Additionally, confirm that including message sending within this transactional method meets your reliability needs.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java (2)
23-24: Consider constructor injection for JmsTemplate
Constructor injection makes dependencies explicit, facilitating easier testing, mocking, and fewer hidden dependencies.
30-33: Extract queue name to a constant
Relying on string literals for queue names can introduce unintended typos. A static constant or configuration property improves clarity and consistency.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java (3)
19-29: Use constructor injection for injected services
Applying constructor injection for AdminService, SmsNotificationService, AshaTargetFileService, and EtlNotificationService ensures clearer dependencies and simpler test setups.
33-130: Consolidate repetitive JMS listener logic
All of these methods follow a similar pattern: logging, delegating toadminService, and handling exceptions. Consider refactoring common logic into a shared helper method or base class to reduce duplication.
154-168: Consider typed messages for ETL notifications
Using a plain string may limit flexibility. Passing a more descriptive object can help validate message contents, especially if more data is needed in the future.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java (1)
45-51: Evaluate more specialized message converters
WhileSimpleMessageConverteris functional, a specialized converter (e.g.,MappingJackson2MessageConverter) can streamline JSON or XML messaging if your application requires rich data formats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
NMSReportingSuite/pom.xml(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/business/impl/EtlNotificationServiceImpl.java(3 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java(2 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java(1 hunks)NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/utils/Global.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/utils/Global.java
- NMSReportingSuite/pom.xml
🔇 Additional comments (5)
NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/business/impl/EtlNotificationServiceImpl.java (1)
15-15: No issues with new importNMSReportingSuite/src/main/java/com/beehyv/nmsreporting/job/AshaSmsAutoTargetFileGeneration.java (1)
9-9: No issues with new importNMSReportingSuite/src/main/java/com/beehyv/nmsreporting/listeners/ReportGenerationListener.java (1)
134-152: Standardize message handling with a dedicated message class
This mirrors the recommendation from a previous review to unify how the target file queue is handled. Adopting a typed message can improve clarity and robust validation.NMSReportingSuite/src/main/java/com/beehyv/nmsreporting/config/ActiveMQConfig.java (2)
31-43: Verify broker security configuration
Currently, no credentials or SSL properties are set. If your environment requires secure or authenticated connections, ensure these settings are configured appropriately.
58-66: Validate concurrency settings and recovery interval
Concurrency is set to "1-5", with a 1-second recovery interval. Double-check these values against the expected message volume, processing time, and system performance requirements.
Summary by CodeRabbit
New Features
Refactor
Chores