Support SNAPSHOTs and fix dependency cache

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);
             }

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<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;
@@ -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);
         }
     }
 }

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);
     }

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,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<>();

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<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"),