Skip to content

Upgrade to OpenMRS core 2.7.5 with Java 8/11/17/21 support#134

Open
dkayiwa wants to merge 3 commits intomasterfrom
upgrade-to-openmrs-2.7.5
Open

Upgrade to OpenMRS core 2.7.5 with Java 8/11/17/21 support#134
dkayiwa wants to merge 3 commits intomasterfrom
upgrade-to-openmrs-2.7.5

Conversation

@dkayiwa
Copy link
Member

@dkayiwa dkayiwa commented Feb 25, 2026

Summary

Upgrades the idgen module to compile and pass all tests on OpenMRS core 2.7.5 with Java 8, 11, 17, and 21.

Changes

Dependency Updates

  • openMRSVersion: 1.10.2 → 2.7.5
  • webservices.rest: 2.23.0 → 3.0.0 (Spring 5 / Servlet 3.1 compatible)
  • Added legacyui-omod:1.16.0 (provided) for AdministrationSectionExt
  • Replaced PowerMock with mockito-inline:4.11.0 for static mocking
  • Added javax.servlet-api:3.1.0 test dependency

API Fixes (Hibernate 5 / OpenMRS 2.x)

  • HibernateIdentifierSourceDAO: Replaced all Expression.xyz() with Restrictions.xyz()
  • IdentifierSourceService: Switched to PrivilegeConstants.EDIT_PATIENT_IDENTIFIERS
  • config.xml: Updated require_version to 2.7.5

Test Infrastructure

  • Migrated SequentialIdentifierGeneratorTest and LocationBasedPrefixProviderTest from PowerMock to Mockito MockedStatic
  • Fixed deprecated Mockito APIs in IdentifierSourceControllerTest
  • Created TestTransactionAttributeSource to downgrade REQUIRES_NEWREQUIRED for H2 1.4.200 MVStore compatibility
  • Added packagesToScan for JPA entity scanning in TestingApplicationContext.xml
  • Fixed TestData.xml: removed CHECK_DIGIT column, used valid user IDs (1, 501, 502)
  • Fixed DuplicateIdentifiersPoolComponentTest: explicit transaction commit, synchronized auth
  • Added @PropertyGetter("identifiers") to IdentifierPoolResourceHandler for proper JSON serialization
  • Removed dummy CodedOrFreeText.java (now in OpenMRS core)

Java Version Compatibility

  • Added Maven profile java9plus with --add-opens JVM args (activated for JDK 9+)
  • Added -Dnet.bytebuddy.experimental=true for ByteBuddy/Java 21 support
  • Java 8 builds without --add-opens (not supported)

Test Results

Java Version API Tests OMOD Tests Result
Java 8 44 (2 skipped) 99 ✅ BUILD SUCCESS
Java 11 44 (2 skipped) 99 ✅ BUILD SUCCESS
Java 17 44 (2 skipped) 99 ✅ BUILD SUCCESS
Java 21 44 (2 skipped) 99 ✅ BUILD SUCCESS

dkayiwa and others added 2 commits February 25, 2026 23:09
- Update openMRSVersion from 1.10.2 to 2.7.5
- Update webservices.rest from 2.23.0 to 3.0.0 (Spring 5 compatible)
- Add legacyui-omod 1.16.0 dependency for AdministrationSectionExt
- Update config.xml require_version to 2.7.5

API changes:
- Replace Expression.xyz() with Restrictions.xyz() (Hibernate 5)
- Use PrivilegeConstants.EDIT_PATIENT_IDENTIFIERS (OpenMRS 2.x)

Test infrastructure:
- Migrate PowerMock tests to Mockito MockedStatic
- Replace mockito-core with mockito-inline for static mocking
- Add --add-opens JVM args for Java 9+ via Maven profile
- Add -Dnet.bytebuddy.experimental=true for Java 21 support
- Create TestTransactionAttributeSource to downgrade REQUIRES_NEW
  to REQUIRED for H2 MVStore compatibility in tests
- Fix TestData.xml for OpenMRS 2.7.5 schema (CHECK_DIGIT, user IDs)
- Fix DuplicateIdentifiersPoolComponentTest concurrency issues
- Add @PropertyGetter for identifiers on IdentifierPoolResourceHandler
- Add javax.servlet-api 3.1.0 test dependency
- Remove dummy CodedOrFreeText.java (now in core)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dkayiwa
Copy link
Member Author

dkayiwa commented Feb 26, 2026

@wikumChamith i did this not to replace your pull request but just for experimental purposes. I switched to CLI and it did what i wanted of running in a loop until all errors are fixed. How do you compare this with yours in terms of quality, completeness, and other things? 😊

pom.xml Outdated
Comment on lines +133 to +152
<profiles>
<profile>
<id>java9plus</id>
<activation>
<jdk>[9,)</jdk>
</activation>
<properties>
<surefire.argLine>
-Dnet.bytebuddy.experimental=true
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.lang.reflect=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/java.text=ALL-UNNAMED
--add-opens java.base/java.io=ALL-UNNAMED
--add-opens java.base/java.net=ALL-UNNAMED
--add-opens java.rmi/sun.rmi.transport=ALL-UNNAMED
</surefire.argLine>
</properties>
</profile>
</profiles>
Copy link
Member

Choose a reason for hiding this comment

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

@dkayiwa, we need to avoid these

Copy link
Member Author

Choose a reason for hiding this comment

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

I completely agree. Let me prompt it further about these. Like i said again, not to replace yours, but for learning how out get the best out of these guys. 😊

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about its followup commit after asking the agent to remove that? 240dd09

Refactor tests to avoid MockedStatic<Context> (which requires
mockito-inline and many --add-opens JVM arguments on Java 9+):

- SequentialIdentifierGeneratorTest: Use spy with stubbed
  getPrefixProvider/getSuffixProvider instead of mocking Context
- LocationBasedPrefixProviderTest: Set prefixLocationAttributeType
  via reflection instead of mocking Context.getAdministrationService()
- Remove java9plus Maven profile with all --add-opens flags
- Remove surefire argLine configuration

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@wikumChamith
Copy link
Member

This looks good but it does something different than the ways my Claude and I tried. I might be able to steal something from this :)

* uncommitted data from suspended transactions, which causes REQUIRES_NEW to fail
* in tests where data is loaded within a test transaction.
*/
public class TestTransactionAttributeSource extends AnnotationTransactionAttributeSource {
Copy link
Member

Choose a reason for hiding this comment

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

This is really interesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same Claude Opus 4.6 (Different Harness Copilot CLI). Please steal as much as you can. This agent will not feel jealous! 🤣

Something i have also done several times is to simply ask it for alternatives. 😊

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.

2 participants