From 10512236963b59e33bf89bb16a111f1ada61ba37 Mon Sep 17 00:00:00 2001 From: Wikum Weerakutti Date: Thu, 23 Nov 2023 19:17:25 +0530 Subject: [PATCH] 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 --- api/pom.xml | 22 ++++--- .../service/AppFrameworkServiceImpl.java | 21 +++---- .../AppConfigurationLoaderFactoryTest.java | 1 - .../service/AppFrameworkServiceImplTest.java | 2 +- .../service/AppFrameworkServiceTest.java | 2 - omod/pom.xml | 9 ++- .../web/AppFrameworkControllerTest.java | 5 +- pom.xml | 60 ++++++++++++++++--- 8 files changed, 86 insertions(+), 36 deletions(-) diff --git a/api/pom.xml b/api/pom.xml index 0e43608..cfe2bd4 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -51,13 +51,6 @@ - - org.mockito - mockito-all - 1.9.0 - test - - org.openmrs.module appframework-appsfortesting @@ -70,6 +63,21 @@ handlebars + + org.graalvm.js + js + + + + org.graalvm.js + js-scriptengine + + + + jakarta.xml.bind + jakarta.xml.bind-api + + joda-time 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 index 1226005..75632bb 100644 --- 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 mapProperties = new HashMap(); + ObjectMapper objectMapper = new ObjectMapper(); + Map mapProperties = new HashMap<>(); for (Map.Entry 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 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/api/src/test/java/org/openmrs/module/appframework/factory/AppConfigurationLoaderFactoryTest.java b/api/src/test/java/org/openmrs/module/appframework/factory/AppConfigurationLoaderFactoryTest.java index ad0438d..6af9943 100644 --- a/api/src/test/java/org/openmrs/module/appframework/factory/AppConfigurationLoaderFactoryTest.java +++ b/api/src/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 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 index f6bf40a..817698a 100644 --- 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 index 3125aa2..cb1084d 100644 --- 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 index e851da8..5b0c395 100644 --- a/omod/pom.xml +++ b/omod/pom.xml @@ -72,7 +72,12 @@ pom test - + + + jakarta.xml.bind + jakarta.xml.bind-api + + @@ -86,7 +91,7 @@ 3.18.2-GA test - + 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 index 92247be..cbe6fb4 100644 --- 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 index c673622..9c01714 100644 --- a/pom.xml +++ b/pom.xml @@ -35,10 +35,11 @@ - 1.9.9 + 2.0.0 2.21.0 UTF-8 1.1.2 + 19.0.0 @@ -137,6 +138,32 @@ + + org.graalvm.js + js + ${graalvm.version} + + + + org.graalvm.js + js-scriptengine + ${graalvm.version} + + + + com.thoughtworks.xstream + xstream + 1.4.17 + test + + + + jakarta.xml.bind + jakarta.xml.bind-api + test + 2.3.2 + + @@ -147,8 +174,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.6 - 1.6 + 1.7 + 1.7 @@ -194,10 +221,6 @@ org.apache.maven.plugins maven-surefire-plugin - 2.22.1 - - -Djdk.net.URLClassPath.disableClassPathURLCheck=true - @@ -234,6 +257,29 @@ https://mavenrepo.openmrs.org/snapshots + + + + Java 17 + + 17 + + + + + org.apache.maven.plugins + maven-surefire-plugin + + --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 + + + + + + +