diff --git a/pom.xml b/pom.xml index 4afc5fd..13badf8 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ jar org.webjars webjars-locator-lite - 1.0.0-SNAPSHOT + 0.0.5-SNAPSHOT webjars-locator-lite WebJars Locator Lite 2012 diff --git a/src/main/java/org/webjars/WebJarCache.java b/src/main/java/org/webjars/WebJarCache.java index 9c15d25..b3d8a71 100644 --- a/src/main/java/org/webjars/WebJarCache.java +++ b/src/main/java/org/webjars/WebJarCache.java @@ -1,7 +1,8 @@ package org.webjars; -import org.jspecify.annotations.Nullable; +import java.util.Optional; +import java.util.function.Function; /** * WebJar Locator Cache Interface @@ -10,8 +11,7 @@ */ public interface WebJarCache { - @Nullable String get(final String key); - - void put(final String key, final String value); + // todo: null can't be cached but if the locator can't find something, it never will, so consider having the compute function return Optional so that we can cache the non-existence + Optional computeIfAbsent(String key, Function> function); } diff --git a/src/main/java/org/webjars/WebJarCacheDefault.java b/src/main/java/org/webjars/WebJarCacheDefault.java index 3029782..b5e09e9 100644 --- a/src/main/java/org/webjars/WebJarCacheDefault.java +++ b/src/main/java/org/webjars/WebJarCacheDefault.java @@ -1,25 +1,20 @@ package org.webjars; -import org.jspecify.annotations.Nullable; - -import java.util.concurrent.ConcurrentHashMap; +import java.util.Optional; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; public class WebJarCacheDefault implements WebJarCache { - final ConcurrentHashMap cache; + final ConcurrentMap> cache; - public WebJarCacheDefault(ConcurrentHashMap cache) { + public WebJarCacheDefault(ConcurrentMap> cache) { this.cache = cache; } @Override - public @Nullable String get(String key) { - return cache.get(key); - } - - @Override - public void put(String key, String value) { - cache.put(key, value); + public Optional computeIfAbsent(String key, Function> function) { + return cache.computeIfAbsent(key, function); } } diff --git a/src/main/java/org/webjars/WebJarVersionLocator.java b/src/main/java/org/webjars/WebJarVersionLocator.java index fd29012..29b245a 100644 --- a/src/main/java/org/webjars/WebJarVersionLocator.java +++ b/src/main/java/org/webjars/WebJarVersionLocator.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.io.InputStream; +import java.util.Optional; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; @@ -44,27 +45,19 @@ public WebJarVersionLocator(WebJarCache cache) { */ @Nullable public String fullPath(final String webJarName, final String exactPath) { - final String cacheKey = "fullpath-" + webJarName + "-" + exactPath; - final String maybeCached = cache.get(cacheKey); - if (maybeCached == null) { - final String version = version(webJarName); - String fullPath = String.format("%s/%s/%s", WEBJARS_PATH_PREFIX, webJarName, exactPath); - if (!isEmpty(version)) { - if (!exactPath.startsWith(version)) { - fullPath = String.format("%s/%s/%s/%s", WEBJARS_PATH_PREFIX, webJarName, version, exactPath); - } - } + final String version = version(webJarName); - if (LOADER.getResource(fullPath) != null) { - cache.put(cacheKey, fullPath); - return fullPath; + if (!isEmpty(version)) { + // todo: not sure why we check this + if (!exactPath.startsWith(version)) { + return String.format("%s/%s/%s/%s", WEBJARS_PATH_PREFIX, webJarName, version, exactPath); + } + else { + return String.format("%s/%s/%s", WEBJARS_PATH_PREFIX, webJarName, exactPath); } - - return null; - } - else { - return maybeCached; } + + return null; } /** @@ -75,13 +68,13 @@ public String fullPath(final String webJarName, final String exactPath) { */ @Nullable public String path(final String webJarName, final String exactPath) { - String maybeFullPath = fullPath(webJarName, exactPath); - if (maybeFullPath != null) { - return maybeFullPath.replace(WEBJARS_PATH_PREFIX + "/", ""); - } - else { - return null; + final String version = version(webJarName); + + if (!isEmpty(version)) { + return String.format("%s/%s/%s", webJarName, version, exactPath); } + + return null; } /** @@ -102,8 +95,7 @@ public String webJarVersion(final String webJarName) { @Nullable public String version(final String webJarName) { final String cacheKey = "version-" + webJarName; - final String maybeCached = cache.get(cacheKey); - if (maybeCached == null) { + final Optional optionalVersion = cache.computeIfAbsent(cacheKey, (key) -> { InputStream resource = LOADER.getResourceAsStream(PROPERTIES_ROOT + NPM + webJarName + POM_PROPERTIES); if (resource == null) { resource = LOADER.getResourceAsStream(PROPERTIES_ROOT + PLAIN + webJarName + POM_PROPERTIES); @@ -128,24 +120,21 @@ public String version(final String webJarName) { // Sometimes a webjar version is not the same as the Maven artifact version if (version != null) { if (hasResourcePath(webJarName, version)) { - cache.put(cacheKey, version); - return version; + return Optional.of(version); } if (version.contains("-")) { version = version.substring(0, version.indexOf("-")); if (hasResourcePath(webJarName, version)) { - cache.put(cacheKey, version); - return version; + return Optional.of(version); } } } } - return null; - } - else { - return maybeCached; - } + return Optional.empty(); + }); + + return optionalVersion.orElse(null); } private boolean hasResourcePath(final String webJarName, final String path) { diff --git a/src/test/java/org/webjars/WebJarVersionLocatorTest.java b/src/test/java/org/webjars/WebJarVersionLocatorTest.java index 498441b..718ab7f 100644 --- a/src/test/java/org/webjars/WebJarVersionLocatorTest.java +++ b/src/test/java/org/webjars/WebJarVersionLocatorTest.java @@ -5,7 +5,10 @@ import org.junit.Test; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; public class WebJarVersionLocatorTest { @@ -41,19 +44,48 @@ public void full_path_exists_version_supplied() { @Test public void cache_is_populated_on_lookup() { - final ConcurrentHashMap cache = new ConcurrentHashMap<>(); - final WebJarVersionLocator webJarVersionLocator = new WebJarVersionLocator(new WebJarCacheDefault(cache)); + AtomicInteger numLookups = new AtomicInteger(0); + + class InspectableCache implements WebJarCache { + final ConcurrentHashMap> cache = new ConcurrentHashMap<>(); + + @Override + public Optional computeIfAbsent(String key, Function> function) { + Function> inspectableFunction = function.andThen((value) -> { + numLookups.incrementAndGet(); + return value; + }); + return cache.computeIfAbsent(key, inspectableFunction); + } + } + + final WebJarVersionLocator webJarVersionLocator = new WebJarVersionLocator(new InspectableCache()); assertEquals("3.1.1", webJarVersionLocator.version("bootstrap")); - assertEquals(1, cache.size()); + assertEquals(1, numLookups.get()); // should hit the cache and produce the same value - // todo: test that it was actually a cache hit assertEquals("3.1.1", webJarVersionLocator.version("bootstrap")); + assertEquals(1, numLookups.get()); + // version is already cached so we shouldn't hit it again assertEquals(WebJarVersionLocator.WEBJARS_PATH_PREFIX + "/bootstrap/3.1.1/js/bootstrap.js", webJarVersionLocator.fullPath("bootstrap", "js/bootstrap.js")); - assertEquals(2, cache.size()); - // should hit the cache and produce the same value - // todo: test that it was actually a cache hit - assertEquals(WebJarVersionLocator.WEBJARS_PATH_PREFIX + "/bootstrap/3.1.1/js/bootstrap.js", webJarVersionLocator.fullPath("bootstrap", "js/bootstrap.js")); + assertEquals(1, numLookups.get()); + + // make sure we don't hit the cache for another file in the already resolved WebJar + assertEquals(WebJarVersionLocator.WEBJARS_PATH_PREFIX + "/bootstrap/3.1.1/css/bootstrap.css", webJarVersionLocator.fullPath("bootstrap", "css/bootstrap.css")); + assertEquals(1, numLookups.get()); + + // another WebJar should hit the cache but only once + assertEquals("3.1.1", webJarVersionLocator.version("bootswatch-yeti")); + assertEquals(2, numLookups.get()); + + assertEquals("3.1.1", webJarVersionLocator.version("bootswatch-yeti")); + assertEquals(2, numLookups.get()); + + assertNull(webJarVersionLocator.version("asdf")); + assertEquals(3, numLookups.get()); + + assertNull(webJarVersionLocator.version("asdf")); + assertEquals(3, numLookups.get()); } }