Skip to content

Update to Platform 2.7.3 and Support Java 21 #13

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.

Sign up for GitHub

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

Merged
dkayiwa merged 1 commit into from
Apr 1, 2025
Merged

Update to Platform 2.7.3 and Support Java 21 #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ jobs:
strategy:
matrix:
platform: [ ubuntu-latest ]
java-version: [ 8 ]
java-version: [ 21 ]

runs-on: ${{ matrix.platform }}
env:
PLATFORM: ${{ matrix.platform }}
JAVA_VERSION: ${{ matrix.java-version }}

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- name: Set up JDK
uses: actions/setup-java@v1
with:
java-version: ${{ matrix.java-version }}
- name: Cache local Maven repository
uses: actions/cache@v2
uses: actions/cache@v3
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
Expand Down
12 changes: 1 addition & 11 deletions api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.openmrs.module</groupId>
<artifactId>calculation</artifactId>
<version>1.3.1-SNAPSHOT</version>
<version>2.0.0-SNAPSHOT</version>
</parent>

<artifactId>calculation-api</artifactId>
Expand Down Expand Up @@ -58,16 +58,6 @@
<type>jar</type>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
</dependency>

<dependency>
<groupId>org.codehaus.groovy</groupId>
<artifactId>groovy-all</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/calculation/ClasspathCalculationProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.junit.Test;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.openmrs.test.Verifies;
import org.springframework.test.annotation.ExpectedException;

/**
* Tests the ClasspathCalculationProvider
Expand Down Expand Up @@ -51,8 +50,7 @@ public void getCalculation_shouldRetrieveANonConfigurableCalculationWithANullCon
* @see ClasspathCalculationProvider#getCalculation(String,String)
* @verifies throw an exception if a configurable calculation is passed an illegal configuration
*/
@Test
@ExpectedException(InvalidCalculationException.class)
@Test(expected = InvalidCalculationException.class)
public void getCalculation_shouldThrowAnExceptionIfAConfigurableCalculationIsPassedAnIllegalConfiguration()
throws Exception {
ClasspathCalculationProvider p = new ClasspathCalculationProvider();
Expand All @@ -64,8 +62,7 @@ public void getCalculation_shouldThrowAnExceptionIfAConfigurableCalculationIsPas
* @verifies throw an exception if a non configurable calculation is passed a configuration
* string
*/
@Test
@ExpectedException(InvalidCalculationException.class)
@Test(expected = InvalidCalculationException.class)
public void getCalculation_shouldThrowAnExceptionIfANonConfigurableCalculationIsPassedAConfigurationString()
throws Exception {
ClasspathCalculationProvider p = new ClasspathCalculationProvider();
Expand Down
21 changes: 19 additions & 2 deletions api/src/test/java/org/openmrs/calculation/MostRecentObsCalculation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
package org.openmrs.calculation;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.openmrs.Cohort;
import org.openmrs.CohortMembership;
import org.openmrs.Concept;
import org.openmrs.Obs;
import org.openmrs.api.context.Context;
Expand Down Expand Up @@ -55,7 +58,7 @@ public void setConfiguration(String configuration) throws InvalidCalculationExce
public CalculationResultMap evaluate(Collection<Integer> cohort, Map<String, Object> parameterValues, PatientCalculationContext context) {

CalculationResultMap results = new CalculationResultMap();
Map<Integer, List<Obs>> patientObs = Context.getPatientSetService().getObservations(new Cohort(cohort), whichConcept);
Map<Integer, List<Obs>> patientObs = getObservations(new Cohort(cohort), whichConcept);
for (Integer pId : patientObs.keySet()) {
String cacheKey = this.getClass().getName() + "." + whichConcept + "." + pId;
CalculationResult r = (CalculationResult) context.getFromCache(cacheKey);
Expand All @@ -74,4 +77,18 @@ public CalculationResultMap evaluate(Collection<Integer> cohort, Map<String, Obj
public Concept getWhichConcept() {
return whichConcept;
}
}

private Map<Integer, List<Obs>> getObservations(Cohort cohort, Concept concept) {
if (cohort == null || concept == null || cohort.getMemberships().isEmpty()) {
return Collections.emptyMap();
}

return cohort.getMemberships().stream()
Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 31, 2025

Choose a reason for hiding this comment

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

I find this to be a very inefficient replacement when compared to the original implementation: https://github.com/openmrs/openmrs-core/blob/1.12.x/api/src/main/java/org/openmrs/api/db/hibernate/HibernatePatientSetDAO.java#L1092-L1141

Copy link
Copy Markdown
Member Author

@wikumChamith wikumChamith Mar 31, 2025

Choose a reason for hiding this comment

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

should we update this to do a Hibernate query?

Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 31, 2025

Choose a reason for hiding this comment

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

Whatever you find better than the current code.

Copy link
Copy Markdown
Member Author

@wikumChamith wikumChamith Apr 1, 2025

Choose a reason for hiding this comment

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

@dkayiwa, does performance matter much in this case since it's only used in tests?

Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Apr 1, 2025

Choose a reason for hiding this comment

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

That is a very good point and you are indeed correct. 😊

.map(CohortMembership::getPatientId)
.collect(Collectors.toMap(
id -> id,
id -> Context.getObsService().getObservationsByPersonAndConcept(
Context.getPersonService().getPerson(id), concept)
));
}
}
11 changes: 6 additions & 5 deletions api/src/test/java/org/openmrs/calculation/patient/PatientBehaviorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.Before;
import org.junit.Test;
import org.openmrs.Cohort;
import org.openmrs.Concept;
import org.openmrs.Encounter;
import org.openmrs.Obs;
import org.openmrs.api.PatientService;
Expand Down Expand Up @@ -204,20 +205,20 @@ public void shouldGetAnObsResultAsADateBasedResult() throws Exception {
@Test
public void ShouldGetADateBasedResultBasingOnACachedResultInTheCalculationContext() throws Exception {
executeDataSet(TEST_DATA_XML);

Integer patientId = 7;
Obs expectedMostRecentObs = Context.getObsService().getObs(103);
String cacheKey = MostRecentObsCalculation.class.getName() + ".5089.7";
String cacheKey = MostRecentObsCalculation.class.getName() + ".Concept" + " #5089.7";

PatientCalculationContext context = getService().createCalculationContext();
Assert.assertTrue(context.getFromCache(cacheKey) == null);

//sanity check, since the cache is empty, it should return the most recent obs amongst all obs for the patient
PatientCalculation mostRecentObsCalculation = getMostRecentWeightCalculation();
ObsResult firstTestResult = (ObsResult) getService().evaluate(patientId, mostRecentObsCalculation, context);
Assert.assertEquals(expectedMostRecentObs, firstTestResult.asType(Obs.class));
Assert.assertTrue(context.getFromCache(cacheKey) == firstTestResult);

PatientCalculation anotherMostRecentObsCalculation = getMostRecentWeightCalculation();
ObsResult secondTestResult = (ObsResult) getService().evaluate(patientId, anotherMostRecentObsCalculation, context);
Assert.assertTrue(context.getFromCache(cacheKey) == secondTestResult);
Expand Down
18 changes: 8 additions & 10 deletions api/src/test/resources/TestingApplicationContext.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:util="http://www.springframework.org/schema/util"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-2.5.xsd
http://www.springframework.org/schema/context
http://www.springframework.org/schema/context/spring-context-2.5.xsd
http://www.springframework.org/schema/util
http://www.springframework.org/schema/util/spring-util-2.0.xsd">
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-2.5.xsd">

<bean id="sessionFactory" class="org.openmrs.api.db.hibernate.HibernateSessionFactoryBean">
<property name="configLocations">
Expand All @@ -20,6 +14,10 @@
<property name="mappingJarLocations">
<ref bean="mappingJarResources"/>
</property>
<property name="packagesToScan">
<list>
<value>org.openmrs</value>
</list>
</property>
</bean>

</beans>
8 changes: 4 additions & 4 deletions api/src/test/resources/org/openmrs/calculation/include/moduleTestData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
<calculation_registration calculation_registration_id="2" token="mostRecentWeight" provider_class_name="org.openmrs.calculation.ClasspathCalculationProvider" calculation_name="org.openmrs.calculation.MostRecentObsCalculation" configuration="5089" uuid="0ee98666-5826-11e1-b480-00248140a5eb" />
<calculation_registration calculation_registration_id="3" token="mostRecentCD4" provider_class_name="org.openmrs.calculation.ClasspathCalculationProvider" calculation_name="org.openmrs.calculation.MostRecentObsCalculation" configuration="5497" uuid="0ee98666-5826-11e1-b591-00248140a5eb" />

<obs obs_id="100" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:35:30.0" location_id="2" value_numeric="62.0" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="354f2bec-5e2c-11e1-8b14-00248140a5eb" />
<obs obs_id="101" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:37:30.0" location_id="2" value_numeric="63.0" creator="1" date_created="2008-08-19 12:35:30.0" voided="1" uuid="499c555c-5e2c-11e1-8b14-00248140a5eb" />
<obs obs_id="102" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:36:30.0" location_id="2" value_numeric="62.5" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="830906aa-5e2c-11e1-8b14-00248140a5eb" />
<obs obs_id="103" person_id="7" concept_id="5089" encounter_id="4" obs_datetime="2008-08-19 10:38:30.0" location_id="2" value_numeric="61.5" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="aeca3036-5e70-11e1-8b14-00248140a5eb" />
<obs obs_id="100" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:35:30.0" location_id="2" value_numeric="62.0" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="354f2bec-5e2c-11e1-8b14-00248140a5eb" status="PRELIMINARY" />
<obs obs_id="101" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:37:30.0" location_id="2" value_numeric="63.0" creator="1" date_created="2008-08-19 12:35:30.0" voided="1" uuid="499c555c-5e2c-11e1-8b14-00248140a5eb" status="PRELIMINARY" />
<obs obs_id="102" person_id="7" concept_id="5089" encounter_id="5" obs_datetime="2008-08-19 10:36:30.0" location_id="2" value_numeric="62.5" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="830906aa-5e2c-11e1-8b14-00248140a5eb" status="PRELIMINARY" />
<obs obs_id="103" person_id="7" concept_id="5089" encounter_id="4" obs_datetime="2008-08-19 10:38:30.0" location_id="2" value_numeric="61.5" creator="1" date_created="2008-08-19 12:35:30.0" voided="0" uuid="aeca3036-5e70-11e1-8b14-00248140a5eb" status="PRELIMINARY" />
</dataset>
9 changes: 8 additions & 1 deletion omod/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.openmrs.module</groupId>
<artifactId>calculation</artifactId>
<version>1.3.1-SNAPSHOT</version>
<version>2.0.0-SNAPSHOT</version>
</parent>

<artifactId>calculation-omod</artifactId>
Expand Down Expand Up @@ -71,6 +71,13 @@

<!-- End OpenMRS core -->

<dependency>
<groupId>org.openmrs.module</groupId>
<artifactId>legacyui-omod</artifactId>
<version>1.22.0</version>
Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 26, 2025

Choose a reason for hiding this comment

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

Shouldn't we add scope?

<scope>provided</scope>
</dependency>

</dependencies>

<build>
Expand Down
20 changes: 6 additions & 14 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>org.openmrs.module</groupId>
<artifactId>calculation</artifactId>
<version>1.3.1-SNAPSHOT</version>
<version>2.0.0-SNAPSHOT</version>
<packaging>pom</packaging>
<name>Calculation</name>
<description>Provides a framework for OpenMRS developers to author simple or complex calculations to produce a particular piece of data for a single patient or a cohort of patients</description>
Expand Down Expand Up @@ -34,9 +34,9 @@
</modules>

<properties>
<openMRSVersion>1.9.9</openMRSVersion>
<openMRSVersion>2.7.3</openMRSVersion>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<groovyVersion>1.8.7</groovyVersion>
<groovyVersion>2.4.21</groovyVersion>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -89,20 +89,12 @@
</dependency>

<!-- End OpenMRS core -->

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.8.5</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.codehaus.groovy</groupId>
Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 30, 2025

Choose a reason for hiding this comment

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

Is there a reason why you changed it from org.codehaus.groovy?

Copy link
Copy Markdown
Member Author

@wikumChamith wikumChamith Mar 31, 2025

Choose a reason for hiding this comment

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

Yes, org.codehaus.groovy has been relocated to org.apache.groovy. https://mvnrepository.com/artifact/org.codehaus.groovy/groovy-all

Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 31, 2025

Choose a reason for hiding this comment

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

Which one are we using in platform 2.7.3?

Copy link
Copy Markdown
Member Author

@wikumChamith wikumChamith Mar 31, 2025

Choose a reason for hiding this comment

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

org.codehaus.groovy:2.4.21. Should I update it there or remove it from here?

https://github.com/openmrs/openmrs-core/blob/02babbcd3d63f717d3f1a118e790b5b957dfa11d/api/pom.xml#L244-L248

Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 31, 2025

Choose a reason for hiding this comment

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

You can update the one in core

Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 31, 2025

Choose a reason for hiding this comment

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

Can you also update the version?

<artifactId>groovy-all</artifactId>
<version>${groovyVersion}</version>
<type>jar</type>
Copy link
Copy Markdown
Member

@dkayiwa dkayiwa Mar 30, 2025

Choose a reason for hiding this comment

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

The same goes for changing jar to pom

Copy link
Copy Markdown
Member Author

@wikumChamith wikumChamith Mar 31, 2025

Choose a reason for hiding this comment

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

https://mvnrepository.com/artifact/org.codehaus.groovy/groovy-all/3.0.24

<scope>provided</scope>
<type>jar</type>
</dependency>

</dependencies>
Expand All @@ -115,8 +107,8 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<target>1.6</target>
<source>1.6</source>
<target>21</target>
<source>21</source>
</configuration>
</plugin>
<plugin>
Expand Down