Migrate to Platform 2.7.3 and Java 21 by wikumChamith · Pull Request #211 · openmrs/openmrs-module-legacyui

wikumChamith · 2025-04-07T06:40:10Z

Updated module to Java 21 and Platform 2.7.3. Replaced WebUtils#getTargetPage, which was removed in newer Spring versions, with a custom implementation.

Ticket: https://openmrs.atlassian.net/browse/LUI-200

dkayiwa · 2025-04-07T11:43:54Z

omod/src/main/java/org/springframework/web/servlet/mvc/AbstractWizardFormController.java

 	 */
 	protected int getTargetPage(HttpServletRequest request, int currentPage) {
-		return WebUtils.getTargetPage(request, PARAM_TARGET, currentPage);
+		Enumeration<String> parameterNames = request.getParameterNames();


Do you mind pointing me to where you got this implementation?

I kinda based this on the original implementation

public static int getTargetPage(ServletRequest request, String paramPrefix, int currentPage) { Enumeration<String> paramNames = request.getParameterNames(); while(paramNames.hasMoreElements()) { String paramName = (String)paramNames.nextElement(); if (paramName.startsWith(paramPrefix)) { for(int i = 0; i < SUBMIT_IMAGE_SUFFIXES.length; ++i) { String suffix = SUBMIT_IMAGE_SUFFIXES[i]; if (paramName.endsWith(suffix)) { paramName = paramName.substring(0, paramName.length() - suffix.length()); } } return Integer.parseInt(paramName.substring(paramPrefix.length())); } } return currentPage; }

Where did you get that from?

Just going to the source using the IDE.

That one includes a logic to remove the submit button image suffixes. I did not include that here. Should I add that?

Which version of spring did you copy it from?

The version the module was using: Spring 4.1.4 RELEASE

But what you have is different from where you copied it: https://github.com/spring-projects/spring-framework/blob/v4.3.30.RELEASE/spring-web/src/main/java/org/springframework/web/util/WebUtils.java#L698-L713

I updated the code

dkayiwa · 2025-04-07T19:18:17Z

You would need to create a branch of the module for older platform versions.
Do not forget to also update the version in the pom.xml files.

wikumChamith · 2025-04-08T05:29:25Z

You would need to create a branch of the module for older platform versions. Do not forget to also update the version in the pom.xml files.

I created the 1.x branch and also updated the development version to 2.0.0-SNAPSHOT: 53c7372

wikumChamith · 2025-04-08T06:14:04Z

@dkayiwa don't merge this yet, I just noticed this is not running all the tests correctly.

wikumChamith · 2025-04-08T08:11:47Z

@dkayiwa don't merge this yet, I just noticed this is not running all the tests correctly.

We can get the tests to run by either removing the parent element or updating the maven-parent artifact and setting the version to the latest snapshot of that artifact:
openmrs/openmrs-contrib-maven-parent-module#7

cc: @ibacher

dkayiwa · 2025-04-08T13:34:21Z

I created the 1.x branch and also updated the development version to 2.0.0-SNAPSHOT: 53c7372

I have switched the bamboo CI build from the master to the 1.x branch.

wikumChamith · 2025-04-09T10:38:10Z

@dkayiwa I'm running into an issue with the findCountAndPatients_shouldMatchPatientWithIdentifiersThatContainNoDigit test. The method getCountOfPatients doesn't return a match for the search query unless I set the includeVoided argument to true—even though the patient identifier being searched for isn’t voided. I was also able to reproduce the same behavior in core by writing a test directly there

dkayiwa · 2025-04-09T13:03:51Z

Is the test failure triggered by simply switching the platform version?

wikumChamith · 2025-04-09T13:20:37Z

Is the test failure triggered by simply switching the platform version?

Yeah

wikumChamith · 2025-04-10T07:22:40Z

@dkayiwa I fixed the test errors.

dkayiwa · 2025-04-10T22:54:08Z

omod/pom.xml

+			<scope>test</scope>
+		</dependency>
+
+		<dependency>


Do we really need these bytebuddy dependencies?

Yeah, we need it for Mockito. I think after we migrate the Core to a newer version of Mockito we could remove this from here.

https://github.com/openmrs/openmrs-core/blob/314cc2fd8646423dd7592e8037edeba3817ab0eb/pom.xml#L1144-L1159

What error do you get when you remove them from just here?

[ERROR] ConceptFormBackingObject_shouldCopyNumericAttributes Time elapsed: 0.175 s <<< ERROR! org.mockito.exceptions.base.MockitoException: Mockito cannot mock this class: class org.openmrs.ConceptNumeric. If you're not sure why you're getting this error, please report to the mailing list. Java : 21 JVM vendor name : Amazon.com Inc. JVM vendor version : 21.0.6+7-LTS JVM name : OpenJDK 64-Bit Server VM JVM version : 21.0.6+7-LTS JVM info : mixed mode, sharing OS name : Linux OS version : 6.1.0-32-amd64 You are seeing this disclaimer because Mockito is configured to create inlined mocks. You can learn about inline mocks and their limitations under item #39 of the Mockito class javadoc. Underlying exception : org.mockito.exceptions.base.MockitoException: Could not modify all classes [interface org.openmrs.OpenmrsObject, class java.lang.Object, class org.openmrs.ConceptNumeric, interface org.openmrs.Auditable, interface java.io.Serializable, interface org.openmrs.customdatatype.Customizable, class org.openmrs.Concept, interface org.openmrs.Changeable, class org.openmrs.BaseOpenmrsObject, interface org.openmrs.Creatable, interface org.openmrs.Attributable, interface org.openmrs.Retireable] at org.openmrs.web.controller.concept.ConceptFormControllerTest.ConceptFormBackingObject_shouldCopyNumericAttributes(ConceptFormControllerTest.java:25) Caused by: org.mockito.exceptions.base.MockitoException: Could not modify all classes [interface org.openmrs.OpenmrsObject, class java.lang.Object, class org.openmrs.ConceptNumeric, interface org.openmrs.Auditable, interface java.io.Serializable, interface org.openmrs.customdatatype.Customizable, class org.openmrs.Concept, interface org.openmrs.Changeable, class org.openmrs.BaseOpenmrsObject, interface org.openmrs.Creatable, interface org.openmrs.Attributable, interface org.openmrs.Retireable] at org.openmrs.web.controller.concept.ConceptFormControllerTest.ConceptFormBackingObject_shouldCopyNumericAttributes(ConceptFormControllerTest.java:25) Caused by: java.lang.IllegalStateException: Byte Buddy could not instrument all classes within the mock's type hierarchy This problem should never occur for javac-compiled classes. This problem has been observed for classes that are: - Compiled by older versions of scalac - Classes that are part of the Android distribution at org.openmrs.web.controller.concept.ConceptFormControllerTest.ConceptFormBackingObject_shouldCopyNumericAttributes(ConceptFormControllerTest.java:25) Caused by: java.lang.IllegalArgumentException: Java 21 (65) is not supported by the current version of Byte Buddy which officially supports Java 20 (64) - update Byte Buddy or set net.bytebuddy.experimental as a VM property at org.openmrs.web.controller.concept.ConceptFormControllerTest.ConceptFormBackingObject_shouldCopyNumericAttributes(ConceptFormControllerTest.java:25)

Did you see my comment about removing them from only that file?

are you referring to "mockito-inline"?

We need it to do static mocks without using powermock.

What i mean is, i put a comment in a specific file to remove the bytebuddy dependencies from only that file. Can you do that and commit?

Ohh ok. I updated that and it is working now :)

dkayiwa · 2025-04-10T23:07:54Z

omod/src/test/resources/org/openmrs/web/controller/include/PortletControllerTest-bmi.xml

  <concept_description concept_description_id="5090" concept_id="5090" description="Patient&apos;s height in kilograms." locale="en" creator="1" date_created="2004-08-12 00:00:00.0" uuid="18ae66c9-cd2e-429d-9c6d-29bd5805dcf8"/>
  <concept_name concept_id="5090" name="HEIGHT (KG)" locale="en" creator="1" date_created="2004-08-12 00:00:00.0" concept_name_id="1439" voided="false" uuid="4302daf5-acc8-4173-b766-bed42ae79e06" concept_name_type="FULLY_SPECIFIED" locale_preferred="0"/>
-  <concept_numeric concept_id="5090" hi_normal="250.0" low_critical="0.0" units="cm" precise="true"/>
+  <concept_numeric concept_id="5090" hi_normal="250.0" low_critical="0.0" units="cm" display_precision="true" allow_decimal="true" />


Do we really need the display_precision?

dkayiwa · 2025-04-10T23:19:42Z

omod/src/main/java/org/openmrs/web/controller/LoginController.java


 			//will be sending the alert via ajax, so we need to escape js special chars
-			model.put("alertMessage", JavaScriptUtils.javaScriptEscape(alertMessage));
+			if(alertMessage != null) {


https://openmrs.atlassian.net/wiki/spaces/docs/pages/25477044/Java+Conventions#Code-Style

dkayiwa · 2025-04-10T23:26:35Z

omod/src/main/java/org/springframework/web/servlet/mvc/AbstractWizardFormController.java

+					// Remove any image button suffixes (like .x or .y)
+					if (param.endsWith(suffix)) {
+						param = param.substring(0, param.length() - suffix.length());
+						break;


We do not have this break in the original code.

dkayiwa · 2025-04-10T23:27:09Z

omod/src/main/java/org/springframework/web/servlet/mvc/AbstractWizardFormController.java

+				try {
+					return Integer.parseInt(pageStr);
+				}
+				catch (NumberFormatException e) {


The original code does not catch this exception.

wikumChamith · 2025-04-11T05:01:14Z

@dkayiwa I updated the PR.

dkayiwa · 2025-04-11T07:43:57Z

pom.xml

+			<dependency>
+				<groupId>net.bytebuddy</groupId>
+				<artifactId>byte-buddy</artifactId>
+				<version>1.17.4</version>


Shouldn't we add scope here?

pom.xml

dkayiwa · 2025-04-30T10:23:07Z

@wikumChamith basing on the approach which have taken for the addresshierarchy module, is there anything that prevents us from running or compiling this on Java 8 too?

wikumChamith · 2025-04-30T11:36:53Z

@wikumChamith basing on the approach which have taken for the addresshierarchy module, is there anything that prevents us from running or compiling this on Java 8 too?

No. We can support both Java 21 and Java 8 for this module.

dkayiwa · 2025-04-30T11:40:45Z

So can we do the needful?

wikumChamith · 2025-04-30T12:04:09Z

So can we do the needful?

@dkayiwa PR: #214

wikumChamith requested a review from dkayiwa April 7, 2025 06:40

wikumChamith changed the title ~~Migrate to platform 2.7.3 and Java 21~~ Migrate to Platform 2.7.3 and Java 21 Apr 7, 2025

wikumChamith force-pushed the LU-200 branch from ceb1e2c to a589ee0 Compare April 7, 2025 06:42

dkayiwa reviewed Apr 7, 2025

View reviewed changes

wikumChamith added 2 commits April 8, 2025 10:57

Migrate to platform 2.7.3 and Java 21

e82fec2

Update Java version to 21 in GitHub Actions workflow

463e090

wikumChamith force-pushed the LU-200 branch from a589ee0 to 463e090 Compare April 8, 2025 05:27

wikumChamith force-pushed the LU-200 branch from 7926275 to 1b206b8 Compare April 10, 2025 07:21

wikumChamith force-pushed the LU-200 branch from 1b206b8 to 2dc8b98 Compare April 10, 2025 07:29

wikumChamith requested a review from dkayiwa April 10, 2025 10:20

dkayiwa reviewed Apr 10, 2025

View reviewed changes

Fix test errors

978a68c

wikumChamith force-pushed the LU-200 branch from 2dc8b98 to 978a68c Compare April 11, 2025 05:01

wikumChamith added 2 commits April 11, 2025 12:06

Remove byte buddy

32be050

Remove byte buddy

1119b15

dkayiwa reviewed Apr 11, 2025

View reviewed changes

pom.xml Show resolved Hide resolved

Move the byte buddy to test scope

11e01a1

dkayiwa merged commit e13bb24 into openmrs:master Apr 11, 2025
1 check passed

Conversation

wikumChamith commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkayiwa commented Apr 7, 2025

Uh oh!

wikumChamith commented Apr 8, 2025

Uh oh!

wikumChamith commented Apr 8, 2025

Uh oh!

wikumChamith commented Apr 8, 2025

Uh oh!

dkayiwa commented Apr 8, 2025

Uh oh!

wikumChamith commented Apr 9, 2025

Uh oh!

dkayiwa commented Apr 9, 2025

Uh oh!

wikumChamith commented Apr 9, 2025

Uh oh!

wikumChamith commented Apr 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wikumChamith commented Apr 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dkayiwa commented Apr 30, 2025

Uh oh!

wikumChamith commented Apr 30, 2025

Uh oh!

dkayiwa commented Apr 30, 2025

Uh oh!

wikumChamith commented Apr 30, 2025