You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2015/02/12 14:24:36 UTC

svn commit: r1659247 - in /sling/trunk/contrib/scripting/sightly/engine/src: main/java/org/apache/sling/scripting/sightly/impl/compiler/ main/java/org/apache/sling/scripting/sightly/impl/engine/ test/java/org/apache/sling/scripting/sightly/impl/compiler/

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);