This is an automated email from the ASF dual-hosted git repository. cziegeler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git
The following commit(s) were added to refs/heads/master by this push: new 6c1ca32 SLING-12194 : Cycle between scripting services 6c1ca32 is described below commit 6c1ca32ea83939a45ebb5f1ac2ac20062cd9ad90 Author: Carsten Ziegeler <cziege...@apache.org> AuthorDate: Sun Dec 10 12:39:33 2023 +0100 SLING-12194 : Cycle between scripting services --- pom.xml | 3 +- .../sling/scripting/core/impl/ScriptCacheImpl.java | 14 +++-- .../core/impl/ScriptCacheInvalidator.java | 15 +++--- .../scripting/core/impl/ScriptCacheImplTest.java | 62 ++++++++++++++++++++++ 4 files changed, 75 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index c46a0f0..7949fe1 100644 --- a/pom.xml +++ b/pom.xml @@ -44,8 +44,7 @@ <connection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git</connection> <developerConnection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git</developerConnection> <url>https://github.com/apache/sling-org-apache-sling-scripting-core.git</url> - <tag>org.apache.sling.scripting.core-2.4.8</tag> - </scm> + </scm> <build> <plugins> diff --git a/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheImpl.java b/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheImpl.java index 0ef6431..e6db674 100644 --- a/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheImpl.java +++ b/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheImpl.java @@ -101,12 +101,15 @@ public class ScriptCacheImpl implements ScriptCache { public boolean removeScript(final String scriptPath) { writeLock.lock(); try { - boolean result = false; - if (scriptPath.endsWith("/") ) { + boolean result = internalMap.remove(scriptPath) != null; + if (result) { + logger.debug("Removed script {} from script cache.", scriptPath); + } else { // prefix removal + final String prefix = scriptPath.concat("/"); final Set<String> removal = new HashSet<>(); for(final Map.Entry<String, SoftReference<CachedScript>> entry : internalMap.entrySet()) { - if ( entry.getKey().startsWith(scriptPath) ) { + if ( entry.getKey().startsWith(prefix) ) { removal.add(entry.getKey()); } } @@ -115,11 +118,6 @@ public class ScriptCacheImpl implements ScriptCache { logger.debug("Detected removal for {} - removed entry {} from the cache.", scriptPath, key); result = true; } - } else { - result = internalMap.remove(scriptPath) != null; - if (result) { - logger.debug("Removed script {} from script cache.", scriptPath); - } } return result; } finally { diff --git a/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheInvalidator.java b/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheInvalidator.java index c5ec200..8fa315c 100644 --- a/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheInvalidator.java +++ b/src/main/java/org/apache/sling/scripting/core/impl/ScriptCacheInvalidator.java @@ -103,15 +103,12 @@ public class ScriptCacheInvalidator implements ResourceChangeListener, ExternalR @Override public void onChange(@NotNull List<ResourceChange> list) { - for (final ResourceChange change : list) { - Runnable eventTask = () -> { - final String path = change.getPath(); - if (!this.scriptCache.removeScript(path)) { - this.scriptCache.removeScript(path.concat("/")); - } - }; - threadPool.execute(eventTask); - } + final Runnable eventTask = () -> { + for (final ResourceChange change : list) { + this.scriptCache.removeScript( change.getPath()); + } + }; + threadPool.execute(eventTask); } private void configureListener() { diff --git a/src/test/java/org/apache/sling/scripting/core/impl/ScriptCacheImplTest.java b/src/test/java/org/apache/sling/scripting/core/impl/ScriptCacheImplTest.java new file mode 100644 index 0000000..5c32e6b --- /dev/null +++ b/src/test/java/org/apache/sling/scripting/core/impl/ScriptCacheImplTest.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.scripting.core.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.List; + +import org.apache.sling.scripting.api.CachedScript; +import org.junit.Test; +import org.mockito.Mockito; + +public class ScriptCacheImplTest { + + @Test public void testRemoval() { + final ScriptCacheImplConfiguration config = Mockito.mock(ScriptCacheImplConfiguration.class); + Mockito.when(config.org_apache_sling_scripting_cache_size()).thenReturn(10); + final ScriptCacheImpl cache = new ScriptCacheImpl(config); + + final CachedScript script1 = Mockito.mock(CachedScript.class); + Mockito.when(script1.getScriptPath()).thenReturn("/foo/bar/script1"); + cache.putScript(script1); + + final CachedScript script2 = Mockito.mock(CachedScript.class); + Mockito.when(script2.getScriptPath()).thenReturn("/foo/bar/script2"); + cache.putScript(script2); + + final CachedScript script3 = Mockito.mock(CachedScript.class); + Mockito.when(script3.getScriptPath()).thenReturn("/foobar"); + cache.putScript(script3); + + List<String> scripts = cache.getCachedScripts(); + assertEquals(3, scripts.size()); + assertTrue(scripts.contains(script1.getScriptPath())); + assertTrue(scripts.contains(script2.getScriptPath())); + assertTrue(scripts.contains(script3.getScriptPath())); + + cache.removeScript("/foo"); + scripts = cache.getCachedScripts(); + assertEquals(1, scripts.size()); + assertTrue(scripts.contains(script3.getScriptPath())); + + cache.removeScript("/foobar"); + scripts = cache.getCachedScripts(); + assertTrue(scripts.isEmpty()); + } +}