Author: radu Date: Thu Feb 12 13:24:36 2015 New Revision: 1659247 URL: http://svn.apache.org/r1659247 Log: SLING-4406 - POJOs stored in the repository might not get recompiled when changed
* expanded POJO search to all possible paths from the repository such that the UnitChangeMonitor is queried with the correct path of the POJO if the POJO is stored in the repository * protected the UnitChangeMonitor against NPE if the queried paths are null Modified: sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitChangeMonitor.java sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java sling/trunk/contrib/scripting/sightly/engine/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerServiceTest.java Modified: sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java?rev=1659247&r1=1659246&r2=1659247&view=diff ============================================================================== --- sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java (original) +++ sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java Thu Feb 12 13:24:36 2015 @@ -46,6 +46,7 @@ import org.apache.sling.jcr.compiler.Jcr import org.apache.sling.scripting.sightly.ResourceResolution; import org.apache.sling.scripting.sightly.SightlyException; import org.apache.sling.scripting.sightly.impl.engine.UnitChangeMonitor; +import org.apache.sling.scripting.sightly.impl.engine.UnitLoader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,7 +92,7 @@ public class SightlyJavaCompilerService */ public Object getInstance(ResourceResolver resolver, Resource callingScript, String className) { if (className.contains(".")) { - String pojoPath = getPathFromJavaName(className); + String pojoPath = getPathFromJavaName(resolver, className); if (unitChangeMonitor.getLastModifiedDateForJavaUseObject(pojoPath) > 0) { // it looks like the POJO comes from the repo and it was changed since it was last loaded Resource pojoResource = resolver.getResource(pojoPath); @@ -108,12 +109,9 @@ public class SightlyJavaCompilerService return loadObject(className); } catch (CompilerException cex) { // the object definitely doesn't come from a bundle so we should attempt to compile it from the repo - Set<String> possiblePaths = getPossiblePojoPaths(pojoPath); - for (String possiblePath : possiblePaths) { - Resource pojoResource = resolver.getResource(possiblePath); - if (pojoResource != null) { - return compileSource(pojoResource, className); - } + Resource pojoResource = resolver.getResource(pojoPath); + if (pojoResource != null) { + return compileSource(pojoResource, className); } } } @@ -284,10 +282,25 @@ public class SightlyJavaCompilerService return path.substring(1).replace("/", ".").replace("-", "_"); } - private String getPathFromJavaName(String className) { + private String getPathFromJavaName(ResourceResolver resolver, String className) { + boolean sightlyGeneratedClass = false; + if (className.contains("." + UnitLoader.CLASS_NAME_PREFIX)) { + sightlyGeneratedClass = true; + } String packageName = className.substring(0, className.lastIndexOf('.')); String shortClassName = className.substring(packageName.length() + 1); - return "/" + packageName.replace(".", "/").replace("_", "-") + "/" + shortClassName + ".java"; + String path = "/" + packageName.replace(".", "/").replace("_", "-") + "/" + shortClassName + ".java"; + if (sightlyGeneratedClass) { + return path; + } else { + Set<String> possiblePaths = getPossiblePojoPaths(path); + for (String possiblePath : possiblePaths) { + if (resolver.getResource(possiblePath) != null) { + return possiblePath; + } + } + return path; + } } private String createErrorMsg(List<CompilerMessage> errors) { Modified: sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitChangeMonitor.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitChangeMonitor.java?rev=1659247&r1=1659246&r2=1659247&view=diff ============================================================================== --- sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitChangeMonitor.java (original) +++ sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitChangeMonitor.java Thu Feb 12 13:24:36 2015 @@ -66,6 +66,9 @@ public class UnitChangeMonitor { * @return the script's last modified date or 0 if there's no information about the script */ public long getLastModifiedDateForScript(String script) { + if (script == null) { + return 0; + } Long date = slyScriptsMap.get(script); return date != null ? date : 0; } @@ -77,6 +80,9 @@ public class UnitChangeMonitor { * @return the file's last modified date or 0 if there's no information about the file */ public long getLastModifiedDateForJavaSourceFile(String file) { + if (file == null) { + return 0; + } Long date = slySourcesMap.get(file); return date != null ? date : 0; } @@ -88,6 +94,9 @@ public class UnitChangeMonitor { * @return the Java Use-API file's last modified date or 0 if there's no information about this file */ public long getLastModifiedDateForJavaUseObject(String path) { + if (path == null) { + return 0; + } Long date = slyJavaUseMap.get(path); return date != null ? date : 0; } @@ -98,7 +107,9 @@ public class UnitChangeMonitor { } public void clearJavaUseObject(String path) { - slyJavaUseMap.remove(path); + if (path != null) { + slyJavaUseMap.remove(path); + } } @Activate Modified: sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java?rev=1659247&r1=1659246&r2=1659247&view=diff ============================================================================== --- sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java (original) +++ sling/trunk/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java Thu Feb 12 13:24:36 2015 @@ -69,8 +69,8 @@ import org.slf4j.LoggerFactory; public class UnitLoader { public static final String DEFAULT_REPO_BASE_PATH = "/var/classes"; + public static final String CLASS_NAME_PREFIX = "SightlyJava_"; private static final Logger log = LoggerFactory.getLogger(UnitLoader.class); - private static final String CLASS_NAME_PREFIX = "SightlyJava_"; private static final String MAIN_TEMPLATE_PATH = "templates/compiled_unit_template.txt"; private static final String CHILD_TEMPLATE_PATH = "templates/subtemplate.txt"; Modified: sling/trunk/contrib/scripting/sightly/engine/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerServiceTest.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/scripting/sightly/engine/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerServiceTest.java?rev=1659247&r1=1659246&r2=1659247&view=diff ============================================================================== --- sling/trunk/contrib/scripting/sightly/engine/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerServiceTest.java (original) +++ sling/trunk/contrib/scripting/sightly/engine/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerServiceTest.java Thu Feb 12 13:24:36 2015 @@ -19,6 +19,8 @@ package org.apache.sling.scripting.sightly.impl.compiler; import java.util.ArrayList; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; @@ -36,22 +38,26 @@ import org.mockito.stubbing.Answer; import org.powermock.reflect.Whitebox; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class SightlyJavaCompilerServiceTest { private SightlyJavaCompilerService compiler; + private UnitChangeMonitor ucm; @Before public void setUp() throws Exception { compiler = new SightlyJavaCompilerService(); - UnitChangeMonitor ucm = new UnitChangeMonitor(); + ucm = spy(new UnitChangeMonitor()); Whitebox.setInternalState(compiler, "unitChangeMonitor", ucm); } @After public void tearDown() throws Exception { compiler = null; + ucm = null; } @Test @@ -75,6 +81,18 @@ public class SightlyJavaCompilerServiceT getInstancePojoTest(pojoPath, className); } + @Test + public void testGetInstanceForCachedPojoFromRepo() throws Exception { + final String pojoPath = "/apps/my-project/test_components/a/Pojo.java"; + String className = "apps.my_project.test_components.a.Pojo"; + Map<String, Long> slyJavaUseMap = new ConcurrentHashMap<String, Long>() {{ + put(pojoPath, System.currentTimeMillis()); + }}; + Whitebox.setInternalState(ucm, "slyJavaUseMap", slyJavaUseMap); + getInstancePojoTest(pojoPath, className); + verify(ucm).clearJavaUseObject(pojoPath); + } + private void getInstancePojoTest(String pojoPath, String className) throws Exception { Resource pojoResource = Mockito.mock(Resource.class); ResourceResolver resolver = Mockito.mock(ResourceResolver.class);