You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2019/05/22 08:38:25 UTC

[sling-org-apache-sling-scripting-core] branch master updated (8e32b80 -> 1b5b0f1)

This is an automated email from the ASF dual-hosted git repository.

radu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git.


    from 8e32b80  Updating badges for org-apache-sling-scripting-core
     new 7114133  SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM
     new 11d59c1  SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM
     new 1b5b0f1  SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../core/impl/jsr223/SlingScriptEngineManager.java | 18 ++++++--
 .../impl/jsr223/SlingScriptEngineManagerTest.java  | 54 +++++++++++++---------
 2 files changed, 47 insertions(+), 25 deletions(-)


[sling-org-apache-sling-scripting-core] 01/03: SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git

commit 7114133f465b608337550aa64a189cf15aa8967f
Author: Paul Bjorkstrand <pa...@gmail.com>
AuthorDate: Mon May 20 10:23:20 2019 -0500

    SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM
    
    * fix NPE on GraalVM
---
 .../core/impl/jsr223/SlingScriptEngineManager.java | 12 +++--
 .../impl/jsr223/SlingScriptEngineManagerTest.java  | 53 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
index 34d975a..b5f800a 100644
--- a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
+++ b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
@@ -293,13 +293,19 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
 
     private void registerAssociations(ScriptEngineFactory factory) {
         for (String extension : factory.getExtensions()) {
-            internalManager.registerEngineExtension(extension, factory);
+            if (extension != null && !extension.isEmpty()) {
+                internalManager.registerEngineExtension(extension, factory);
+            }
         }
         for (String mimeType : factory.getMimeTypes()) {
-            internalManager.registerEngineMimeType(mimeType, factory);
+            if (mimeType != null && !mimeType.isEmpty()) {
+                internalManager.registerEngineMimeType(mimeType, factory);
+            }
         }
         for (String name : factory.getNames()) {
-            internalManager.registerEngineName(name, factory);
+            if (name != null && !name.isEmpty()) {
+                internalManager.registerEngineName(name, factory);
+            }
         }
     }
 
diff --git a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
index b61757f..e737ede 100644
--- a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
+++ b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
@@ -72,15 +72,20 @@ public class SlingScriptEngineManagerTest {
 
     @Test
     public void testPlatformScriptEngines() {
+        int jvmProvidedScriptEngineFactoryCount = jvmProvidedScriptEngineFactoryCount();
         ScriptEngineManager scriptEngineManager = context.getService(ScriptEngineManager.class);
         assertNotNull("Expected a ScriptEngineManager would be already registered.", scriptEngineManager);
-        assertEquals("The ScriptEngineManager should have had 1 ScriptEngineFactory registered.", 1, scriptEngineManager
-                .getEngineFactories().size());
+        {
+            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount;
+            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactory registered.", expectedScriptEngineFactories, scriptEngineManager
+                    .getEngineFactories().size());
+        }
     }
 
     @Test
     public void testOSGiRegisteredFactoriesDifferentServiceRanking() throws Exception {
-        int expectedEvents = 3;
+        int jvmProvidedScriptEngineFactoryCount = jvmProvidedScriptEngineFactoryCount();
+        int expectedEvents = jvmProvidedScriptEngineFactoryCount + 1;
         CountDownLatch latch = new CountDownLatch(expectedEvents);
         TestEventHandler eventHandler = new TestEventHandler
                 (latch, "org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager/UPDATED");
@@ -108,9 +113,12 @@ public class SlingScriptEngineManagerTest {
         ScriptEngineManager scriptEngineManager = context.getService(ScriptEngineManager.class);
         assertNotNull("Expected a ScriptEngineManager would be already registered.", scriptEngineManager);
         List<ScriptEngineFactory> factories = scriptEngineManager.getEngineFactories();
-        assertEquals("The ScriptEngineManager should have had 3 ScriptEngineFactories registered.", 3, factories.size());
-        assertEquals(f1.getEngineName(), factories.get(2).getEngineName());
-        assertEquals(f2.getEngineName(), factories.get(1).getEngineName());
+        {
+            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 2;
+            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
+        }
+        assertEquals(f1.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount + 1).getEngineName());
+        assertEquals(f2.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
 
         SlingScriptEngineManager slingScriptEngineManager = context.getService(SlingScriptEngineManager.class);
         assertEquals(2, slingScriptEngineManager.getProperties(f1).get(Constants.SERVICE_RANKING));
@@ -118,8 +126,11 @@ public class SlingScriptEngineManagerTest {
         f1SR.unregister();
 
         factories = scriptEngineManager.getEngineFactories();
-        assertEquals("The ScriptEngineManager should have had 2 ScriptEngineFactories registered.", 2, factories.size());
-        assertEquals(f2.getEngineName(), factories.get(1).getEngineName());
+        {
+            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 1;
+            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
+        }
+        assertEquals(f2.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
 
         assertEquals(f2, scriptEngineManager.getEngineByName("f2").getFactory());
         assertEquals(f2, scriptEngineManager.getEngineByExtension("f2").getFactory());
@@ -138,6 +149,7 @@ public class SlingScriptEngineManagerTest {
 
     @Test
     public void testBundledScriptEngineFactory() throws Exception {
+        int jvmProvidedScriptEngineFactoryCount = jvmProvidedScriptEngineFactoryCount();
         final URL url = createFactoryFile().toURI().toURL();
         Bundle bundle = mock(Bundle.class);
         BundleWiring wiring = mock(BundleWiring.class);
@@ -166,14 +178,20 @@ public class SlingScriptEngineManagerTest {
         assertNotNull("Expected that the SlingScriptEngineManager would already be registered.", slingScriptEngineManager);
         slingScriptEngineManager.bundleChanged(bundleEvent);
         List<ScriptEngineFactory> factories = slingScriptEngineManager.getEngineFactories();
-        assertEquals("Expected 2 ScriptEngineFactories.", 2, factories.size());
-        assertEquals("Dummy Scripting Engine", factories.get(1).getEngineName());
+        {
+            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 1;
+            assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactories.", expectedScriptEngineFactories, factories.size());
+        }
+        assertEquals("Dummy Scripting Engine", factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
 
         bundleEvent = new BundleEvent(BundleEvent.STOPPED, bundle);
         slingScriptEngineManager.bundleChanged(bundleEvent);
         factories = slingScriptEngineManager.getEngineFactories();
-        assertEquals("Expected 1 ScriptEngineFactory.", 1, factories.size());
-        assertEquals("Oracle Nashorn", factories.get(0).getEngineName());
+        {
+            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount;
+            assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactory.", expectedScriptEngineFactories, factories.size());
+        }
+        //assertEquals("Oracle Nashorn", factories.get(0).getEngineName());
         assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory", slingScriptEngineManager
                 .getEngineByExtension("dummy"));
         assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory",
@@ -182,6 +200,17 @@ public class SlingScriptEngineManagerTest {
                 slingScriptEngineManager.getEngineByName("Dummy"));
     }
 
+    private int jvmProvidedScriptEngineFactoryCount() {
+        ClassLoader loader = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(null);
+            return new ScriptEngineManager(ClassLoader.getSystemClassLoader()).getEngineFactories().size();
+        }
+        finally {
+            Thread.currentThread().setContextClassLoader(loader);
+        }
+    }
+
     private ScriptEngineFactory mockScriptEngineFactory(String engineName, String engineVersion, List<String> extensions, String
             languageName, String languageVersion, List<String> mimeTypes) {
         ScriptEngineFactory factory = mock(ScriptEngineFactory.class);


[sling-org-apache-sling-scripting-core] 03/03: SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git

commit 1b5b0f13ac8190bb02be6c3bc6018e696e20b355
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Wed May 22 10:35:12 2019 +0200

    SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM
    
    * corrected SlingScriptEngineManagerTest.testOSGiRegisteredFactoriesDifferentServiceRanking
---
 .../core/impl/jsr223/SlingScriptEngineManager.java |  6 +--
 .../impl/jsr223/SlingScriptEngineManagerTest.java  | 57 ++++++++--------------
 2 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
index fe7b425..14cb13a 100644
--- a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
+++ b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
@@ -296,21 +296,21 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
             if (extension != null && !extension.isEmpty()) {
                 internalManager.registerEngineExtension(extension, factory);
             } else {
-                LOG.warn("Could not register an empty or null extension for script engine factory {}", factory.getEngineName());
+                LOG.warn("Could not register an empty or null extension for script engine factory {}.", factory.getEngineName());
             }
         }
         for (String mimeType : factory.getMimeTypes()) {
             if (mimeType != null && !mimeType.isEmpty()) {
                 internalManager.registerEngineMimeType(mimeType, factory);
             } else {
-                LOG.warn("Could not register an empty or null mime type for script engine factory {}", factory.getEngineName());
+                LOG.warn("Could not register an empty or null mime type for script engine factory {}.", factory.getEngineName());
             }
         }
         for (String name : factory.getNames()) {
             if (name != null && !name.isEmpty()) {
                 internalManager.registerEngineName(name, factory);
             } else {
-                LOG.warn("Could not register an empty or null engine name for script engine factory {}", factory.getEngineName());
+                LOG.warn("Could not register an empty or null engine name for script engine factory {}.", factory.getEngineName());
             }
         }
     }
diff --git a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
index e737ede..4f28324 100644
--- a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
+++ b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
@@ -40,8 +40,6 @@ import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleEvent;
 import org.osgi.framework.Constants;
@@ -75,17 +73,15 @@ public class SlingScriptEngineManagerTest {
         int jvmProvidedScriptEngineFactoryCount = jvmProvidedScriptEngineFactoryCount();
         ScriptEngineManager scriptEngineManager = context.getService(ScriptEngineManager.class);
         assertNotNull("Expected a ScriptEngineManager would be already registered.", scriptEngineManager);
-        {
-            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount;
-            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactory registered.", expectedScriptEngineFactories, scriptEngineManager
-                    .getEngineFactories().size());
-        }
+        assertEquals("The ScriptEngineManager should have had " + jvmProvidedScriptEngineFactoryCount + " ScriptEngineFactory registered.", jvmProvidedScriptEngineFactoryCount, scriptEngineManager.getEngineFactories().size());
     }
 
     @Test
     public void testOSGiRegisteredFactoriesDifferentServiceRanking() throws Exception {
+        int numberOfOSGiRegisteredFactories = 2;
         int jvmProvidedScriptEngineFactoryCount = jvmProvidedScriptEngineFactoryCount();
-        int expectedEvents = jvmProvidedScriptEngineFactoryCount + 1;
+        // we register 2 factories, then unregister 1 of them
+        int expectedEvents = 3;
         CountDownLatch latch = new CountDownLatch(expectedEvents);
         TestEventHandler eventHandler = new TestEventHandler
                 (latch, "org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager/UPDATED");
@@ -113,24 +109,20 @@ public class SlingScriptEngineManagerTest {
         ScriptEngineManager scriptEngineManager = context.getService(ScriptEngineManager.class);
         assertNotNull("Expected a ScriptEngineManager would be already registered.", scriptEngineManager);
         List<ScriptEngineFactory> factories = scriptEngineManager.getEngineFactories();
-        {
-            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 2;
-            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
-        }
-        assertEquals(f1.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount + 1).getEngineName());
-        assertEquals(f2.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
+        int expectedScriptEngineFactories = numberOfOSGiRegisteredFactories + jvmProvidedScriptEngineFactoryCount;
+        assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
+        assertEquals(f1.getEngineName(), factories.get(expectedScriptEngineFactories - 1).getEngineName());
+        assertEquals(f2.getEngineName(), factories.get(expectedScriptEngineFactories - 2).getEngineName());
 
         SlingScriptEngineManager slingScriptEngineManager = context.getService(SlingScriptEngineManager.class);
         assertEquals(2, slingScriptEngineManager.getProperties(f1).get(Constants.SERVICE_RANKING));
 
         f1SR.unregister();
+        expectedScriptEngineFactories--;
 
         factories = scriptEngineManager.getEngineFactories();
-        {
-            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 1;
-            assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
-        }
-        assertEquals(f2.getEngineName(), factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
+        assertEquals("The ScriptEngineManager should have had " + expectedScriptEngineFactories + " ScriptEngineFactories registered.", expectedScriptEngineFactories, factories.size());
+        assertEquals(f2.getEngineName(), factories.get(expectedScriptEngineFactories - 1).getEngineName());
 
         assertEquals(f2, scriptEngineManager.getEngineByName("f2").getFactory());
         assertEquals(f2, scriptEngineManager.getEngineByExtension("f2").getFactory());
@@ -178,20 +170,16 @@ public class SlingScriptEngineManagerTest {
         assertNotNull("Expected that the SlingScriptEngineManager would already be registered.", slingScriptEngineManager);
         slingScriptEngineManager.bundleChanged(bundleEvent);
         List<ScriptEngineFactory> factories = slingScriptEngineManager.getEngineFactories();
-        {
-            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 1;
-            assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactories.", expectedScriptEngineFactories, factories.size());
-        }
-        assertEquals("Dummy Scripting Engine", factories.get(jvmProvidedScriptEngineFactoryCount).getEngineName());
+        int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount + 1;
+        assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactories.", expectedScriptEngineFactories, factories.size());
+        assertEquals("Dummy Scripting Engine", factories.get(expectedScriptEngineFactories - 1).getEngineName());
 
         bundleEvent = new BundleEvent(BundleEvent.STOPPED, bundle);
         slingScriptEngineManager.bundleChanged(bundleEvent);
+        expectedScriptEngineFactories--;
+
         factories = slingScriptEngineManager.getEngineFactories();
-        {
-            int expectedScriptEngineFactories = jvmProvidedScriptEngineFactoryCount;
-            assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactory.", expectedScriptEngineFactories, factories.size());
-        }
-        //assertEquals("Oracle Nashorn", factories.get(0).getEngineName());
+        assertEquals("Expected " + expectedScriptEngineFactories + " ScriptEngineFactory.", expectedScriptEngineFactories, factories.size());
         assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory", slingScriptEngineManager
                 .getEngineByExtension("dummy"));
         assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory",
@@ -230,16 +218,9 @@ public class SlingScriptEngineManagerTest {
     private File createFactoryFile() throws IOException {
         File tempFile = File.createTempFile("scriptEngine", "tmp");
         tempFile.deleteOnExit();
-
-        FileOutputStream fos = null;
-        try {
-            fos = new FileOutputStream(tempFile);
+        try (FileOutputStream fos = new FileOutputStream(tempFile)) {
             fos.write("#I'm a test-comment\n".getBytes());
             fos.write(SCRIPT_ENGINE_FACTORY.getName().getBytes());
-        } finally {
-            if (fos != null) {
-                fos.close();
-            }
         }
         return tempFile;
     }
@@ -251,7 +232,7 @@ public class SlingScriptEngineManagerTest {
         CountDownLatch latch;
         int processedEvents = 0;
 
-        public TestEventHandler(CountDownLatch latch, String topic) {
+        TestEventHandler(CountDownLatch latch, String topic) {
             this.topic = topic;
             this.latch = latch;
         }


[sling-org-apache-sling-scripting-core] 02/03: SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git

commit 11d59c1d8dba37fc4090057ca96b11f0853374a7
Author: Paul Bjorkstrand <pa...@gmail.com>
AuthorDate: Mon May 20 16:19:58 2019 -0500

    SLING-8425 - NPE in SlingScriptEngineManager when Sling is run on GraalVM
    
    * add else case/logging to the script engine registration loops
---
 .../sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java  | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
index b5f800a..fe7b425 100644
--- a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
+++ b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
@@ -295,16 +295,22 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
         for (String extension : factory.getExtensions()) {
             if (extension != null && !extension.isEmpty()) {
                 internalManager.registerEngineExtension(extension, factory);
+            } else {
+                LOG.warn("Could not register an empty or null extension for script engine factory {}", factory.getEngineName());
             }
         }
         for (String mimeType : factory.getMimeTypes()) {
             if (mimeType != null && !mimeType.isEmpty()) {
                 internalManager.registerEngineMimeType(mimeType, factory);
+            } else {
+                LOG.warn("Could not register an empty or null mime type for script engine factory {}", factory.getEngineName());
             }
         }
         for (String name : factory.getNames()) {
             if (name != null && !name.isEmpty()) {
                 internalManager.registerEngineName(name, factory);
+            } else {
+                LOG.warn("Could not register an empty or null engine name for script engine factory {}", factory.getEngineName());
             }
         }
     }