Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for locator.properties #15

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@
<version>3.1.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.webjars.bower</groupId>
<artifactId>js-base64</artifactId>
<version>2.3.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
41 changes: 40 additions & 1 deletion src/main/java/org/webjars/WebJarVersionLocator.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.Enumeration;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;
Expand All @@ -25,17 +28,22 @@ public class WebJarVersionLocator {
private static final String NPM = "org.webjars.npm/";
private static final String PLAIN = "org.webjars/";
private static final String POM_PROPERTIES = "/pom.properties";
private static final String LOCATOR_PROPERTIES = "/locator.properties";

private static final String CACHE_KEY_PREFIX = "version-";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this to avoid duplication of the string


private static final ClassLoader LOADER = WebJarVersionLocator.class.getClassLoader();

private final WebJarCache cache;

public WebJarVersionLocator() {
this.cache = new WebJarCacheDefault(new ConcurrentHashMap<>());
readLocatorProperties();
}

WebJarVersionLocator(WebJarCache cache) {
this.cache = cache;
readLocatorProperties();
}

/**
Expand Down Expand Up @@ -83,7 +91,7 @@ public String path(final String webJarName, final String exactPath) {
*/
@Nullable
public String version(final String webJarName) {
final String cacheKey = "version-" + webJarName;
final String cacheKey = CACHE_KEY_PREFIX + webJarName;
final Optional<String> optionalVersion = cache.computeIfAbsent(cacheKey, (key) -> {
InputStream resource = LOADER.getResourceAsStream(PROPERTIES_ROOT + NPM + webJarName + POM_PROPERTIES);
if (resource == null) {
Expand Down Expand Up @@ -126,6 +134,37 @@ public String version(final String webJarName) {
return optionalVersion.orElse(null);
}

private void readLocatorProperties() {
try {
Enumeration<URL> resources =
LOADER.getResources(WEBJARS_PATH_PREFIX + LOCATOR_PROPERTIES);
while (resources.hasMoreElements()) {
URL resourceUrl = resources.nextElement();
try (InputStream resource = resourceUrl.openStream()) {
Properties properties = new Properties();
properties.load(resource);
for (Map.Entry<Object, Object> entry : properties.entrySet()) {
String webJarName = entry.getKey().toString();
if (!webJarName.endsWith(".version")) {
// ".version" suffix is required
continue;
}

webJarName = webJarName.substring(0, webJarName.lastIndexOf(".version"));

String version = entry.getValue().toString();
if (hasResourcePath(webJarName, version)) {
// Only add configured versions if their path exists
cache.computeIfAbsent(CACHE_KEY_PREFIX + webJarName, x -> Optional.of(version));
}
}
}
}
} catch (IOException e) {
throw new RuntimeException("unable to load locator properties", e);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this exception should be rethrown, logged or just ignored (similar to version()). For now I opted for throwing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this Exception should only happen if the the locator.properties was found but could not be read. So we should be good if there is no locator.properties - is that right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, LOADER.getResources returns an empty Enumeration when there are no files found. I verified that by removing the locator.properties from test resources.

}
}

private boolean hasResourcePath(final String webJarName, final String path) {
return LOADER.getResource(WEBJARS_PATH_PREFIX + "/" + webJarName + "/" + path) != null;
}
Expand Down
23 changes: 22 additions & 1 deletion src/test/java/org/webjars/WebJarVersionLocatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;

Expand All @@ -23,6 +24,21 @@ void should_get_a_webjar_version() {
assertEquals("3.1.1", new WebJarVersionLocator().version("bootswatch-yeti"));
}

@Test
void should_find_good_custom_webjar_version() {
assertEquals("3.2.1", new WebJarVersionLocator().version("goodwebjar"));
}

@Test
void should_not_find_bad_custom_webjar_version() {
assertNull(new WebJarVersionLocator().version("badwebjar"));
}

@Test
void should_find_bower_webjar_version() {
assertEquals("2.3.2", new WebJarVersionLocator().version("js-base64"));
}

@Test
void webjar_version_doesnt_match_path() {
assertEquals("3.1.1", new WebJarVersionLocator().version("bootstrap"));
Expand All @@ -45,6 +61,7 @@ void full_path_exists_version_supplied() {

@Test
void cache_is_populated_on_lookup() {
AtomicBoolean shouldInspect = new AtomicBoolean(false);
AtomicInteger numLookups = new AtomicInteger(0);

@NullMarked
Expand All @@ -54,14 +71,18 @@ class InspectableCache implements WebJarCache {
@Override
public Optional<String> computeIfAbsent(String key, Function<String, Optional<String>> function) {
Function<String, Optional<String>> inspectableFunction = function.andThen((value) -> {
numLookups.incrementAndGet();
if(shouldInspect.get()) {
numLookups.incrementAndGet();
}
return value;
});
return cache.computeIfAbsent(key, inspectableFunction);
}
}

final WebJarVersionLocator webJarVersionLocator = new WebJarVersionLocator(new InspectableCache());
// enable inspection after webJarVersionLocator has been constructed, to ignore lookups caused by loading locator.properties
shouldInspect.set(true);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first Idea was to modifiy the assertEquals calls for numLookups but I think that this is a better solution.


assertEquals("3.1.1", webJarVersionLocator.version("bootstrap"));
assertEquals(1, numLookups.get());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Awesome WebJar content
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# WebJar that can be found on classpath
goodwebjar.version=3.2.1

# WebJar that is not present in classpath
badwebjar.version=1.2.3

# Bower Webjar from dependencies
js-base64.version=2.3.2