Skip to content

Commit

Permalink
Support SNAPSHOTs and fix dependency cache
Browse files Browse the repository at this point in the history
We will no longer fail when a SNAPSHOT dependency is used. In order
to make sure that SNAPSHOTs are periodically re-resolved, a TTL of
1 day was added to the cache file so that the cache is invalidated
every 24 hours. The cache can be manually invalidated using
`smithy clean`.

When implementing a TTL, I noticed that caching wasn't actually
working and was always invalidated. That's fixed now by using the
correct timestamp comparisons, and several new test cases were added.

Closes #1850
  • Loading branch information
mtdowling committed Jul 12, 2023
1 parent ca7f93c commit 8ec66cd
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
Expand All @@ -39,6 +41,9 @@
*/
public final class FileCacheResolver implements DependencyResolver {

// This is hard-coded for now to 1 day, and it can become an environment variable in the future if needed.
private static final Duration SMITHY_MAVEN_TTL = Duration.parse("P1D");

private static final Logger LOGGER = Logger.getLogger(FileCacheResolver.class.getName());
private final DependencyResolver delegate;
private final File location;
Expand Down Expand Up @@ -80,31 +85,45 @@ public List<ResolvedArtifact> resolve() {
}

private List<ResolvedArtifact> load() {
// Invalidate the cache if smithy-build.json was updated after the cache was written.
Path filePath = location.toPath();
if (!Files.exists(filePath)) {
// Invalidate the cache if:
// 1. smithy-build.json was updated after the cache was written.
// 2. the cache file is older than the allowed TTL.
// 3. a cached artifact was deleted.
// 4. a cached artifact is newer than the cache file.
long cacheLastModifiedMillis = location.lastModified();
long currentTimeMillis = new Date().getTime();
long ttlMillis = SMITHY_MAVEN_TTL.toMillis();

if (location.length() == 0) {
return Collections.emptyList();
} else if (!isCacheValid(location)) {
invalidate(filePath);
} else if (!isCacheValid(cacheLastModifiedMillis)) {
LOGGER.fine("Invalidating dependency cache: config is newer than the cache");
invalidate();
return Collections.emptyList();
} else if (currentTimeMillis - cacheLastModifiedMillis > ttlMillis) {
LOGGER.fine(() -> "Invalidating dependency cache: Cache exceeded TTL (TTL: " + ttlMillis + ")");
invalidate();
return Collections.emptyList();
}

ObjectNode node;
try (InputStream stream = Files.newInputStream(filePath)) {
try (InputStream stream = Files.newInputStream(location.toPath())) {
node = Node.parse(stream, location.toString()).expectObjectNode();
} catch (ModelSyntaxException | IOException e) {
throw new DependencyResolverException("Error loading dependency cache file from " + filePath, e);
throw new DependencyResolverException("Error loading dependency cache file from " + location, e);
}

List<ResolvedArtifact> result = new ArrayList<>(node.getStringMap().size());
for (Map.Entry<String, Node> entry : node.getStringMap().entrySet()) {
Path location = Paths.get(entry.getValue().expectStringNode().getValue());
// Invalidate the cache if the JAR file was updated after the cache was written.
if (isArtifactUpdatedSinceReferenceTime(location)) {
invalidate(filePath);
Path artifactLocation = Paths.get(entry.getValue().expectStringNode().getValue());
long lastModifiedOfArtifact = artifactLocation.toFile().lastModified();
// Invalidate the cache if the JAR file was updated since the cache was created.
if (lastModifiedOfArtifact == 0 || lastModifiedOfArtifact > cacheLastModifiedMillis) {
LOGGER.fine(() -> "Invalidating dependency cache: artifact is newer than cache: " + artifactLocation);
invalidate();
return Collections.emptyList();
}
result.add(ResolvedArtifact.fromCoordinates(location, entry.getKey()));
result.add(ResolvedArtifact.fromCoordinates(artifactLocation, entry.getKey()));
}

return result;
Expand All @@ -130,23 +149,13 @@ private void save(List<ResolvedArtifact> result) {
}
}

private boolean isCacheValid(File file) {
return referenceTimeInMillis <= file.lastModified() && file.length() > 0;
}

private boolean isArtifactUpdatedSinceReferenceTime(Path path) {
File file = path.toFile();
return !file.exists() || (referenceTimeInMillis > 0 && file.lastModified() > referenceTimeInMillis);
private boolean isCacheValid(long cacheLastModifiedMillis) {
return referenceTimeInMillis <= cacheLastModifiedMillis;
}

private void invalidate(Path filePath) {
try {
if (Files.exists(filePath)) {
LOGGER.fine("Invalidating dependency cache file: " + location);
Files.delete(filePath);
}
} catch (IOException e) {
throw new DependencyResolverException("Unable to delete cache file: " + e.getMessage(), e);
private void invalidate() {
if (location.exists() && !location.delete()) {
LOGGER.warning("Unable to invalidate dependency cache file: " + location);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,6 @@ private static Dependency createDependency(String coordinates, String scope) {
} catch (IllegalArgumentException e) {
throw new DependencyResolverException("Invalid dependency: " + e.getMessage());
}
if (artifact.isSnapshot()) {
throw new DependencyResolverException("Snapshot dependencies are not supported: " + artifact);
}
validateDependencyVersion(artifact);
return new Dependency(artifact, scope);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.function.Consumer;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.build.model.MavenRepository;
import software.amazon.smithy.utils.IoUtils;
Expand Down Expand Up @@ -48,38 +51,113 @@ public void ignoresAndDeletesEmptyCacheFiles() throws IOException {
}

@Test
public void loadsCacheFromDelegateWhenCacheMissingAndSaves() throws IOException {
public void invalidatesCacheWhenArtifactDeleted() throws IOException {
// Delete the "JAR" to invalidate the cache.
validateCacheScenario(File::delete);
}

@Test
public void invalidatesCacheWhenArtifactIsNewerThanCache() throws IOException {
// Set the last modified time of the "JAR" to the future to ensure the cache is invalidated.
validateCacheScenario(jar -> jar.setLastModified(new Date().getTime() + Duration.parse("P1D").toMillis()));
}

private void validateCacheScenario(Consumer<File> jarFileMutation) throws IOException {
File cache = File.createTempFile("classpath", ".json");
File jar = File.createTempFile("foo", ".json");
File jar = File.createTempFile("foo", ".jar");
Files.write(jar.toPath(), "{}".getBytes(StandardCharsets.UTF_8));

ResolvedArtifact artifact = ResolvedArtifact.fromCoordinates(jar.toPath(), "com.foo:bar:1.0.0");
List<ResolvedArtifact> result = new ArrayList<>();
result.add(artifact);

Mock mock = new Mock(result);
DependencyResolver resolver = new FileCacheResolver(cache, jar.lastModified(), mock);
List<ResolvedArtifact> resolved = resolver.resolve();
DependencyResolver cachingResolver = new FileCacheResolver(cache, jar.lastModified(), mock);
List<ResolvedArtifact> resolved = cachingResolver.resolve();

// Make sure artifacts were cached as expected.
assertThat(resolved, contains(artifact));
assertThat(IoUtils.readUtf8File(cache.toPath()), containsString("com.foo:bar:1.0.0"));

// Remove the canned entry from the mock to ensure the cache is working before delegating.
// Remove the canned entry from the mock so that when the cache is invalidated, we get a different result.
result.clear();

// Calling it again will load from the cached file.
assertThat(resolver.resolve(), contains(artifact));
// Calling it again will load from the cached file and not from the delegate mock that's now empty.
assertThat(cachingResolver.resolve(), contains(artifact));

// The cache should still be there.
assertThat(IoUtils.readUtf8File(cache.toPath()), containsString("com.foo:bar:1.0.0"));

// Removing the cache artifact invalidates the cache.
assertThat(jar.delete(), is(true));
// Mutate the JAR using the provided method. This method should invalidate the cache.
jarFileMutation.accept(jar);

assertThat(resolver.resolve(), empty());
// Resolving here skips the cache (which contains artifacts) and calls the delegate (which is now empty).
assertThat(cachingResolver.resolve(), empty());

// The caching resolver should now write an empty cache file.
assertThat(IoUtils.readUtf8File(cache.toPath()), containsString("{}"));
}

@Test
public void invalidatesCacheWhenConfigIsNewerThanCache() throws IOException {
File cache = File.createTempFile("classpath", ".json");
File jar = File.createTempFile("foo", ".jar");
Files.write(jar.toPath(), "{}".getBytes(StandardCharsets.UTF_8));

ResolvedArtifact artifact = ResolvedArtifact.fromCoordinates(jar.toPath(), "com.foo:bar:1.0.0");
List<ResolvedArtifact> result = new ArrayList<>();
result.add(artifact);

Mock mock = new Mock(result);
// Set the "config" last modified to a future date to ensure it's newer than the "JAR" file.
DependencyResolver cachingResolver = new FileCacheResolver(
cache,
jar.lastModified() + Duration.parse("P1D").toMillis(),
mock
);
List<ResolvedArtifact> resolved = cachingResolver.resolve();

// Make sure artifacts were cached as expected.
assertThat(resolved, contains(artifact));
assertThat(IoUtils.readUtf8File(cache.toPath()), containsString("com.foo:bar:1.0.0"));

// Remove the canned entry from the mock so that when the cache is invalidated, we get a different result.
result.clear();

// The cache will be invalidated here and reloaded from source, resulting in an empty result.
assertThat(cachingResolver.resolve(), empty());
}

@Test
public void invalidatesCacheWhenCacheExceedsTTL() throws IOException {
long tenDaysAgo = new Date().getTime() - Duration.parse("P10D").toMillis();
File cache = File.createTempFile("classpath", ".json");
File jar = File.createTempFile("foo", ".jar");
Files.write(jar.toPath(), "{}".getBytes(StandardCharsets.UTF_8));

ResolvedArtifact artifact = ResolvedArtifact.fromCoordinates(jar.toPath(), "com.foo:bar:1.0.0");
List<ResolvedArtifact> result = new ArrayList<>();
result.add(artifact);

Mock mock = new Mock(result);
// Make sure the config is set to 10 days ago too, so that config date checking doesn't invalidate.
DependencyResolver cachingResolver = new FileCacheResolver(cache, tenDaysAgo, mock);
List<ResolvedArtifact> resolved = cachingResolver.resolve();

// Make sure artifacts were cached as expected.
assertThat(resolved, contains(artifact));
assertThat(IoUtils.readUtf8File(cache.toPath()), containsString("com.foo:bar:1.0.0"));

// Remove the canned entry from the mock so that when the cache is invalidated, we get a different result.
result.clear();

// Change the last modified of the cache to a date in the distant past to invalidate the cache.
assertThat(cache.setLastModified(tenDaysAgo), is(true));

// The cache will be invalidated here and reloaded from source, resulting in an empty result.
assertThat(cachingResolver.resolve(), empty());
}

private static final class Mock implements DependencyResolver {
final List<ResolvedArtifact> artifacts;
final List<MavenRepository> repositories = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public void allowsValidDependenciesAndRepos() {
resolver.addDependency("com.foo:baz1:1.0.0");
resolver.addDependency("com.foo:baz2:[1.0.0]");
resolver.addDependency("com.foo:baz3:[1.0.0,]");
resolver.addDependency("smithy.foo:bar:1.25.0-SNAPSHOT");
}

@ParameterizedTest
Expand All @@ -36,7 +37,6 @@ public void validatesDependencies(String value) {
public static Stream<Arguments> invalidDependencies() {
return Stream.of(
Arguments.of("X"),
Arguments.of("smithy.foo:bar:1.25.0-SNAPSHOT"),
Arguments.of("smithy.foo:bar:RELEASE"),
Arguments.of("smithy.foo:bar:latest-status"),
Arguments.of("smithy.foo:bar:LATEST"),
Expand Down

0 comments on commit 8ec66cd

Please sign in to comment.