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-sightly.git
The following commit(s) were added to refs/heads/master by this push:
new 8bac51c SLING-12755 - Break cyclic dependencies between the
SightlyScriptEngineFactory and the BundledUnitManagerImpl service
8bac51c is described below
commit 8bac51ce361b632b3bb8db915ab50861a3e9d503
Author: Radu Cotescu <[email protected]>
AuthorDate: Wed Apr 23 12:22:48 2025 +0200
SLING-12755 - Break cyclic dependencies between the
SightlyScriptEngineFactory and the BundledUnitManagerImpl service
* removed any potential cyclic dependency between the
SightlyScriptEngineFactory and the BundledUnitManagerImpl service
* reduced the number of required dependencies overall (i.e. the ScriptCache
should only
be held by script engines)
* retrieve the SightlyScriptEngineFactory directly rather than via the
ScriptEngineManager
---
.../impl/engine/SightlyScriptEngineFactory.java | 46 ++++++++++++++++---
.../engine/bundled/BundledUnitManagerImpl.java | 34 ++++++++------
.../engine/extension/use/RenderUnitProvider.java | 53 ++++++----------------
.../engine/extension/use/ScriptUseProvider.java | 34 ++++----------
.../engine/SightlyScriptEngineFactoryTest.java | 13 ++++--
5 files changed, 90 insertions(+), 90 deletions(-)
diff --git
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactory.java
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactory.java
index 5935e1b..2fcc025 100644
---
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactory.java
+++
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactory.java
@@ -21,11 +21,18 @@ package org.apache.sling.scripting.sightly.impl.engine;
import javax.script.ScriptEngine;
import javax.script.ScriptEngineFactory;
+import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.scripting.api.AbstractScriptEngineFactory;
+import org.apache.sling.scripting.api.ScriptCache;
+import org.apache.sling.scripting.sightly.engine.BundledUnitManager;
import
org.apache.sling.scripting.sightly.impl.engine.bundled.BundledUnitManagerImpl;
import
org.apache.sling.scripting.sightly.impl.engine.compiled.SlingHTLMasterCompiler;
+import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceRegistration;
+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.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
@@ -45,32 +52,57 @@ import
org.osgi.service.component.annotations.ReferencePolicyOption;
})
public class SightlyScriptEngineFactory extends AbstractScriptEngineFactory {
+ public static final String SHORT_NAME = "sightly";
+ public static final String EXTENSION = "html";
+ private static final String LANGUAGE_NAME = "The HTL Templating Language";
+ private static final String LANGUAGE_VERSION = "1.4";
+
@Reference(
cardinality = ReferenceCardinality.OPTIONAL,
policy = ReferencePolicy.DYNAMIC,
policyOption = ReferencePolicyOption.GREEDY)
private volatile SlingHTLMasterCompiler slingHTLMasterCompiler;
- @Reference
- private BundledUnitManagerImpl bundledUnitManager;
-
@Reference
private ExtensionRegistryService extensionRegistryService;
@Reference
private SightlyEngineConfiguration configuration;
- public static final String SHORT_NAME = "sightly";
- public static final String EXTENSION = "html";
+ @Reference
+ private ScriptCache scriptCache;
- private static final String LANGUAGE_NAME = "The HTL Templating Language";
- private static final String LANGUAGE_VERSION = "1.4";
+ @Reference
+ private ResourceResolverFactory resourceResolverFactory;
+
+ private ServiceRegistration<BundledUnitManager>
bundledUnitManagerServiceRegistration;
+ private ServiceRegistration<BundledUnitManagerImpl>
bundledUnitManagerImplServiceRegistration;
+ private BundledUnitManagerImpl bundledUnitManager;
public SightlyScriptEngineFactory() {
setNames("htl", "HTL", SHORT_NAME);
setExtensions(EXTENSION);
}
+ @Activate
+ public void activate(BundleContext bundleContext) {
+ bundledUnitManager = new BundledUnitManagerImpl(this, scriptCache,
resourceResolverFactory);
+ this.bundledUnitManagerImplServiceRegistration =
+ bundleContext.registerService(BundledUnitManagerImpl.class,
bundledUnitManager, null);
+ this.bundledUnitManagerServiceRegistration =
+ bundleContext.registerService(BundledUnitManager.class,
bundledUnitManager, null);
+ }
+
+ @Deactivate
+ public void deactivate() {
+ if (bundledUnitManagerServiceRegistration != null) {
+ bundledUnitManagerServiceRegistration.unregister();
+ }
+ if (bundledUnitManagerImplServiceRegistration != null) {
+ bundledUnitManagerImplServiceRegistration.unregister();
+ }
+ }
+
@Override
public String getLanguageName() {
return LANGUAGE_NAME;
diff --git
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/bundled/BundledUnitManagerImpl.java
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/bundled/BundledUnitManagerImpl.java
index d0e2aac..7480d5b 100644
---
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/bundled/BundledUnitManagerImpl.java
+++
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/bundled/BundledUnitManagerImpl.java
@@ -19,7 +19,6 @@
package org.apache.sling.scripting.sightly.impl.engine.bundled;
import javax.script.Bindings;
-import javax.script.ScriptEngineManager;
import javax.script.ScriptException;
import java.io.IOException;
@@ -41,6 +40,7 @@ import org.apache.sling.scripting.core.ScriptNameAwareReader;
import org.apache.sling.scripting.sightly.engine.BundledUnitManager;
import org.apache.sling.scripting.sightly.impl.engine.SightlyCompiledScript;
import org.apache.sling.scripting.sightly.impl.engine.SightlyScriptEngine;
+import
org.apache.sling.scripting.sightly.impl.engine.SightlyScriptEngineFactory;
import org.apache.sling.scripting.sightly.impl.utils.BindingsUtils;
import org.apache.sling.scripting.sightly.render.RenderUnit;
import org.apache.sling.scripting.spi.bundle.BundledRenderUnit;
@@ -48,25 +48,28 @@ import org.apache.sling.scripting.spi.bundle.TypeProvider;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.osgi.framework.wiring.BundleWiring;
-import org.osgi.service.component.annotations.Component;
-import org.osgi.service.component.annotations.Reference;
/**
* This service allows various components to work with {@link
* BundledRenderUnit} instance and perform dependency resolution based on
their availability in
- * the {@link Bindings} maps passed to the HTL Script Engine.
+ * the {@link Bindings} maps passed to the HTL Script Engine. Given the
dependency on the
+ * {@link SightlyScriptEngineFactory}, this service will be registered by the
{@code SightlyScriptEngineFactory} when
+ * the factory is initialised.
*/
-@Component(service = {BundledUnitManagerImpl.class, BundledUnitManager.class})
public class BundledUnitManagerImpl implements BundledUnitManager {
- @Reference
- private ScriptEngineManager scriptEngineManager;
+ private final ScriptCache scriptCache;
+ private final ResourceResolverFactory resourceResolverFactory;
+ private final SightlyScriptEngineFactory sightlyScriptEngineFactory;
- @Reference
- private ScriptCache scriptCache;
-
- @Reference
- private ResourceResolverFactory resourceResolverFactory;
+ public BundledUnitManagerImpl(
+ @NotNull SightlyScriptEngineFactory sightlyScriptEngineFactory,
+ @NotNull ScriptCache scriptCache,
+ @NotNull ResourceResolverFactory resourceResolverFactory) {
+ this.sightlyScriptEngineFactory = sightlyScriptEngineFactory;
+ this.scriptCache = scriptCache;
+ this.resourceResolverFactory = resourceResolverFactory;
+ }
/**
* Given a {@link Bindings} map, this method will check if the {@code
bindings} contain a value for the {@link
@@ -148,7 +151,7 @@ public class BundledUnitManagerImpl implements
BundledUnitManager {
if (bundledScriptURL != null) {
try {
SightlyScriptEngine sightlyScriptEngine =
- (SightlyScriptEngine)
scriptEngineManager.getEngineByName("htl");
+ (SightlyScriptEngine)
sightlyScriptEngineFactory.getScriptEngine();
if (sightlyScriptEngine != null) {
CachedScript cachedScript =
scriptCache.getScript(bundledScriptURL.toExternalForm());
@@ -256,6 +259,11 @@ public class BundledUnitManagerImpl implements
BundledUnitManager {
return null;
}
+ @NotNull
+ public SightlyScriptEngineFactory getScriptEngineFactory() {
+ return sightlyScriptEngineFactory;
+ }
+
@Nullable
private BundledRenderUnit getBundledRenderUnit(Bindings bindings) {
Object bru = bindings.get(BundledRenderUnit.VARIABLE);
diff --git
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/RenderUnitProvider.java
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/RenderUnitProvider.java
index 8d7b0e1..f2bcb6d 100644
---
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/RenderUnitProvider.java
+++
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/RenderUnitProvider.java
@@ -19,8 +19,6 @@
package org.apache.sling.scripting.sightly.impl.engine.extension.use;
import javax.script.Bindings;
-import javax.script.CompiledScript;
-import javax.script.ScriptEngineManager;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -29,8 +27,6 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.scripting.SlingScriptHelper;
-import org.apache.sling.scripting.api.CachedScript;
-import org.apache.sling.scripting.api.ScriptCache;
import org.apache.sling.scripting.core.ScriptNameAwareReader;
import org.apache.sling.scripting.sightly.SightlyException;
import org.apache.sling.scripting.sightly.engine.ResourceResolution;
@@ -69,15 +65,9 @@ public class RenderUnitProvider implements UseProvider {
int service_ranking() default 100;
}
- @Reference
- private ScriptCache scriptCache;
-
@Reference
private BundledUnitManagerImpl bundledUnitManager;
- @Reference
- private ScriptEngineManager scriptEngineManager;
-
@Reference
private ScriptDependencyResolver scriptDependencyResolver;
@@ -118,37 +108,20 @@ public class RenderUnitProvider implements UseProvider {
return
ProviderOutcome.success(bundledRenderUnit.getUnit());
}
}
- CachedScript cachedScript =
scriptCache.getScript(renderUnitResource.getPath());
- final SightlyCompiledScript compiledScript;
- if (cachedScript != null) {
- compiledScript = (SightlyCompiledScript)
cachedScript.getCompiledScript();
- } else {
- SightlyScriptEngine sightlyScriptEngine =
(SightlyScriptEngine)
-
scriptEngineManager.getEngineByName(SightlyScriptEngineFactory.SHORT_NAME);
- String encoding =
renderUnitResource.getResourceMetadata().getCharacterEncoding();
- if (StringUtils.isEmpty(encoding)) {
- encoding = "UTF-8";
- }
- InputStream inputStream =
renderUnitResource.adaptTo(InputStream.class);
- if (inputStream == null) {
- return ProviderOutcome.failure();
- }
- InputStreamReader inputStreamReader = new
InputStreamReader(inputStream, encoding);
- ScriptNameAwareReader reader =
- new ScriptNameAwareReader(inputStreamReader,
renderUnitResource.getPath());
- compiledScript = (SightlyCompiledScript)
sightlyScriptEngine.compile(reader);
- scriptCache.putScript(new CachedScript() {
- @Override
- public String getScriptPath() {
- return renderUnitResource.getPath();
- }
-
- @Override
- public CompiledScript getCompiledScript() {
- return compiledScript;
- }
- });
+ SightlyScriptEngine sightlyScriptEngine = (SightlyScriptEngine)
+
bundledUnitManager.getScriptEngineFactory().getScriptEngine();
+ String encoding =
renderUnitResource.getResourceMetadata().getCharacterEncoding();
+ if (StringUtils.isEmpty(encoding)) {
+ encoding = "UTF-8";
+ }
+ InputStream inputStream =
renderUnitResource.adaptTo(InputStream.class);
+ if (inputStream == null) {
+ return ProviderOutcome.failure();
}
+ InputStreamReader inputStreamReader = new
InputStreamReader(inputStream, encoding);
+ ScriptNameAwareReader reader =
+ new ScriptNameAwareReader(inputStreamReader,
renderUnitResource.getPath());
+ SightlyCompiledScript compiledScript = (SightlyCompiledScript)
sightlyScriptEngine.compile(reader);
return ProviderOutcome.success(compiledScript.getRenderUnit());
} catch (Exception e) {
return ProviderOutcome.failure(e);
diff --git
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/ScriptUseProvider.java
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/ScriptUseProvider.java
index 3936710..5e63356 100644
---
a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/ScriptUseProvider.java
+++
b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/ScriptUseProvider.java
@@ -24,17 +24,14 @@ import javax.script.CompiledScript;
import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
-import java.io.StringReader;
+import java.io.InputStreamReader;
import java.net.URL;
import java.nio.charset.StandardCharsets;
-import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.scripting.SlingBindings;
import org.apache.sling.api.scripting.SlingScript;
-import org.apache.sling.scripting.api.CachedScript;
-import org.apache.sling.scripting.api.ScriptCache;
import org.apache.sling.scripting.core.ScriptNameAwareReader;
import
org.apache.sling.scripting.sightly.impl.engine.SightlyScriptEngineFactory;
import
org.apache.sling.scripting.sightly.impl.engine.bundled.BundledUnitManagerImpl;
@@ -82,9 +79,6 @@ public class ScriptUseProvider implements UseProvider {
@Reference
private ScriptEngineManager scriptEngineManager;
- @Reference
- private ScriptCache scriptCache;
-
@Reference
protected ScriptDependencyResolver scriptDependencyResolver;
@@ -101,30 +95,20 @@ public class ScriptUseProvider implements UseProvider {
String scriptUrlAsString = script.toExternalForm();
bindings.remove(BundledRenderUnit.VARIABLE);
bindings.put(ScriptEngine.FILENAME, scriptUrlAsString);
- try {
- ScriptEngine scriptEngine =
scriptEngineManager.getEngineByExtension(extension);
- if (scriptEngine != null) {
+ ScriptEngine scriptEngine =
scriptEngineManager.getEngineByExtension(extension);
+ if (scriptEngine != null) {
+ try (ScriptNameAwareReader reader = new ScriptNameAwareReader(
+ new InputStreamReader(script.openStream(),
StandardCharsets.UTF_8), scriptUrlAsString)) {
if (scriptEngine instanceof Compilable) {
- CompiledScript compiledScript;
- CachedScript cachedScript =
scriptCache.getScript(scriptUrlAsString);
- if (cachedScript == null) {
- Compilable compilableScriptEngine = (Compilable)
scriptEngine;
- ScriptNameAwareReader reader = new
ScriptNameAwareReader(
- new StringReader(IOUtils.toString(script,
StandardCharsets.UTF_8)),
- scriptUrlAsString);
- compiledScript =
compilableScriptEngine.compile(reader);
- } else {
- compiledScript = cachedScript.getCompiledScript();
- }
+ Compilable compilableScriptEngine = (Compilable)
scriptEngine;
+ CompiledScript compiledScript =
compilableScriptEngine.compile(reader);
return
ProviderOutcome.notNullOrFailure(compiledScript.eval(bindings));
} else {
- ScriptNameAwareReader reader = new
ScriptNameAwareReader(
- new StringReader(IOUtils.toString(script,
StandardCharsets.UTF_8)), scriptUrlAsString);
return
ProviderOutcome.notNullOrFailure(scriptEngine.eval(reader, bindings));
}
+ } catch (Exception e) {
+ return ProviderOutcome.failure(e);
}
- } catch (Exception e) {
- return ProviderOutcome.failure(e);
}
}
Resource scriptResource =
scriptDependencyResolver.resolveScript(renderContext, scriptName);
diff --git
a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactoryTest.java
b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactoryTest.java
index 3bfc752..c3688ab 100644
---
a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactoryTest.java
+++
b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyScriptEngineFactoryTest.java
@@ -21,7 +21,8 @@ package org.apache.sling.scripting.sightly.impl.engine;
import javax.script.ScriptEngineFactory;
import javax.script.SimpleScriptContext;
-import
org.apache.sling.scripting.sightly.impl.engine.bundled.BundledUnitManagerImpl;
+import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.scripting.api.ScriptCache;
import
org.apache.sling.scripting.sightly.impl.engine.runtime.RenderContextImpl;
import org.apache.sling.scripting.sightly.render.RenderContext;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
@@ -41,10 +42,11 @@ public class SightlyScriptEngineFactoryTest {
@Test
public void testLegacyBooleanCastingFalse() {
ExtensionRegistryService extensionRegistryService =
mock(ExtensionRegistryService.class);
- context.registerService(BundledUnitManagerImpl.class,
mock(BundledUnitManagerImpl.class));
+ context.registerService(ScriptCache.class, mock(ScriptCache.class));
+ context.registerService(ResourceResolverFactory.class,
mock(ResourceResolverFactory.class));
context.registerService(ExtensionRegistryService.class,
extensionRegistryService);
context.registerInjectActivateService(new
SightlyEngineConfiguration(), "legacyBooleanCasting", false);
- context.registerInjectActivateService(new
SightlyScriptEngineFactory());
+
context.registerInjectActivateService(SightlyScriptEngineFactory.class);
SightlyScriptEngineFactory factory = (SightlyScriptEngineFactory)
context.getService(ScriptEngineFactory.class);
assertNotNull(factory);
@@ -56,10 +58,11 @@ public class SightlyScriptEngineFactoryTest {
@Test
public void testLegacyBooleanCasting() {
ExtensionRegistryService extensionRegistryService =
mock(ExtensionRegistryService.class);
- context.registerService(BundledUnitManagerImpl.class,
mock(BundledUnitManagerImpl.class));
+ context.registerService(ScriptCache.class, mock(ScriptCache.class));
context.registerService(ExtensionRegistryService.class,
extensionRegistryService);
+ context.registerService(ResourceResolverFactory.class,
mock(ResourceResolverFactory.class));
context.registerInjectActivateService(new
SightlyEngineConfiguration(), "legacyBooleanCasting", true);
- context.registerInjectActivateService(new
SightlyScriptEngineFactory());
+
context.registerInjectActivateService(SightlyScriptEngineFactory.class);
SightlyScriptEngineFactory factory = (SightlyScriptEngineFactory)
context.getService(ScriptEngineFactory.class);
assertNotNull(factory);
