AF-70: Replacing Nashorn Javascript Engine with GraalVM (#57)

* AF-70: Replacing Nashorn Javascript Engine with GraalVM * AF-70: Replacing Nashorn Javascript Engine with GraalVM AF-70: Replacing Nashorn Javascript Engine with GraalVM * Java 17 support * Java 17 support * Java 17 support
openmrs · Nov 23, 2023 · 1051223 · 1051223
1 parent b002d64
commit 1051223
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 36 deletions.
diff --git a/api/pom.xml b/api/pom.xml
@@ -51,13 +51,6 @@
 
 		<!-- End OpenMRS core -->
 
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <version>1.9.0</version>
-            <scope>test</scope>
-        </dependency>
-
         <dependency>
             <groupId>org.openmrs.module</groupId>
             <artifactId>appframework-appsfortesting</artifactId>
@@ -70,6 +63,21 @@
             <artifactId>handlebars</artifactId>
         </dependency>
 
+		<dependency>
+			<groupId>org.graalvm.js</groupId>
+			<artifactId>js</artifactId>
+		</dependency>
+
+		<dependency>
+			<groupId>org.graalvm.js</groupId>
+			<artifactId>js-scriptengine</artifactId>
+		</dependency>
+
+		<dependency>
+			<groupId>jakarta.xml.bind</groupId>
+			<artifactId>jakarta.xml.bind-api</artifactId>
+		</dependency>
+
 		<!-- For Testing -->
 		<dependency>
 			<groupId>joda-time</groupId>

diff --git a/api/src/main/java/org/openmrs/module/appframework/service/AppFrameworkServiceImpl.java b/api/src/main/java/org/openmrs/module/appframework/service/AppFrameworkServiceImpl.java
@@ -13,7 +13,6 @@
  */
 package org.openmrs.module.appframework.service;
 
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -44,12 +43,10 @@
 import org.openmrs.module.appframework.repository.AllUserApps;
 import org.springframework.transaction.annotation.Transactional;
 
-import javax.script.Bindings;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
-import javax.script.SimpleBindings;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -92,13 +89,10 @@ public AppFrameworkServiceImpl(AllAppTemplates allAppTemplates, AllAppDescriptor
 		this.locationService = locationService;
 		this.featureToggles = featureToggles;
 		this.appFrameworkConfig = appFrameworkConfig;
+		System.setProperty("polyglot.js.nashorn-compat", "true");
 		this.javascriptEngine = new ScriptEngineManager().getEngineByName("JavaScript");
 		this.allUserApps = allUserApps;
 
-		if (javascriptEngine == null) {
-			javascriptEngine = new jdk.nashorn.api.scripting.NashornScriptEngineFactory().getScriptEngine();
-			javascriptEngine.setBindings(javascriptEngine.createBindings(), ScriptContext.GLOBAL_SCOPE);
-		}
 
 		// there is surely a cleaner way to define this utility function in the global scope
 		this.javascriptEngine.eval("function hasMemberWithProperty(list, propName, val) { " + "if (!list) { return false; } "
@@ -338,21 +332,20 @@ public boolean checkRequireExpression(Requireable candidate, AppContextModel con
 				// with dot notation, but ScriptEngine and Bindings don't naturally handle this. Instead we will convert
 				// all Map-type properties to JSON and use this to define objects directly within the ScriptEngine.
 				// (Properties that are not Maps don't need this special treatment.)
-				Bindings bindings = new SimpleBindings();
-				Map<String, Object> mapProperties = new HashMap<String, Object>();
+				ObjectMapper objectMapper = new ObjectMapper();
+				Map<String, Object> mapProperties = new HashMap<>();
 				for (Map.Entry<String, Object> e : contextModel.entrySet()) {
 					if (e.getValue() instanceof Map) {
 						mapProperties.put(e.getKey(), e.getValue());
-					} else {
-						bindings.put(e.getKey(), e.getValue());
+					}
+					else if(!"util".equals(e.getKey())) {
+						javascriptEngine.eval("var " + e.getKey() + " = " + objectMapper.writeValueAsString(e.getValue()) + ";");
 					}
 				}
 
-				javascriptEngine.setBindings(bindings, ScriptContext.ENGINE_SCOPE);
-				ObjectMapper jackson = new ObjectMapper();
 				for (Map.Entry<String, Object> e : mapProperties.entrySet()) {
 					try {
-						javascriptEngine.eval("var " + e.getKey() + " = " + jackson.writeValueAsString(e.getValue()) + ";");
+						javascriptEngine.eval("var " + e.getKey() + " = " + objectMapper.writeValueAsString(e.getValue()) + ";");
 					}
 					catch (Exception ex) {
 						StringBuilder extraInfo = new StringBuilder();

diff --git a/.../test/java/org/openmrs/module/appframework/factory/AppConfigurationLoaderFactoryTest.java b/.../test/java/org/openmrs/module/appframework/factory/AppConfigurationLoaderFactoryTest.java
@@ -26,7 +26,6 @@ public class AppConfigurationLoaderFactoryTest extends BaseModuleContextSensitiv
     AllFreeStandingExtensions allFreeStandingExtensions;
 
     @Test
-    @DirtiesContext
     public void testConfigurationLoad() throws IOException {
 
         List<AppDescriptor> appDescriptors = appConfigurationLoaderFactory.getAppDescriptors();

diff --git a/api/src/test/java/org/openmrs/module/appframework/service/AppFrameworkServiceImplTest.java b/api/src/test/java/org/openmrs/module/appframework/service/AppFrameworkServiceImplTest.java
@@ -2,7 +2,7 @@
 
 import org.hibernate.Criteria;
 import org.hibernate.SessionFactory;
-import org.hibernate.classic.Session;
+import org.hibernate.Session;
 import org.joda.time.LocalDate;
 import org.joda.time.Months;
 import org.junit.After;

diff --git a/api/src/test/java/org/openmrs/module/appframework/service/AppFrameworkServiceTest.java b/api/src/test/java/org/openmrs/module/appframework/service/AppFrameworkServiceTest.java
@@ -56,7 +56,6 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.test.annotation.DirtiesContext;
 
-@DirtiesContext
 public class AppFrameworkServiceTest extends BaseModuleContextSensitiveTest {
 
 	public static final String LOCATION_UUID1 = "6f42abbc-caac-40ae-a94e-9277ea15c125";
@@ -438,7 +437,6 @@ public void purgeUserApp_shouldRemoveTheUserAppFromTheDatabaseAndUpdateTheListOf
 		assertEquals(--originalAppDescriptorCount, appFrameworkService.getAllApps().size());
 	}
 
-	@DirtiesContext
 	@Test
 	public void getLoginLocations_shouldReturnLoginLocationsConfiguredForTheUser() {
 		LocationTag tag = new LocationTag();

diff --git a/omod/pom.xml b/omod/pom.xml
@@ -72,7 +72,12 @@
 			<type>pom</type>
 			<scope>test</scope>
 		</dependency>
-
+
+		<dependency>
+			<groupId>jakarta.xml.bind</groupId>
+			<artifactId>jakarta.xml.bind-api</artifactId>
+		</dependency>
+
 		<!-- End OpenMRS core -->
 
 		<dependency>
@@ -86,7 +91,7 @@
 	        <version>3.18.2-GA</version>
 	        <scope>test</scope>
 	    </dependency>
-		
+
 	</dependencies>
 
 	<build>

diff --git a/omod/src/test/java/org/openmrs/module/appframework/web/AppFrameworkControllerTest.java b/omod/src/test/java/org/openmrs/module/appframework/web/AppFrameworkControllerTest.java
@@ -1,12 +1,12 @@
 package org.openmrs.module.appframework.web;
 
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.openmrs.api.context.Context;
-import org.openmrs.module.appframework.service.AppFrameworkService;
 import org.openmrs.module.appframework.domain.AppDescriptor;
+import org.openmrs.module.appframework.service.AppFrameworkService;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
@@ -20,6 +20,7 @@
 
 @PrepareForTest(Context.class)
 @RunWith(PowerMockRunner.class)
+@PowerMockIgnore("jdk.internal.reflect.*")
 public class AppFrameworkControllerTest {
 
     @Mock

diff --git a/pom.xml b/pom.xml
@@ -35,10 +35,11 @@
     </modules>
 
 	<properties>
-		<openMRSVersion>1.9.9</openMRSVersion>
+		<openMRSVersion>2.0.0</openMRSVersion>
 		<webservicesRestVersion>2.21.0</webservicesRestVersion>
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <handlebarsVersion>1.1.2</handlebarsVersion>
+		<graalvm.version>19.0.0</graalvm.version>
 	</properties>
 
 	<dependencyManagement>
@@ -137,6 +138,32 @@
                 </exclusions>
             </dependency>
 
+			<dependency>
+				<groupId>org.graalvm.js</groupId>
+				<artifactId>js</artifactId>
+				<version>${graalvm.version}</version>
+			</dependency>
+
+			<dependency>
+				<groupId>org.graalvm.js</groupId>
+				<artifactId>js-scriptengine</artifactId>
+				<version>${graalvm.version}</version>
+			</dependency>
+
+			<dependency>
+				<groupId>com.thoughtworks.xstream</groupId>
+				<artifactId>xstream</artifactId>
+				<version>1.4.17</version>
+				<scope>test</scope>
+			</dependency>
+
+			<dependency>
+				<groupId>jakarta.xml.bind</groupId>
+				<artifactId>jakarta.xml.bind-api</artifactId>
+				<scope>test</scope>
+				<version>2.3.2</version>
+			</dependency>
+
 		</dependencies>
 	</dependencyManagement>
 
@@ -147,8 +174,8 @@
 					<groupId>org.apache.maven.plugins</groupId>
 					<artifactId>maven-compiler-plugin</artifactId>
 					<configuration>
-						<target>1.6</target>
-						<source>1.6</source>
+						<target>1.7</target>
+						<source>1.7</source>
 					</configuration>
 				</plugin>
 				<plugin>
@@ -194,10 +221,6 @@
 				<plugin>
 					<groupId>org.apache.maven.plugins</groupId>
 					<artifactId>maven-surefire-plugin</artifactId>
-					<version>2.22.1</version>
-					<configuration>
-						<argLine>-Djdk.net.URLClassPath.disableClassPathURLCheck=true</argLine>
-					</configuration>
 				</plugin>
 			</plugins>
 		</pluginManagement>
@@ -234,6 +257,29 @@
             <url>https://mavenrepo.openmrs.org/snapshots</url>
         </snapshotRepository>
 	</distributionManagement>
+
+	<profiles>
+		<profile>
+			<id>Java 17</id>
+			<activation>
+				<jdk>17</jdk>
+			</activation>
+			<build>
+				<plugins>
+					<plugin>
+						<groupId>org.apache.maven.plugins</groupId>
+						<artifactId>maven-surefire-plugin</artifactId>
+						<configuration>
+							<argLine>--add-opens java.base/java.lang=ALL-UNNAMED
+								--add-opens java.base/jdk.internal.loader=ALL-UNNAMED
+								--add-opens java.base/java.security=ALL-UNNAMED
+							</argLine>
+						</configuration>
+					</plugin>
+				</plugins>
+			</build>
+		</profile>
+	</profiles>
 
 
 </project>