SLING-7306 - The JS Use Provider bundle should explicitly depend on the Rhino Script...
authorRadu Cotescu <radu@apache.org>
Wed, 13 Dec 2017 16:29:22 +0000 (17:29 +0100)
committerRadu Cotescu <radu@apache.org>
Wed, 13 Dec 2017 16:29:22 +0000 (17:29 +0100)
* explicitly ask for the Rhino script engine in the JsUseProvider; fail early if the
engine is not available
* improved used resource resolver

src/main/java/org/apache/sling/scripting/sightly/js/impl/JsEnvironment.java
src/main/java/org/apache/sling/scripting/sightly/js/impl/JsUseProvider.java
src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/ProxyAsyncScriptableFactory.java
src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/SlyBindingsValuesProvider.java

index 6175ce6..2e88a35 100644 (file)
@@ -21,6 +21,8 @@ package org.apache.sling.scripting.sightly.js.impl;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+
 import javax.script.Bindings;
 import javax.script.Compilable;
 import javax.script.ScriptContext;
@@ -115,11 +117,11 @@ public class JsEnvironment {
                 try {
                     Object result;
                     if (jsEngine instanceof Compilable) {
-                        reader = new ScriptNameAwareReader(new InputStreamReader(scriptResource.adaptTo(InputStream.class)),
-                                scriptResource.getPath());
+                        reader = new ScriptNameAwareReader(new InputStreamReader(scriptResource.adaptTo(InputStream.class),
+                                StandardCharsets.UTF_8), scriptResource.getPath());
                         result = ((Compilable) jsEngine).compile(reader).eval(scriptContext);
                     } else {
-                        reader = new InputStreamReader(scriptResource.adaptTo(InputStream.class));
+                        reader = new InputStreamReader(scriptResource.adaptTo(InputStream.class), StandardCharsets.UTF_8);
                         result = jsEngine.eval(reader, scriptContext);
                     }
                     if (commonJsModule.isModified()) {
index ef8d035..948cef3 100644 (file)
@@ -38,10 +38,6 @@ import org.osgi.framework.Constants;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.metatype.annotations.AttributeDefinition;
-import org.osgi.service.metatype.annotations.Designate;
-import org.osgi.service.metatype.annotations.ObjectClassDefinition;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Use provider for JavaScript Use-API objects.
@@ -66,18 +62,17 @@ public class JsUseProvider implements UseProvider {
 
     }
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(JsUseProvider.class);
-    private static final String JS_ENGINE_NAME = "javascript";
+    private static final String JS_ENGINE_NAME = "rhino";
     private static final JsValueAdapter jsValueAdapter = new JsValueAdapter(new AsyncExtractor());
 
     @Reference
-    private ScriptEngineManager scriptEngineManager = null;
+    private ScriptEngineManager scriptEngineManager;
 
     @Reference
-    private ProxyAsyncScriptableFactory proxyAsyncScriptableFactory = null;
+    private ProxyAsyncScriptableFactory proxyAsyncScriptableFactory;
 
     @Reference
-    private ScriptingResourceResolverProvider scriptingResourceResolverProvider = null;
+    private ScriptingResourceResolverProvider scriptingResourceResolverProvider;
 
     @Override
     public ProviderOutcome provide(String identifier, RenderContext renderContext, Bindings arguments) {
@@ -87,7 +82,7 @@ public class JsUseProvider implements UseProvider {
         }
         ScriptEngine jsEngine = scriptEngineManager.getEngineByName(JS_ENGINE_NAME);
         if (jsEngine == null) {
-            return ProviderOutcome.failure(new SightlyException("No JavaScript engine was defined."));
+            return ProviderOutcome.failure(new SightlyException("Failed to obtain a " + JS_ENGINE_NAME + " JavaScript engine."));
         }
         SlingScriptHelper scriptHelper = Utils.getHelper(globalBindings);
         JsEnvironment environment = null;
@@ -98,7 +93,7 @@ public class JsUseProvider implements UseProvider {
             Resource callerScript = slingScriptingResolver.getResource(scriptHelper.getScript().getScriptResource().getPath());
             Resource scriptResource = Utils.getScriptResource(callerScript, identifier, globalBindings);
             globalBindings.put(ScriptEngine.FILENAME, scriptResource.getPath());
-            proxyAsyncScriptableFactory.registerProxies(globalBindings);
+            proxyAsyncScriptableFactory.registerProxies(slingScriptingResolver, environment, globalBindings);
             AsyncContainer asyncContainer = environment.runResource(scriptResource, globalBindings, arguments);
             return ProviderOutcome.success(jsValueAdapter.adapt(asyncContainer));
         } finally {
index ae8c200..00b1ee5 100644 (file)
@@ -24,6 +24,8 @@ import javax.script.ScriptEngine;
 import javax.script.SimpleBindings;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.scripting.sightly.js.impl.JsEnvironment;
 import org.apache.sling.scripting.sightly.js.impl.rhino.HybridObject;
 import org.mozilla.javascript.Scriptable;
 import org.mozilla.javascript.ScriptableObject;
@@ -43,8 +45,8 @@ public class ProxyAsyncScriptableFactory {
     @Reference
     private SlyBindingsValuesProvider slyBindingsValuesProvider = null;
 
-    public void registerProxies(Bindings bindings) {
-        slyBindingsValuesProvider.initialise(bindings);
+    public void registerProxies(ResourceResolver resourceResolver, JsEnvironment environment, Bindings bindings) {
+        slyBindingsValuesProvider.initialise(resourceResolver, environment, bindings);
         Bindings bindingsCopy = new SimpleBindings();
         for (String factoryName : slyBindingsValuesProvider.getScriptPaths().keySet()) {
             ShadowScriptableObject shadowScriptableObject = new ShadowScriptableObject(factoryName, bindingsCopy);
index 9c8ba88..87905a1 100644 (file)
@@ -21,21 +21,19 @@ package org.apache.sling.scripting.sightly.js.impl.jsapi;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
+
 import javax.script.Bindings;
-import javax.script.ScriptEngine;
-import javax.script.ScriptEngineManager;
 import javax.script.SimpleBindings;
 import javax.servlet.http.HttpServletRequest;
 
 import org.apache.commons.io.IOUtils;
-import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.api.resource.ResourceResolverFactory;
 import org.apache.sling.api.scripting.SlingBindings;
 import org.apache.sling.commons.osgi.PropertiesUtil;
 import org.apache.sling.scripting.sightly.SightlyException;
@@ -48,7 +46,6 @@ import org.apache.sling.scripting.sightly.js.impl.async.TimingFunction;
 import org.apache.sling.scripting.sightly.js.impl.cjs.CommonJsModule;
 import org.apache.sling.scripting.sightly.js.impl.rhino.HybridObject;
 import org.apache.sling.scripting.sightly.js.impl.rhino.JsValueAdapter;
-import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.mozilla.javascript.Context;
 import org.mozilla.javascript.Function;
 import org.mozilla.javascript.Script;
@@ -58,7 +55,6 @@ import org.osgi.service.component.ComponentContext;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Deactivate;
-import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.metatype.annotations.AttributeDefinition;
 import org.osgi.service.metatype.annotations.Designate;
 import org.osgi.service.metatype.annotations.ObjectClassDefinition;
@@ -104,15 +100,6 @@ public class SlyBindingsValuesProvider {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(SlyBindingsValuesProvider.class);
 
-    @Reference
-    private ScriptEngineManager scriptEngineManager = null;
-
-    @Reference
-    private ResourceResolverFactory rrf = null;
-
-    @Reference
-    private ServiceUserMapped serviceUserMapped;
-
     private final AsyncExtractor asyncExtractor = new AsyncExtractor();
     private final JsValueAdapter jsValueAdapter = new JsValueAdapter(asyncExtractor);
 
@@ -122,9 +109,9 @@ public class SlyBindingsValuesProvider {
     private Script qScript;
     private final ScriptableObject qScope = createQScope();
 
-    public void initialise(Bindings bindings) {
+    public void initialise(ResourceResolver resourceResolver, JsEnvironment environment, Bindings bindings) {
         if (needsInit()) {
-            init(bindings);
+            init(resourceResolver, environment, bindings);
         }
     }
 
@@ -192,37 +179,13 @@ public class SlyBindingsValuesProvider {
         return factories == null || factories.isEmpty() || qScript == null;
     }
 
-    private synchronized void init(Bindings bindings) {
+    private synchronized void init(ResourceResolver resourceResolver, JsEnvironment jsEnvironment, Bindings bindings) {
         if (needsInit()) {
-            ensureFactoriesLoaded(bindings);
-        }
-    }
-
-    private void ensureFactoriesLoaded(Bindings bindings) {
-        JsEnvironment jsEnvironment = null;
-        ResourceResolver resolver = null;
-        try {
-            ScriptEngine scriptEngine = obtainEngine();
-            if (scriptEngine == null) {
-                return;
-            }
-            jsEnvironment = new JsEnvironment(scriptEngine);
-            jsEnvironment.initialize();
-            resolver = rrf.getServiceResourceResolver(null);
             factories = new HashMap<>(scriptPaths.size());
             for (Map.Entry<String, String> entry : scriptPaths.entrySet()) {
-                factories.put(entry.getKey(), loadFactory(resolver, jsEnvironment, entry.getValue(), bindings));
-            }
-            qScript = loadQScript(resolver);
-        } catch (LoginException e) {
-            LOGGER.error("Cannot load HTL Use-API factories.", e);
-        } finally {
-            if (jsEnvironment != null) {
-                jsEnvironment.cleanup();
-            }
-            if (resolver != null) {
-                resolver.close();
+                factories.put(entry.getKey(), loadFactory(resourceResolver, jsEnvironment, entry.getValue(), bindings));
             }
+            qScript = loadQScript(resourceResolver);
         }
     }
 
@@ -246,10 +209,6 @@ public class SlyBindingsValuesProvider {
         return bindings;
     }
 
-    private ScriptEngine obtainEngine() {
-        return scriptEngineManager.getEngineByName("javascript");
-    }
-
     private Object obtainQInstance(Context context, Bindings bindings) {
         if (qScript == null) {
             return null;
@@ -305,7 +264,7 @@ public class SlyBindingsValuesProvider {
                 LOGGER.warn("Could not read content of Q library");
                 return null;
             }
-            return context.compileReader(new InputStreamReader(reader), Q_PATH, 0, null);
+            return context.compileReader(new InputStreamReader(reader, StandardCharsets.UTF_8), Q_PATH, 0, null);
         } catch (IOException e) {
             LOGGER.error("Unable to compile the Q library at path " + Q_PATH + ".", e);
         } finally {