From 943e74fd412f701247e1bea5aa3760418a10ec0d Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Wed, 30 Sep 2015 15:16:56 +0200 Subject: [PATCH] Decouple cache invalidation criteria --- .../batch/cache/WSLoaderTestWithServer.java | 5 +- .../org/sonar/home/cache/PersistentCache.java | 61 ++++---------- .../home/cache/PersistentCacheBuilder.java | 11 +-- .../cache/PersistentCacheInvalidation.java | 27 ++++++ .../home/cache/TTLCacheInvalidation.java | 48 +++++++++++ .../home/cache/UnlockOnCloseInputStream.java | 82 ------------------- .../sonar/home/cache/PersistentCacheTest.java | 17 ++-- .../home/cache/TTLCacheInvalidationTest.java | 55 +++++++++++++ 8 files changed, 165 insertions(+), 141 deletions(-) create mode 100644 sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheInvalidation.java create mode 100644 sonar-home/src/main/java/org/sonar/home/cache/TTLCacheInvalidation.java delete mode 100644 sonar-home/src/main/java/org/sonar/home/cache/UnlockOnCloseInputStream.java create mode 100644 sonar-home/src/test/java/org/sonar/home/cache/TTLCacheInvalidationTest.java diff --git a/sonar-batch/src/test/java/org/sonar/batch/cache/WSLoaderTestWithServer.java b/sonar-batch/src/test/java/org/sonar/batch/cache/WSLoaderTestWithServer.java index 1781a143f895..def5566c2743 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/cache/WSLoaderTestWithServer.java +++ b/sonar-batch/src/test/java/org/sonar/batch/cache/WSLoaderTestWithServer.java @@ -21,11 +21,12 @@ import static org.mockito.Mockito.mock; +import org.sonar.home.cache.TTLCacheInvalidation; + import org.sonar.batch.bootstrap.GlobalProperties; import org.sonar.batch.bootstrap.MockHttpServer; import org.sonar.batch.bootstrap.ServerClient; import org.sonar.batch.bootstrap.Slf4jLogger; - import org.sonar.batch.cache.WSLoader; import org.sonar.batch.cache.WSLoader.LoadStrategy; import org.junit.Rule; @@ -57,7 +58,7 @@ public void setUp() throws Exception { when(bootstrapProps.property("sonar.host.url")).thenReturn("http://localhost:" + server.getPort()); client = new ServerClient(bootstrapProps, new EnvironmentInformation("Junit", "4")); - cache = new PersistentCache(temp.getRoot().toPath(), 1000 * 60, new Slf4jLogger(), null); + cache = new PersistentCache(temp.getRoot().toPath(), new TTLCacheInvalidation(100_000L), new Slf4jLogger(), null); } @After diff --git a/sonar-home/src/main/java/org/sonar/home/cache/PersistentCache.java b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCache.java index 991fadf8cdc7..210bb20e4bda 100644 --- a/sonar-home/src/main/java/org/sonar/home/cache/PersistentCache.java +++ b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCache.java @@ -28,7 +28,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.nio.file.attribute.BasicFileAttributes; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -44,20 +43,19 @@ public class PersistentCache { private static final Charset ENCODING = StandardCharsets.UTF_8; private static final String DIGEST_ALGO = "MD5"; - // eviction strategy is to expire entries after modification once a time duration has elapsed - private final long defaultDurationToExpireMs; + private final PersistentCacheInvalidation invalidation; private final Logger logger; private final Path dir; private DirectoryLock lock; - public PersistentCache(Path dir, long defaultDurationToExpireMs, Logger logger, DirectoryLock lock) { + public PersistentCache(Path dir, PersistentCacheInvalidation invalidation, Logger logger, DirectoryLock lock) { this.dir = dir; - this.defaultDurationToExpireMs = defaultDurationToExpireMs; + this.invalidation = invalidation; this.logger = logger; this.lock = lock; reconfigure(); - logger.debug("cache: " + dir + ", default expiration time (ms): " + defaultDurationToExpireMs); + logger.debug("cache: " + dir); } public synchronized void reconfigure() { @@ -149,7 +147,7 @@ public synchronized void clear() { logger.info("cache: clearing"); try { lock(); - deleteCacheEntries(new DirectoryClearFilter(lock.getFileLockName())); + deleteCacheEntries(new DirectoryClearFilter()); } catch (IOException e) { logger.error("Error clearing cache", e); } finally { @@ -164,7 +162,7 @@ public synchronized void clean() { logger.info("cache: cleaning"); try { lock(); - deleteCacheEntries(new DirectoryCleanFilter(defaultDurationToExpireMs, lock.getFileLockName())); + deleteCacheEntries(new DirectoryCleanFilter()); } catch (IOException e) { logger.error("Error cleaning cache", e); } finally { @@ -203,35 +201,21 @@ private void deleteCacheEntries(DirectoryStream.Filter filter) throws IOEx } } - private static class DirectoryClearFilter implements DirectoryStream.Filter { - private String lockFileName; - - DirectoryClearFilter(String lockFileName) { - this.lockFileName = lockFileName; - } - + private class DirectoryClearFilter implements DirectoryStream.Filter { @Override public boolean accept(Path entry) throws IOException { - return !lockFileName.equals(entry.getFileName().toString()); + return !lock.getFileLockName().equals(entry.getFileName().toString()); } } - private static class DirectoryCleanFilter implements DirectoryStream.Filter { - private long defaultDurationToExpireMs; - private String lockFileName; - - DirectoryCleanFilter(long defaultDurationToExpireMs, String lockFileName) { - this.defaultDurationToExpireMs = defaultDurationToExpireMs; - this.lockFileName = lockFileName; - } - + private class DirectoryCleanFilter implements DirectoryStream.Filter { @Override public boolean accept(Path entry) throws IOException { - if (lockFileName.equals(entry.getFileName().toString())) { + if (lock.getFileLockName().equals(entry.getFileName().toString())) { return false; } - return isCacheEntryExpired(entry, defaultDurationToExpireMs); + return invalidation.test(entry); } } @@ -248,7 +232,7 @@ private void putCache(String key, InputStream stream) throws IOException { private byte[] getCache(String key) throws IOException { Path cachePath = getCacheEntryPath(key); - if (!validateCacheEntry(cachePath, this.defaultDurationToExpireMs)) { + if (!validateCacheEntry(cachePath)) { return null; } @@ -258,7 +242,7 @@ private byte[] getCache(String key) throws IOException { private Path getCacheCopy(String key) throws IOException { Path cachePath = getCacheEntryPath(key); - if (!validateCacheEntry(cachePath, this.defaultDurationToExpireMs)) { + if (!validateCacheEntry(cachePath)) { return null; } @@ -267,13 +251,13 @@ private Path getCacheCopy(String key) throws IOException { return temp; } - private boolean validateCacheEntry(Path cacheEntryPath, long durationToExpireMs) throws IOException { + private boolean validateCacheEntry(Path cacheEntryPath) throws IOException { if (!Files.exists(cacheEntryPath)) { return false; } - if (isCacheEntryExpired(cacheEntryPath, durationToExpireMs)) { - logger.debug("cache: expiring entry"); + if (invalidation.test(cacheEntryPath)) { + logger.debug("cache: evicting entry"); Files.delete(cacheEntryPath); return false; } @@ -281,19 +265,6 @@ private boolean validateCacheEntry(Path cacheEntryPath, long durationToExpireMs) return true; } - private static boolean isCacheEntryExpired(Path cacheEntryPath, long durationToExpireMs) throws IOException { - BasicFileAttributes attr = Files.readAttributes(cacheEntryPath, BasicFileAttributes.class); - long modTime = attr.lastModifiedTime().toMillis(); - - long age = System.currentTimeMillis() - modTime; - - if (age > durationToExpireMs) { - return true; - } - - return false; - } - private Path getCacheEntryPath(String key) { return dir.resolve(key); } diff --git a/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheBuilder.java b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheBuilder.java index 51a05b7bf9cc..152f17fc4020 100644 --- a/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheBuilder.java +++ b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheBuilder.java @@ -55,20 +55,20 @@ public PersistentCacheBuilder setAreaForProject(String serverUrl, String serverV .resolve(sanitizeFilename(projectKey)); return this; } - + public PersistentCacheBuilder setAreaForGlobal(String serverUrl) { relativePath = Paths.get(sanitizeFilename(serverUrl)) .resolve("global"); return this; } - + public PersistentCacheBuilder setAreaForLocalProject(String serverUrl, String serverVersion) { relativePath = Paths.get(sanitizeFilename(serverUrl)) .resolve(sanitizeFilename(serverVersion)) .resolve("local"); return this; } - + public PersistentCacheBuilder setSonarHome(@Nullable Path p) { if (p != null) { this.cacheBasePath = p.resolve(DIR_NAME); @@ -77,7 +77,7 @@ public PersistentCacheBuilder setSonarHome(@Nullable Path p) { } public PersistentCache build() { - if(relativePath == null) { + if (relativePath == null) { throw new IllegalStateException("area must be set before building"); } if (cacheBasePath == null) { @@ -85,7 +85,8 @@ public PersistentCache build() { } Path cachePath = cacheBasePath.resolve(relativePath); DirectoryLock lock = new DirectoryLock(cacheBasePath, logger); - return new PersistentCache(cachePath, DEFAULT_EXPIRE_DURATION, logger, lock); + PersistentCacheInvalidation criteria = new TTLCacheInvalidation(DEFAULT_EXPIRE_DURATION); + return new PersistentCache(cachePath, criteria, logger, lock); } private static Path findHome() { diff --git a/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheInvalidation.java b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheInvalidation.java new file mode 100644 index 000000000000..b2ca92a6d44e --- /dev/null +++ b/sonar-home/src/main/java/org/sonar/home/cache/PersistentCacheInvalidation.java @@ -0,0 +1,27 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.home.cache; + +import java.io.IOException; +import java.nio.file.Path; + +public interface PersistentCacheInvalidation { + boolean test(Path cacheEntryPath) throws IOException; +} diff --git a/sonar-home/src/main/java/org/sonar/home/cache/TTLCacheInvalidation.java b/sonar-home/src/main/java/org/sonar/home/cache/TTLCacheInvalidation.java new file mode 100644 index 000000000000..bc2ef2d4551c --- /dev/null +++ b/sonar-home/src/main/java/org/sonar/home/cache/TTLCacheInvalidation.java @@ -0,0 +1,48 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.home.cache; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; + +public class TTLCacheInvalidation implements PersistentCacheInvalidation { + private final long durationToExpireMs; + + public TTLCacheInvalidation(long durationToExpireMs) { + this.durationToExpireMs = durationToExpireMs; + } + + @Override + public boolean test(Path cacheEntryPath) throws IOException { + BasicFileAttributes attr = Files.readAttributes(cacheEntryPath, BasicFileAttributes.class); + long modTime = attr.lastModifiedTime().toMillis(); + + long age = System.currentTimeMillis() - modTime; + + if (age > durationToExpireMs) { + return true; + } + + return false; + } + +} diff --git a/sonar-home/src/main/java/org/sonar/home/cache/UnlockOnCloseInputStream.java b/sonar-home/src/main/java/org/sonar/home/cache/UnlockOnCloseInputStream.java deleted file mode 100644 index fdfac82cff10..000000000000 --- a/sonar-home/src/main/java/org/sonar/home/cache/UnlockOnCloseInputStream.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.home.cache; - -import java.io.IOException; -import java.io.InputStream; - -public class UnlockOnCloseInputStream extends InputStream { - private final DirectoryLock lock; - private final InputStream is; - - public UnlockOnCloseInputStream(InputStream stream, DirectoryLock lock) { - this.is = stream; - this.lock = lock; - } - - @Override - public int read() throws IOException { - return is.read(); - } - - @Override - public int read(byte[] b) throws IOException { - return is.read(b); - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - return is.read(b, off, len); - } - - @Override - public long skip(long n) throws IOException { - return is.skip(n); - } - - @Override - public synchronized void mark(int readlimit) { - is.mark(readlimit); - } - - @Override - public synchronized void reset() throws IOException { - is.reset(); - } - - @Override - public int available() throws IOException { - return is.available(); - } - - @Override - public boolean markSupported() { - return is.markSupported(); - } - - @Override - public void close() throws IOException { - try { - super.close(); - } finally { - lock.unlock(); - } - } -} diff --git a/sonar-home/src/test/java/org/sonar/home/cache/PersistentCacheTest.java b/sonar-home/src/test/java/org/sonar/home/cache/PersistentCacheTest.java index 5063fbdb22f3..5bb52298bb68 100644 --- a/sonar-home/src/test/java/org/sonar/home/cache/PersistentCacheTest.java +++ b/sonar-home/src/test/java/org/sonar/home/cache/PersistentCacheTest.java @@ -28,6 +28,8 @@ import java.nio.file.Files; import java.nio.file.Path; +import static org.mockito.Matchers.any; + import static org.mockito.Mockito.when; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.verify; @@ -44,15 +46,18 @@ public class PersistentCacheTest { private final static String VALUE = "cache content"; private PersistentCache cache = null; private DirectoryLock lock = null; + private PersistentCacheInvalidation invalidation = null; @Rule public TemporaryFolder tmp = new TemporaryFolder(); @Before - public void setUp() { + public void setUp() throws IOException { + invalidation = mock(PersistentCacheInvalidation.class); + when(invalidation.test(any(Path.class))).thenReturn(false); lock = mock(DirectoryLock.class); when(lock.getFileLockName()).thenReturn("lock"); - cache = new PersistentCache(tmp.getRoot().toPath(), Long.MAX_VALUE, mock(Logger.class), lock); + cache = new PersistentCache(tmp.getRoot().toPath(), invalidation, mock(Logger.class), lock); } @Test @@ -67,9 +72,9 @@ public void testClean() throws Exception { cache.put(URI, VALUE.getBytes(StandardCharsets.UTF_8)); Files.write(lockFile, "test".getBytes(StandardCharsets.UTF_8)); assertCacheHit(true); - // negative time to make sure it is expired - cache = new PersistentCache(tmp.getRoot().toPath(), -100, mock(Logger.class), lock); + when(invalidation.test(any(Path.class))).thenReturn(true); cache.clean(); + when(invalidation.test(any(Path.class))).thenReturn(false); assertCacheHit(false); // lock file should not get deleted assertThat(new String(Files.readAllBytes(lockFile), StandardCharsets.UTF_8)).isEqualTo("test"); @@ -104,7 +109,6 @@ public void testCacheHit() throws Exception { @Test public void testReconfigure() throws Exception { - cache = new PersistentCache(tmp.getRoot().toPath(), Long.MAX_VALUE, mock(Logger.class), lock); assertCacheHit(false); cache.put(URI, VALUE.getBytes(StandardCharsets.UTF_8)); assertCacheHit(true); @@ -123,8 +127,7 @@ public void testReconfigure() throws Exception { @Test public void testExpiration() throws Exception { - // negative time to make sure it is expired - cache = new PersistentCache(tmp.getRoot().toPath(), -100, mock(Logger.class), lock); + when(invalidation.test(any(Path.class))).thenReturn(true); cache.put(URI, VALUE.getBytes(StandardCharsets.UTF_8)); assertCacheHit(false); } diff --git a/sonar-home/src/test/java/org/sonar/home/cache/TTLCacheInvalidationTest.java b/sonar-home/src/test/java/org/sonar/home/cache/TTLCacheInvalidationTest.java new file mode 100644 index 000000000000..92c7e5134a6a --- /dev/null +++ b/sonar-home/src/test/java/org/sonar/home/cache/TTLCacheInvalidationTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.home.cache; + +import org.junit.Before; + +import java.io.IOException; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; + +public class TTLCacheInvalidationTest { + private Path testFile; + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + + @Before + public void setUp() throws IOException { + testFile = temp.newFile().toPath(); + } + + @Test + public void testExpired() throws IOException { + TTLCacheInvalidation invalidation = new TTLCacheInvalidation(-100); + assertThat(invalidation.test(testFile)).isEqualTo(true); + } + + @Test + public void testValid() throws IOException { + TTLCacheInvalidation invalidation = new TTLCacheInvalidation(100_000); + assertThat(invalidation.test(testFile)).isEqualTo(false); + } +}