This is an automated email from the ASF dual-hosted git repository.
joerghoh 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 1374752 SLING-12473 replace cache by a ConcurrentHashMap (#29)
1374752 is described below
commit 13747528a06df38494a09d362bb4d869d2bb6e1f
Author: Jörg Hoh <[email protected]>
AuthorDate: Wed Nov 20 14:43:27 2024 +0100
SLING-12473 replace cache by a ConcurrentHashMap (#29)
---
.../impl/utils/ScriptDependencyResolver.java | 112 +++++----------------
.../impl/utils/ScriptDependencyResolverTest.java | 63 ++++++------
2 files changed, 58 insertions(+), 117 deletions(-)
diff --git
a/src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java
b/src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java
index ad3f29d..2678cd3 100644
---
a/src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java
+++
b/src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java
@@ -18,12 +18,9 @@ package org.apache.sling.scripting.sightly.impl.utils;
import java.util.Dictionary;
import java.util.Hashtable;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
@@ -75,10 +72,7 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
@Reference
private ResourceResolverFactory resourceResolverFactory;
- private Map<String, String> resolutionCache = new Cache(0);
- private final ReentrantReadWriteLock rwl = new
ReentrantReadWriteLock(true);
- private final Lock readLock = rwl.readLock();
- private final Lock writeLock = rwl.writeLock();
+ private Map<String, String> resolutionCache = new ConcurrentHashMap<>();
private ServiceRegistration<ResourceChangeListener>
resourceChangeListenerServiceRegistration;
@@ -86,17 +80,11 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
private static final String NOT_FOUND_MARKER = "#NOT_FOUND#";
- @SuppressWarnings("squid:S1149")
@Activate
private void activate(ComponentContext componentContext) {
int cacheSize =
sightlyEngineConfiguration.getScriptResolutionCacheSize();
- if (cacheSize < 1024) {
- cacheEnabled = false;
- resolutionCache = new Cache(0);
- } else {
- cacheEnabled = true;
- resolutionCache = new Cache(cacheSize);
- }
+ cacheEnabled = (cacheSize >= 1024);
+
if (cacheEnabled) {
componentContext.getBundleContext().addBundleListener(this);
Dictionary<String, Object> resourceChangeListenerProperties = new
Hashtable<>();
@@ -119,42 +107,32 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
componentContext.getBundleContext().removeBundleListener(this);
}
+ /**
+ * Resolves a script identifier to a resource
+ * @param renderContext the context
+ * @param scriptIdentifier the script identifier
+ * @return the matching resource or null if the looked up resource does
not exist
+ */
public Resource resolveScript(RenderContext renderContext, String
scriptIdentifier) {
SlingHttpServletRequest request =
BindingsUtils.getRequest(renderContext.getBindings());
if (!cacheEnabled) {
return internalResolveScript(request, renderContext,
scriptIdentifier);
}
- readLock.lock();
- try {
- String cacheKey = request.getResource().getResourceType() + ":" +
scriptIdentifier;
- Resource result = null;
- if (!resolutionCache.containsKey(cacheKey)) {
- readLock.unlock();
- writeLock.lock();
- try {
- // recheck state in case another writer thread acquired
the lock before this one
- if (!resolutionCache.containsKey(cacheKey)) {
- result = internalResolveScript(request, renderContext,
scriptIdentifier);
- if (result != null) {
- resolutionCache.put(cacheKey, result.getPath());
- } else {
- resolutionCache.put(cacheKey, NOT_FOUND_MARKER);
- }
+ String cacheKey = request.getResource().getResourceType() + ":" +
scriptIdentifier;
+ String scriptPath = resolutionCache.computeIfAbsent(cacheKey,
+ t -> {
+ Resource r = internalResolveScript(request, renderContext,
scriptIdentifier);
+ if (r == null) {
+ return NOT_FOUND_MARKER;
+ } else {
+ return r.getPath();
}
- readLock.lock();
- } finally {
- writeLock.unlock();
- }
- }
- String scriptPath = resolutionCache.get(cacheKey);
- // in case another thread was downgrading the lock, we need to
recheck the value
- if (result == null && scriptPath != null &&
!scriptPath.equals(NOT_FOUND_MARKER)) {
- result =
scriptingResourceResolverProvider.getRequestScopedResourceResolver().getResource(scriptPath);
- }
- return result;
- } finally {
- readLock.unlock();
+ });
+ if (scriptPath.equals(NOT_FOUND_MARKER)) {
+ return null;
}
+ return
scriptingResourceResolverProvider.getRequestScopedResourceResolver().getResource(scriptPath);
+
}
private @Nullable Resource internalResolveScript(SlingHttpServletRequest
request, RenderContext renderContext,
@@ -176,12 +154,7 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
@Override
public void onChange(@NotNull List<ResourceChange> changes) {
// we won't be specific about the changes; wipe the whole cache
- writeLock.lock();
- try {
- resolutionCache.clear();
- } finally {
- writeLock.unlock();
- }
+ resolutionCache.clear();
}
@Override
@@ -190,42 +163,7 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
Dictionary<String, String> bundleHeaders =
event.getBundle().getHeaders();
String requireCapabilityHeader =
bundleHeaders.get("Require-Capability");
if (StringUtils.isNotEmpty(requireCapabilityHeader) &&
requireCapabilityHeader.contains(BUNDLED_SCRIPTS_REQUIREMENT)) {
- writeLock.lock();
- try {
- resolutionCache.clear();
- } finally {
- writeLock.unlock();
- }
- }
- }
-
- private static class Cache extends LinkedHashMap<String, String> {
-
- private final int cacheSize;
-
- public Cache(int cacheSize) {
- super();
- this.cacheSize = cacheSize;
- }
-
- @Override
- protected boolean removeEldestEntry(Map.Entry<String, String> eldest) {
- return size() > cacheSize;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o instanceof Cache) {
- Cache other = (Cache) o;
- return super.equals(o) && cacheSize == other.cacheSize;
- }
- return false;
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(super.hashCode(), cacheSize);
+ resolutionCache.clear();
}
}
-
}
diff --git
a/src/test/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolverTest.java
b/src/test/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolverTest.java
index 4c4417c..0949839 100644
---
a/src/test/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolverTest.java
+++
b/src/test/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolverTest.java
@@ -42,6 +42,7 @@ import org.junit.Rule;
import org.junit.Test;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
@@ -57,6 +58,9 @@ public class ScriptDependencyResolverTest {
@Rule
public SlingContext context = new SlingContext();
+ private ResourceResolver scriptingResolver;
+ private RenderContext renderContext;
+
@Before
public void before() throws PersistenceException {
// resource hierarchy
@@ -73,15 +77,24 @@ public class ScriptDependencyResolverTest {
testResourceProperties.put("sling:resourceType", "inherit");
ResourceUtil.getOrCreateResource(context.resourceResolver(),
"/content/test", testResourceProperties, "sling:Folder", true);
context.request().setResource(context.resourceResolver().getResource("/content/test"));
+
+ renderContext = mock(RenderContext.class);
+ Bindings bindings = mock(Bindings.class);
+ when(renderContext.getBindings()).thenReturn(bindings);
+
when(bindings.get(SlingBindings.REQUEST)).thenReturn(context.request());
}
- @Test
- public void testDependenciesResolvingCacheDisabled() {
+ // Perform the remaining setup work, which depends on the configuration
+ public void setupDependencies(boolean enableCaching) {
+ int cacheSize=0;
+ if (enableCaching) {
+ cacheSize=1024;
+ }
SightlyEngineConfiguration configuration =
mock(SightlyEngineConfiguration.class);
- when(configuration.getScriptResolutionCacheSize()).thenReturn(0);
+
when(configuration.getScriptResolutionCacheSize()).thenReturn(cacheSize);
context.registerService(configuration);
- ResourceResolver scriptingResolver = spy(context.resourceResolver());
+ scriptingResolver = spy(context.resourceResolver());
ScriptingResourceResolverProvider scriptingResourceResolverProvider =
mock(ScriptingResourceResolverProvider.class);
@@ -89,11 +102,11 @@ public class ScriptDependencyResolverTest {
context.registerService(ScriptingResourceResolverProvider.class,
scriptingResourceResolverProvider);
scriptDependencyResolver = context.registerInjectActivateService(new
ScriptDependencyResolver());
+ }
- RenderContext renderContext = mock(RenderContext.class);
- Bindings bindings = mock(Bindings.class);
- when(renderContext.getBindings()).thenReturn(bindings);
-
when(bindings.get(SlingBindings.REQUEST)).thenReturn(context.request());
+ @Test
+ public void testDependenciesResolvingCacheDisabled() {
+ setupDependencies(false);
Resource partial =
scriptDependencyResolver.resolveScript(renderContext, "partial.html");
assertNotNull(partial);
@@ -104,45 +117,35 @@ public class ScriptDependencyResolverTest {
@Test
public void testDependenciesResolvingCacheEnabled() {
- SightlyEngineConfiguration configuration =
mock(SightlyEngineConfiguration.class);
- when(configuration.getScriptResolutionCacheSize()).thenReturn(1024);
- context.registerService(configuration);
-
- ResourceResolver scriptingResolver = spy(context.resourceResolver());
-
- ScriptingResourceResolverProvider scriptingResourceResolverProvider =
- mock(ScriptingResourceResolverProvider.class);
-
when(scriptingResourceResolverProvider.getRequestScopedResourceResolver()).thenReturn(scriptingResolver);
- context.registerService(ScriptingResourceResolverProvider.class,
scriptingResourceResolverProvider);
-
- scriptDependencyResolver = context.registerInjectActivateService(new
ScriptDependencyResolver());
-
- RenderContext renderContext = mock(RenderContext.class);
- Bindings bindings = mock(Bindings.class);
- when(renderContext.getBindings()).thenReturn(bindings);
-
when(bindings.get(SlingBindings.REQUEST)).thenReturn(context.request());
+ setupDependencies(true);
Resource partial =
scriptDependencyResolver.resolveScript(renderContext, "partial.html");
assertNotNull(partial);
partial = scriptDependencyResolver.resolveScript(renderContext,
"partial.html");
assertNotNull(partial);
- verify(scriptingResolver, times(5)).getResource(anyString());
+ verify(scriptingResolver, times(6)).getResource(anyString());
// simulate a change in the resource tree
List<ResourceChange> changes = new ArrayList<>();
changes.add(mock(ResourceChange.class));
scriptDependencyResolver.onChange(changes);
-
- // cache should be empty; another retrieval should increase the
invocations by 4
+ // cache should be empty; another retrieval should increase the
invocations by 5
partial = scriptDependencyResolver.resolveScript(renderContext,
"partial.html");
assertNotNull(partial);
- verify(scriptingResolver, times(9)).getResource(anyString());
+ verify(scriptingResolver, times(11)).getResource(anyString());
// cache repopulated; another retrieval should increase the
invocations only by 1
partial = scriptDependencyResolver.resolveScript(renderContext,
"partial.html");
assertNotNull(partial);
- verify(scriptingResolver, times(10)).getResource(anyString());
+ verify(scriptingResolver, times(12)).getResource(anyString());
+ }
+
+ @Test
+ public void noExistingResourceReturnsNull() {
+ setupDependencies(true);
+ Resource nonexisting =
scriptDependencyResolver.resolveScript(renderContext, "nonexisting.html");
+ assertNull(nonexisting);
}
}
