diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java index fbe2196aab..93269f25f4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java @@ -8,7 +8,15 @@ import java.io.File; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.file.FileVisitOption; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -35,7 +43,8 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { protected final ConcurrentMap fileResultsCache; protected final ConcurrentMap updatedResultsCache; protected long rulesetChecksum; - protected long classpathChecksum; + protected long auxClassPathChecksum; + protected long executionClassPathChecksum; protected final CachedRuleMapper ruleMapper = new CachedRuleMapper(); /** @@ -65,7 +74,7 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { LOG.fine("Incremental Analysis cache HIT"); } else { LOG.fine("Incremental Analysis cache MISS - " - + (analysisResult != null ? "file changed" : "no previous results found")); + + (analysisResult != null ? "file changed" : "no previous result found")); } } @@ -90,7 +99,7 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { } @Override - public void checkValidity(final RuleSets ruleSets, final ClassLoader classLoader) { + public void checkValidity(final RuleSets ruleSets, final ClassLoader auxclassPathClassLoader) { boolean cacheIsValid = true; if (ruleSets.getChecksum() != rulesetChecksum) { @@ -98,23 +107,29 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { cacheIsValid = false; } - final long classLoaderChecksum; - if (classLoader instanceof URLClassLoader) { - final URLClassLoader urlClassLoader = (URLClassLoader) classLoader; - classLoaderChecksum = computeClassLoaderHash(urlClassLoader); + final long currentAuxClassPathChecksum; + if (auxclassPathClassLoader instanceof URLClassLoader) { + final URLClassLoader urlClassLoader = (URLClassLoader) auxclassPathClassLoader; + currentAuxClassPathChecksum = computeClassPathHash(urlClassLoader.getURLs()); - if (cacheIsValid && classLoaderChecksum != classpathChecksum) { + if (cacheIsValid && currentAuxClassPathChecksum != auxClassPathChecksum) { // Do we even care? for (final Rule r : ruleSets.getAllRules()) { if (r.usesDFA() || r.usesTypeResolution()) { - LOG.info("Analysis cache invalidated, classpath changed."); + LOG.info("Analysis cache invalidated, auxclasspath changed."); cacheIsValid = false; break; } } } } else { - classLoaderChecksum = 0; + currentAuxClassPathChecksum = 0; + } + + final long currentExecutionClassPathChecksum = computeClassPathHash(getClassPathEntries()); + if (currentExecutionClassPathChecksum != executionClassPathChecksum) { + LOG.info("Analysis cache invalidated, execution classpath changed."); + cacheIsValid = false; } if (!cacheIsValid) { @@ -124,13 +139,46 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { // Update the local checksums rulesetChecksum = ruleSets.getChecksum(); - classpathChecksum = classLoaderChecksum; + auxClassPathChecksum = currentAuxClassPathChecksum; + executionClassPathChecksum = currentExecutionClassPathChecksum; ruleMapper.initialize(ruleSets); } - private long computeClassLoaderHash(final URLClassLoader classLoader) { + private URL[] getClassPathEntries() { + final String classpath = System.getProperty("java.class.path"); + final String[] classpathEntries = classpath.split(File.pathSeparator); + final List entries = new ArrayList<>(); + + try { + for (final String entry : classpathEntries) { + final File f = new File(entry); + if (f.isFile()) { + entries.add(f.toURI().toURL()); + } else { + Files.walkFileTree(f.toPath(), EnumSet.of(FileVisitOption.FOLLOW_LINKS), Integer.MAX_VALUE, + new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(final Path file, + final BasicFileAttributes attrs) throws IOException { + if (!attrs.isSymbolicLink()) { // Broken link that can't be followed + entries.add(file.toUri().toURL()); + } + return FileVisitResult.CONTINUE; + } + }); + } + } + } catch (final IOException e) { + LOG.log(Level.SEVERE, "Incremental analysis can't check execution classpath contents", e); + throw new RuntimeException(e); + } + + return entries.toArray(new URL[0]); + } + + private long computeClassPathHash(final URL... classpathEntry) { final Adler32 adler32 = new Adler32(); - for (final URL url : classLoader.getURLs()) { + for (final URL url : classpathEntry) { try (CheckedInputStream inputStream = new CheckedInputStream(url.openStream(), adler32)) { // Just read it, the CheckedInputStream will update the checksum on it's own while (IOUtils.skip(inputStream, Long.MAX_VALUE) == Long.MAX_VALUE) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java index c2632d74c2..c7afb466f1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java @@ -44,7 +44,7 @@ public interface AnalysisCache extends ThreadSafeReportListener { /** * Checks if the cache is valid for the configured rulesets and class loader. * @param ruleSets The rulesets configured for this analysis. - * @param classLoader The class loader configured for this analysis. + * @param auxclassPathClassLoader The class loader for auxclasspath configured for this analysis. */ - void checkValidity(RuleSets ruleSets, ClassLoader classLoader); + void checkValidity(RuleSets ruleSets, ClassLoader auxclassPathClassLoader); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java index 2cf77121c6..93728fdfe9 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java @@ -55,7 +55,8 @@ public class FileAnalysisCache extends AbstractAnalysisCache { // Get checksums rulesetChecksum = inputStream.readLong(); - classpathChecksum = inputStream.readLong(); + auxClassPathChecksum = inputStream.readLong(); + executionClassPathChecksum = inputStream.readLong(); // Cached results while (inputStream.available() > 0) { @@ -95,12 +96,13 @@ public class FileAnalysisCache extends AbstractAnalysisCache { try ( DataOutputStream outputStream = new DataOutputStream( - new BufferedOutputStream(new FileOutputStream(cacheFile))); + new BufferedOutputStream(new FileOutputStream(cacheFile))) ) { outputStream.writeUTF(pmdVersion); outputStream.writeLong(rulesetChecksum); - outputStream.writeLong(classpathChecksum); + outputStream.writeLong(auxClassPathChecksum); + outputStream.writeLong(executionClassPathChecksum); for (final Map.Entry resultEntry : updatedResultsCache.entrySet()) { final List violations = resultEntry.getValue().getViolations(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java index 8c21fd2106..f67c0a9f6a 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java @@ -24,6 +24,7 @@ import java.util.List; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; import org.junit.rules.TemporaryFolder; import org.mockito.Mockito; @@ -35,6 +36,9 @@ public class FileAnalysisCacheTest { @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + private File unexistingCacheFile; private File newCacheFile; private File emptyCacheFile; @@ -135,7 +139,7 @@ public class FileAnalysisCacheTest { } @Test - public void testClasspathChangeWithoutDFAorTypeResolutionDoesNotInvalidatesCache() throws MalformedURLException, IOException { + public void testAuxClasspathChangeWithoutDFAorTypeResolutionDoesNotInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); when(cl.getURLs()).thenReturn(new URL[] { }); @@ -145,12 +149,12 @@ public class FileAnalysisCacheTest { final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); when(cl.getURLs()).thenReturn(new URL[] { tempFolder.newFile().toURI().toURL(), }); reloadedCache.checkValidity(rs, cl); - assertTrue("Cache believes unmodified file is not up to date after classpath changed when no rule cares", + assertTrue("Cache believes unmodified file is not up to date after auxclasspath changed when no rule cares", reloadedCache.isUpToDate(sourceFile)); } @Test - public void testClasspathChangeInvalidatesCache() throws MalformedURLException, IOException { + public void testAuxClasspathChangeInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); when(cl.getURLs()).thenReturn(new URL[] { }); @@ -161,19 +165,19 @@ public class FileAnalysisCacheTest { final File classpathFile = tempFolder.newFile(); when(cl.getURLs()).thenReturn(new URL[] { classpathFile.toURI().toURL(), }); - // Make sure the classpath file is not empty + // Make sure the auxclasspath file is not empty Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); when(r.usesDFA()).thenReturn(true); when(rs.getAllRules()).thenReturn(Collections.singleton(r)); reloadedCache.checkValidity(rs, cl); - assertFalse("Cache believes unmodified file is up to date after classpath changed", + assertFalse("Cache believes unmodified file is up to date after auxclasspath changed", reloadedCache.isUpToDate(sourceFile)); } @Test - public void testClasspathJarContentsChangeInvalidatesCache() throws MalformedURLException, IOException { + public void testAuxClasspathJarContentsChangeInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); @@ -186,15 +190,56 @@ public class FileAnalysisCacheTest { setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); + // Edit the auxclasspath referenced file + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + + final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); + reloadedCache.checkValidity(rs, cl); + assertFalse("Cache believes cache is up to date when a auxclasspath file changed", + reloadedCache.isUpToDate(sourceFile)); + } + + @Test + public void testClasspathChangeInvalidatesCache() throws MalformedURLException, IOException { + final RuleSets rs = mock(RuleSets.class); + final ClassLoader cl = mock(ClassLoader.class); + + final File classpathFile = tempFolder.newFile(); + + setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); + // Edit the classpath referenced file Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + System.setProperty("java.class.path", System.getProperty("java.class.path") + File.pathSeparator + classpathFile.getAbsolutePath()); + + final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); + reloadedCache.checkValidity(rs, cl); + assertFalse("Cache believes cache is up to date when the classpath changed", + reloadedCache.isUpToDate(sourceFile)); + } + + @Test + public void testClasspathContentsChangeInvalidatesCache() throws MalformedURLException, IOException { + final RuleSets rs = mock(RuleSets.class); + final ClassLoader cl = mock(ClassLoader.class); + + final File classpathFile = tempFolder.newFile(); + + // Add a file to classpath + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + System.setProperty("java.class.path", System.getProperty("java.class.path") + File.pathSeparator + classpathFile.getAbsolutePath()); + + setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); + + // Change the file's contents + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some other text".getBytes()); final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); reloadedCache.checkValidity(rs, cl); assertFalse("Cache believes cache is up to date when a classpath file changed", reloadedCache.isUpToDate(sourceFile)); } - + @Test public void testUnknownFileIsNotUpToDate() throws IOException { final FileAnalysisCache cache = new FileAnalysisCache(newCacheFile);