Skip to content

TRUNK-4988: Convenience method to allow us to change the configuration more easily#5934

Open
shivvani-r wants to merge 3 commits intoopenmrs:masterfrom
shivvani-r:TRUNK-4988
Open

TRUNK-4988: Convenience method to allow us to change the configuration more easily#5934
shivvani-r wants to merge 3 commits intoopenmrs:masterfrom
shivvani-r:TRUNK-4988

Conversation

@shivvani-r
Copy link
Contributor

Description of what I changed

I have implemented dynamic Spring dependency injection for MessageService in Context.java to allow easier configuration of message senders and preparators.

Why the change was needed:
Previously, the MessageService dependencies (MessageSender and MessagePreparator) were strictly hardcoded to instantiate MailMessageSender and VelocityMessagePreparator. This rigid coupling prevented hospitals or developers from easily swapping in custom modules (such as an SMS sender) without modifying and recompiling the core Java code.

Specific changes include:

  • Replaced the manual instantiations in Context.getMessageService() with dynamic Spring bean lookups using getRegisteredComponents().
  • Added fallback defensive logic to preserve backward compatibility: if no custom beans are registered in the Spring context, it safely defaults to the original MailMessageSender and VelocityMessagePreparator.
  • Added a warning log if multiple beans of the same type are registered, ensuring developers know which bean was selected.
  • Removed the outdated private static helper methods for creating the default senders.
  • Added comprehensive JUnit integration tests in ContextTest.java to verify that dynamically registered MessageSender and MessagePreparator beans are successfully discovered and loaded from the ApplicationContext.

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-4988

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.
  • I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)
  • I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.29%. Comparing base (bbf3317) to head (ee12643).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...src/main/java/org/openmrs/api/context/Context.java 63.63% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5934      +/-   ##
============================================
- Coverage     59.31%   59.29%   -0.02%     
- Complexity     9252     9254       +2     
============================================
  Files           686      686              
  Lines         37123    37136      +13     
  Branches       5452     5457       +5     
============================================
+ Hits          22018    22019       +1     
- Misses        13141    13150       +9     
- Partials       1964     1967       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@jwnasambu jwnasambu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly we need to verify that the system correctly falls back to the default implementations MailMessageSender, VelocityMessagePreparator when no beans are registered.

In addition, MessageService is a singleton, modifying it in a test can break other tests if the state isn't perfectly restored. I would recommend adding this test case using a try-finally block to guarantee cleanup. Otherwise, Great work!

@shivvani-r
Copy link
Contributor Author

Kindly we need to verify that the system correctly falls back to the default implementations MailMessageSender, VelocityMessagePreparator when no beans are registered.

In addition, MessageService is a singleton, modifying it in a test can break other tests if the state isn't perfectly restored. I would recommend adding this test case using a try-finally block to guarantee cleanup. Otherwise, Great work!

Thanks for the feedback, @jwnasambu! I've extracted the duplicate logic into a getSingleRegisteredComponent helper method. I also added the getMessageService_shouldFallbackToDefaultSenderAndPreparatorWhenNoBeansRegisteredtest (with a try-finally cleanup block) to explicitly verify the fallback behavior. Let me know if everything looks good to go!

cc: @dkayiwa @ibacher

@sonarqubecloud
Copy link

@shivvani-r shivvani-r requested a review from jwnasambu March 18, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants