diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverTest.java index 37bc7f8a707..40bb424a8a7 100644 --- a/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverTest.java +++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverTest.java @@ -117,7 +117,7 @@ public void ignoresEmptyCacheFiles() { RunResult result = IntegUtils.run(path, ListUtils.of("validate", "--debug", "model")); assertThat(result.getExitCode(), is(0)); - assertThat(result.getOutput(), containsString("Invalidating dependency cache")); + assertThat(result.getOutput(), containsString("Resolving Maven dependencies for Smithy CLI")); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/FileCacheResolver.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/FileCacheResolver.java index 6d1fe3d4b41..3c439f7e0d1 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/FileCacheResolver.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/FileCacheResolver.java @@ -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; @@ -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; @@ -80,31 +85,45 @@ public List resolve() { } private List 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 result = new ArrayList<>(node.getStringMap().size()); for (Map.Entry 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; @@ -130,23 +149,13 @@ private void save(List 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); } } } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java index 08061dce90b..b10d0da50cd 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java @@ -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); } diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/FileCacheResolverTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/FileCacheResolverTest.java index 12b95c5b5ba..629219eb46c 100644 --- a/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/FileCacheResolverTest.java +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/FileCacheResolverTest.java @@ -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; @@ -48,9 +51,20 @@ 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 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"); @@ -58,28 +72,92 @@ public void loadsCacheFromDelegateWhenCacheMissingAndSaves() throws IOException result.add(artifact); Mock mock = new Mock(result); - DependencyResolver resolver = new FileCacheResolver(cache, jar.lastModified(), mock); - List resolved = resolver.resolve(); + DependencyResolver cachingResolver = new FileCacheResolver(cache, jar.lastModified(), mock); + List 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 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 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 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 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 artifacts; final List repositories = new ArrayList<>(); diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolverTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolverTest.java index c618d1af2b4..05d861d6ca1 100644 --- a/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolverTest.java +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolverTest.java @@ -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 @@ -36,7 +37,6 @@ public void validatesDependencies(String value) { public static Stream 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"),