This is an automated email from the ASF dual-hosted git repository. radu pushed a commit to branch issue/SLING-12635 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly-js-provider.git
commit e0a6ae7addae480a4002bec2f59ba75223403c3c Author: Radu Cotescu <[email protected]> AuthorDate: Fri Jan 24 11:57:37 2025 +0100 SLING-12635 - JS Use Scripts should only be read on-demand * read all scripts lazily, only when they really need to be loaded/reloaded --- pom.xml | 6 -- .../scripting/sightly/js/impl/JsEnvironment.java | 13 ++- .../js/impl/jsapi/SlyBindingsValuesProvider.java | 56 +++++------ .../sightly/js/impl/use/DependencyResolver.java | 105 +++++++++------------ 4 files changed, 83 insertions(+), 97 deletions(-) diff --git a/pom.xml b/pom.xml index f63cfbe..1e3244e 100644 --- a/pom.xml +++ b/pom.xml @@ -134,12 +134,6 @@ <version>3.5</version> <scope>provided</scope> </dependency> - <dependency> - <groupId>commons-io</groupId> - <artifactId>commons-io</artifactId> - <version>2.14.0</version> - <scope>provided</scope> - </dependency> <dependency> <groupId>javax.servlet</groupId> <artifactId>javax.servlet-api</artifactId> diff --git a/src/main/java/org/apache/sling/scripting/sightly/js/impl/JsEnvironment.java b/src/main/java/org/apache/sling/scripting/sightly/js/impl/JsEnvironment.java index 8e36e8a..577d603 100644 --- a/src/main/java/org/apache/sling/scripting/sightly/js/impl/JsEnvironment.java +++ b/src/main/java/org/apache/sling/scripting/sightly/js/impl/JsEnvironment.java @@ -18,6 +18,8 @@ ******************************************************************************/ package org.apache.sling.scripting.sightly.js.impl; +import java.io.Closeable; + import javax.script.Bindings; import javax.script.Compilable; import javax.script.ScriptContext; @@ -25,7 +27,6 @@ import javax.script.ScriptEngine; import javax.script.ScriptException; import javax.script.SimpleScriptContext; -import org.apache.commons.io.IOUtils; import org.apache.sling.api.scripting.LazyBindings; import org.apache.sling.scripting.core.ScriptNameAwareReader; import org.apache.sling.scripting.sightly.SightlyException; @@ -129,10 +130,18 @@ public class JsEnvironment { } catch (ScriptException e) { throw new SightlyException(e); } finally { - IOUtils.closeQuietly(reader); + closeQuietly(reader); } }); } + private void closeQuietly(Closeable closeable) { + try { + closeable.close(); + } catch (Exception e) { + // ignore + } + } + } diff --git a/src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/SlyBindingsValuesProvider.java b/src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/SlyBindingsValuesProvider.java index 31351e9..8bb98d0 100644 --- a/src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/SlyBindingsValuesProvider.java +++ b/src/main/java/org/apache/sling/scripting/sightly/js/impl/jsapi/SlyBindingsValuesProvider.java @@ -21,7 +21,6 @@ package org.apache.sling.scripting.sightly.js.impl.jsapi; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; @@ -32,7 +31,6 @@ import javax.script.Bindings; import javax.script.ScriptEngine; import javax.servlet.http.HttpServletRequest; -import org.apache.commons.io.IOUtils; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.scripting.LazyBindings; @@ -145,12 +143,12 @@ public class SlyBindingsValuesProvider { @Activate protected void activate(Configuration configuration) { - String[] factories = PropertiesUtil.toStringArray( + String[] configuredFactories = PropertiesUtil.toStringArray( configuration.org_apache_sling_scripting_sightly_js_bindings(), new String[]{SLING_NS_PATH} ); - scriptPaths = new LinkedHashMap<>(factories.length); - for (String f : factories) { + scriptPaths = new LinkedHashMap<>(configuredFactories.length); + for (String f : configuredFactories) { String[] parts = f.split(":"); if (parts.length == 2) { scriptPaths.put(parts[0], parts[1]); @@ -197,24 +195,24 @@ public class SlyBindingsValuesProvider { if (resource == null) { throw new SightlyException("Sly namespace loader could not find the following script: " + path); } - try { - AsyncContainer container = - jsEnvironment.runScript( - new ScriptNameAwareReader( - new StringReader(IOUtils.toString(resource.adaptTo(InputStream.class), StandardCharsets.UTF_8)), - resource.getPath() - ), - createBindings(bindings, resource.getPath()), - new LazyBindings() - ); - Object obj = container.getResult(); - if (!(obj instanceof Function)) { - throw new SightlyException("Script " + path + " was expected to return a function."); - } - return (Function) obj; - } catch (IOException e) { - throw new SightlyException("Cannot read script " + path + " ."); + InputStream inputStream = resource.adaptTo(InputStream.class); + if (inputStream == null) { + throw new SightlyException("Sly namespace loader could not read the following script: " + path); } + AsyncContainer container = + jsEnvironment.runScript( + new ScriptNameAwareReader( + new InputStreamReader(inputStream, StandardCharsets.UTF_8), + resource.getPath() + ), + createBindings(bindings, resource.getPath()), + new LazyBindings() + ); + Object obj = container.getResult(); + if (!(obj instanceof Function)) { + throw new SightlyException("Script " + path + " was expected to return a function."); + } + return (Function) obj; } private Bindings createBindings(Bindings global, String factoryPath) { @@ -268,14 +266,12 @@ public class SlyBindingsValuesProvider { Context context = Context.enter(); context.initStandardObjects(); context.setOptimizationLevel(9); - InputStream reader = null; - try { - Resource resource = resolver.getResource(Q_PATH); - if (resource == null) { - LOGGER.warn("Could not load Q library at path: " + Q_PATH); - return null; - } - reader = resource.adaptTo(InputStream.class); + Resource resource = resolver.getResource(Q_PATH); + if (resource == null) { + LOGGER.warn("Could not load Q library at path: " + Q_PATH); + return null; + } + try (InputStream reader = resource.adaptTo(InputStream.class)) { if (reader == null) { LOGGER.warn("Could not read content of Q library"); return null; diff --git a/src/main/java/org/apache/sling/scripting/sightly/js/impl/use/DependencyResolver.java b/src/main/java/org/apache/sling/scripting/sightly/js/impl/use/DependencyResolver.java index bfca8c8..ed3bd66 100644 --- a/src/main/java/org/apache/sling/scripting/sightly/js/impl/use/DependencyResolver.java +++ b/src/main/java/org/apache/sling/scripting/sightly/js/impl/use/DependencyResolver.java @@ -19,15 +19,13 @@ package org.apache.sling.scripting.sightly.js.impl.use; -import java.io.IOException; -import java.io.InputStream; -import java.io.StringReader; -import java.nio.charset.StandardCharsets; - import javax.script.Bindings; import javax.script.ScriptEngine; -import org.apache.commons.io.IOUtils; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; + import org.apache.commons.lang3.StringUtils; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.resource.Resource; @@ -56,71 +54,60 @@ public class DependencyResolver { if (!Utils.isJsScript(dependency)) { throw new SightlyException("Only JS scripts are allowed as dependencies. Invalid dependency: " + dependency); } - ScriptNameAwareReader reader = null; - IOException ioException = null; - try { - // attempt to retrieve the dependency directly (as an absolute path or relative to the search paths) - Resource scriptResource = scriptingResourceResolver.getResource(dependency); - Resource caller = getCaller(bindings); - if (caller != null) { - Resource callerType = caller.getParent(); - if (scriptResource == null && callerType != null) { - SlingHttpServletRequest request = (SlingHttpServletRequest) bindings.get(SlingBindings.REQUEST); - String driverType = request.getResource().getResourceType(); - Resource driver = scriptingResourceResolver.getResource(driverType); - if (driver != null) { - Resource hierarchyResource = getHierarchyResource(callerType, driver); - while (hierarchyResource != null && scriptResource == null) { - if (dependency.startsWith("..")) { - // relative path - String absolutePath = ResourceUtil.normalize(hierarchyResource.getPath() + "/" + dependency); - if (StringUtils.isNotEmpty(absolutePath)) { - scriptResource = scriptingResourceResolver.getResource(absolutePath); - } - } else { - scriptResource = hierarchyResource.getChild(dependency); - } - if (scriptResource == null) { - String nextType = hierarchyResource.getResourceSuperType(); - if (nextType != null) { - hierarchyResource = scriptingResourceResolver.getResource(nextType); - } else { - hierarchyResource = null; - } - } - } - } - // cannot find a dependency relative to the resource type; locate it solely based on the caller - if (scriptResource == null) { + // attempt to retrieve the dependency directly (as an absolute path or relative to the search paths) + Resource scriptResource = scriptingResourceResolver.getResource(dependency); + Resource caller = getCaller(bindings); + if (caller != null) { + Resource callerType = caller.getParent(); + if (scriptResource == null && callerType != null) { + SlingHttpServletRequest request = (SlingHttpServletRequest) bindings.get(SlingBindings.REQUEST); + String driverType = request.getResource().getResourceType(); + Resource driver = scriptingResourceResolver.getResource(driverType); + if (driver != null) { + Resource hierarchyResource = getHierarchyResource(callerType, driver); + while (hierarchyResource != null && scriptResource == null) { if (dependency.startsWith("..")) { // relative path - String absolutePath = ResourceUtil.normalize(callerType.getPath() + "/" + dependency); + String absolutePath = ResourceUtil.normalize(hierarchyResource.getPath() + "/" + dependency); if (StringUtils.isNotEmpty(absolutePath)) { scriptResource = scriptingResourceResolver.getResource(absolutePath); } } else { - scriptResource = callerType.getChild(dependency); + scriptResource = hierarchyResource.getChild(dependency); + } + if (scriptResource == null) { + String nextType = hierarchyResource.getResourceSuperType(); + if (nextType != null) { + hierarchyResource = scriptingResourceResolver.getResource(nextType); + } else { + hierarchyResource = null; + } } } } + // cannot find a dependency relative to the resource type; locate it solely based on the caller + if (scriptResource == null) { + if (dependency.startsWith("..")) { + // relative path + String absolutePath = ResourceUtil.normalize(callerType.getPath() + "/" + dependency); + if (StringUtils.isNotEmpty(absolutePath)) { + scriptResource = scriptingResourceResolver.getResource(absolutePath); + } + } else { + scriptResource = callerType.getChild(dependency); + } + } } - if (scriptResource == null) { - throw new SightlyException(String.format("Unable to load script dependency %s.", dependency)); - } - InputStream scriptStream = scriptResource.adaptTo(InputStream.class); - if (scriptStream == null) { - throw new SightlyException(String.format("Unable to read script %s.", dependency)); - } - reader = new ScriptNameAwareReader(new StringReader(IOUtils.toString(scriptStream, StandardCharsets.UTF_8)), - scriptResource.getPath()); - IOUtils.closeQuietly(scriptStream); - } catch (IOException e) { - ioException = e; } - if (ioException != null) { - throw new SightlyException(String.format("Unable to load script dependency %s.", dependency), ioException); + if (scriptResource == null) { + throw new SightlyException(String.format("Unable to load script dependency %s.", dependency)); + } + InputStream scriptStream = scriptResource.adaptTo(InputStream.class); + if (scriptStream == null) { + throw new SightlyException(String.format("Unable to read script %s.", dependency)); } - return reader; + return new ScriptNameAwareReader(new InputStreamReader(scriptStream, StandardCharsets.UTF_8), + scriptResource.getPath()); } private Resource getCaller(Bindings bindings) {
