Support for full semantic versioning in LSP artifacts

aws · Jan 23, 2025 · 8fee1ac · 8fee1ac
1 parent faa503e
commit 8fee1ac
Show file tree

Hide file tree

Showing 13 changed files with 130 additions and 57 deletions.
diff --git a/plugin/.classpath b/plugin/.classpath
@@ -32,4 +32,4 @@
 	<classpathentry excluding="main/resources/|test/resources/" kind="src" path="src"/>
 	<classpathentry kind="src" path="target/generated-sources"/>
 	<classpathentry kind="output" path="target/classes"/>
-</classpath>
+</classpath>
diff --git a/plugin/.settings/org.eclipse.jdt.core.prefs b/plugin/.settings/org.eclipse.jdt.core.prefs
@@ -5,6 +5,7 @@ org.eclipse.jdt.core.compiler.compliance=17
 org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
 org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled
 org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
+org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning
 org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning
 org.eclipse.jdt.core.compiler.release=enabled
 org.eclipse.jdt.core.compiler.source=17
diff --git a/plugin/META-INF/MANIFEST.MF b/plugin/META-INF/MANIFEST.MF
@@ -51,6 +51,7 @@ Bundle-Classpath: target/classes/,
  target/dependency/jackson-databind-2.17.2.jar,
  target/dependency/jakarta.inject-api-2.0.1.jar,
  target/dependency/json-utils-2.28.26.jar,
+ target/dependency/maven-artifact-3.9.9.jar,
  target/dependency/metrics-spi-2.28.26.jar,
  target/dependency/netty-nio-client-2.28.26.jar,
  target/dependency/nimbus-jose-jwt-9.41.2.jar,

diff --git a/plugin/pom.xml b/plugin/pom.xml
@@ -91,6 +91,11 @@
            <artifactId>nimbus-jose-jwt</artifactId>
           <version>9.41.2</version>
         </dependency>
+        <dependency>
+            <groupId>org.apache.maven</groupId>
+            <artifactId>maven-artifact</artifactId>
+            <version>3.9.9</version>
+        </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter</artifactId>
@@ -134,7 +139,7 @@
                         </goals>
                         <configuration>
                             <outputDirectory>${project.build.directory}/dependency</outputDirectory>
-                            <includeGroupIds>software.amazon.awssdk,com.fasterxml.jackson,com.nimbusds,jakarta.inject,commons-codec,org.apache.httpcomponents,org.reactivestreams</includeGroupIds>
+                            <includeGroupIds>software.amazon.awssdk,com.fasterxml.jackson,com.nimbusds,jakarta.inject,commons-codec,org.apache.httpcomponents,org.reactivestreams,org.apache.maven</includeGroupIds>
                         </configuration>
                     </execution>
                     <execution>
@@ -207,7 +212,6 @@
                     </execution>
                 </executions>
                 <configuration>
-                    <argLine>@{argLine} -javaagent:${org.mockito:mockito-core:jar}</argLine>
                     <testSourceDirectory>${project.build.testSourceDirectory}</testSourceDirectory>
                 </configuration>
             </plugin>

diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/LspConstants.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/LspConstants.java
@@ -5,7 +5,10 @@
 
 import java.nio.file.Paths;
 
-import org.eclipse.osgi.service.resolver.VersionRange;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+
+import software.aws.toolkits.eclipse.amazonq.exception.AmazonQPluginException;
 
 public final class LspConstants {
     private LspConstants() {
@@ -25,5 +28,13 @@ private LspConstants() {
     public static final String LSP_SUBDIRECTORY = "lsp";
     public static final String AMAZONQ_LSP_SUBDIRECTORY = Paths.get(LSP_SUBDIRECTORY, "AmazonQ").toString();
 
-    public static final VersionRange LSP_SUPPORTED_VERSION_RANGE = new VersionRange("[3.0.0, 3.0.10)");
+    public static final VersionRange LSP_SUPPORTED_VERSION_RANGE = createVersionRange();
+
+    private static VersionRange createVersionRange() {
+        try {
+            return VersionRange.createFromVersionSpec("[3.0.0, 3.0.10)");
+        } catch (InvalidVersionSpecificationException e) {
+            throw new AmazonQPluginException("Failed to parse LSP supported version range", e);
+        }
+    }
 }
diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/ArtifactUtils.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/ArtifactUtils.java
@@ -16,7 +16,8 @@
 import java.util.zip.ZipFile;
 
 import org.apache.commons.codec.digest.DigestUtils;
-import org.osgi.framework.Version;
+import org.apache.maven.artifact.versioning.ArtifactVersion;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
 
 import software.aws.toolkits.eclipse.amazonq.exception.AmazonQPluginException;
 import software.aws.toolkits.eclipse.amazonq.plugin.Activator;
@@ -60,8 +61,8 @@ public static void copyDirectory(final Path source, final Path target) throws IO
              });
     }
 
-    public static Version parseVersion(final String versionString) {
-        return new Version(versionString);
+    public static ArtifactVersion parseVersion(final String versionString) {
+        return new DefaultArtifactVersion(versionString);
     }
 
     public static boolean validateHash(final Path file, final List<String> hashes, final boolean strict) {

diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcher.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcher.java
@@ -22,14 +22,15 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
-import org.osgi.framework.Version;
-import org.osgi.framework.VersionRange;
+import org.apache.maven.artifact.versioning.ArtifactVersion;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.VersionRange;
 
 import software.aws.toolkits.eclipse.amazonq.exception.AmazonQPluginException;
 import software.aws.toolkits.eclipse.amazonq.exception.LspError;
 import software.aws.toolkits.eclipse.amazonq.lsp.manager.LspConstants;
 import software.aws.toolkits.eclipse.amazonq.lsp.manager.LspFetchResult;
-import software.aws.toolkits.eclipse.amazonq.lsp.manager.model.ArtifactVersion;
+import software.aws.toolkits.eclipse.amazonq.lsp.manager.model.ManifestArtifactVersion;
 import software.aws.toolkits.eclipse.amazonq.lsp.manager.model.Content;
 import software.aws.toolkits.eclipse.amazonq.lsp.manager.model.Manifest;
 import software.aws.toolkits.eclipse.amazonq.lsp.manager.model.Target;
@@ -187,7 +188,7 @@ private void logMessageWithLicense(final String message, final String attributio
         }
     }
 
-    private Optional<ArtifactVersion> resolveVersion(final Manifest manifestFile, final PluginPlatform platform,
+    private Optional<ManifestArtifactVersion> resolveVersion(final Manifest manifestFile, final PluginPlatform platform,
             final PluginArchitecture architecture, final Instant start) {
         if (manifestFile == null) {
             String failureReason = "No valid manifest version data was received. An error could have caused this. Please check logs.";
@@ -201,8 +202,8 @@ private Optional<ArtifactVersion> resolveVersion(final Manifest manifestFile, fi
                 .filter(version -> version.targets().stream()
                         .anyMatch(target -> hasRequiredTargetContent(target, platform, architecture)))
                 .max(Comparator.comparing(
-                        artifactVersion -> Version.parseVersion(artifactVersion.serverVersion())
-                    ));
+                        artifactVersion -> new DefaultArtifactVersion(artifactVersion.serverVersion())
+                ));
     }
 
     private boolean hasRequiredTargetContent(final Target target, final PluginPlatform platform, final PluginArchitecture architecture) {
@@ -215,11 +216,11 @@ private boolean isCompatibleTarget(final Target target, final PluginPlatform pla
         return target.platform().equalsIgnoreCase(platform.getValue()) && target.arch().equalsIgnoreCase(architecture.getValue());
     }
 
-    private boolean isCompatibleVersion(final ArtifactVersion version) {
-        return versionRange.includes(ArtifactUtils.parseVersion(version.serverVersion())) && !version.isDelisted();
+    private boolean isCompatibleVersion(final ManifestArtifactVersion version) {
+        return versionRange.containsVersion(ArtifactUtils.parseVersion(version.serverVersion())) && !version.isDelisted();
     }
 
-    private Optional<Target> resolveTarget(final Optional<ArtifactVersion> targetVersion, final PluginPlatform platform,
+    private Optional<Target> resolveTarget(final Optional<ManifestArtifactVersion> targetVersion, final PluginPlatform platform,
             final PluginArchitecture architecture) {
         return targetVersion.flatMap(version -> version.targets().stream()
                 .filter(target -> isCompatibleTarget(target, platform, architecture))
@@ -320,9 +321,9 @@ private Path getFallback(final String expectedLspVersion, final PluginPlatform p
         // filter to get sorted list of compatible lsp versions that have a valid cache
         var sortedCachedLspVersions = compatibleLspVersions.stream()
                 .filter(artifactVersion -> isValidCachedVersion(artifactVersion, expectedServerVersion, cachedVersions))
-                .sorted(Comparator.comparing(x -> Version.parseVersion(x.serverVersion()), Comparator.reverseOrder()))
+                .sorted((v1, v2) -> new DefaultArtifactVersion(v2.serverVersion())
+                        .compareTo(new DefaultArtifactVersion(v1.serverVersion())))
                 .collect(Collectors.toList());
-
         var fallbackDir = sortedCachedLspVersions.stream()
                 .map(x -> getValidLocalCacheDirectory(x, platform, architecture, destinationFolder))
                 .filter(Objects::nonNull).findFirst().orElse(null);
@@ -333,7 +334,7 @@ private Path getFallback(final String expectedLspVersion, final PluginPlatform p
      * Validate the local cache directory of the given lsp version(matches expected hash)
      * If valid return cache directory, else return null
      */
-    private Path getValidLocalCacheDirectory(final ArtifactVersion artifactVersion, final PluginPlatform platform,
+    private Path getValidLocalCacheDirectory(final ManifestArtifactVersion artifactVersion, final PluginPlatform platform,
             final PluginArchitecture architecture, final Path destinationFolder) {
         var target = resolveTarget(Optional.of(artifactVersion), platform, architecture);
         if (!target.isPresent() || target.get().contents() == null || target.get().contents().isEmpty()) {
@@ -345,14 +346,14 @@ private Path getValidLocalCacheDirectory(final ArtifactVersion artifactVersion,
         return hasValidCache ? cacheDir : null;
     }
 
-    private boolean isValidCachedVersion(final ArtifactVersion lspVersion, final Version expectedServerVersion,
-            final List<Version> cachedVersions) {
+    private boolean isValidCachedVersion(final ManifestArtifactVersion lspVersion, final ArtifactVersion expectedServerVersion,
+            final List<ArtifactVersion> cachedVersions) {
         var serverVersion = ArtifactUtils.parseVersion(lspVersion.serverVersion());
 
         return cachedVersions.contains(serverVersion) && (serverVersion.compareTo(expectedServerVersion) <= 0);
     }
 
-    private List<Version> getCachedVersions(final Path destinationFolder) {
+    private List<ArtifactVersion> getCachedVersions(final Path destinationFolder) {
         try {
             return Files.list(destinationFolder)
                        .filter(Files::isDirectory)
@@ -366,15 +367,15 @@ private List<Version> getCachedVersions(final Path destinationFolder) {
         }
     }
 
-    private Version getVersionedName(final String filename) {
+    private ArtifactVersion getVersionedName(final String filename) {
         try {
             return ArtifactUtils.parseVersion(filename);
         } catch (Exception e) {
             return null;
         }
     }
 
-    private List<ArtifactVersion> getCompatibleArtifactVersions() {
+    private List<ManifestArtifactVersion> getCompatibleArtifactVersions() {
         return manifest.versions().stream()
                 .filter(version -> isCompatibleVersion(version))
                 .toList();
@@ -385,7 +386,8 @@ private void deleteDelistedVersions(final Path destinationFolder) {
         var cachedVersions = getCachedVersions(destinationFolder);
 
         // delete de-listed versions in the toolkit compatible version range
-        var delistedVersions = cachedVersions.stream().filter(x -> !compatibleVersions.contains(x) && versionRange.includes(x)).collect(Collectors.toList());
+        var delistedVersions = cachedVersions.stream()
+                .filter(x -> !compatibleVersions.contains(x) && versionRange.containsVersion(x)).collect(Collectors.toList());
         if (delistedVersions.size() > 0) {
             Activator.getLogger().info(String.format("Cleaning up %s cached de-listed versions for Amazon Q Language Server", delistedVersions.size()));
         }
@@ -398,7 +400,7 @@ private void deleteExtraVersions(final Path destinationFolder) {
         var cachedVersions = getCachedVersions(destinationFolder);
         // delete extra versions in the compatible toolkit version range except highest 2 versions
         var extraVersions = cachedVersions.stream()
-                .filter(x -> versionRange.includes(x))
+                .filter(x -> versionRange.containsVersion(x))
                 .sorted(Comparator.reverseOrder())
                 .skip(2)
                 .collect(Collectors.toList());
@@ -410,7 +412,7 @@ private void deleteExtraVersions(final Path destinationFolder) {
         });
     }
 
-    private void deleteCachedVersion(final Path destinationFolder, final Version version) {
+    private void deleteCachedVersion(final Path destinationFolder, final ArtifactVersion version) {
         var versionPath = destinationFolder.resolve(version.toString());
         ArtifactUtils.deleteDirectory(versionPath);
     }

diff --git a/...src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/VersionManifestFetcher.java b/...src/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/VersionManifestFetcher.java
@@ -145,7 +145,7 @@ private Optional<Manifest> validateManifest(final String content) {
         try {
             var manifest = OBJECT_MAPPER.readValue(content, Manifest.class);
             var version = ArtifactUtils.parseVersion(manifest.manifestSchemaVersion());
-            if (version.getMajor() == LspConstants.MANIFEST_MAJOR_VERSION) {
+            if (version.getMajorVersion() == LspConstants.MANIFEST_MAJOR_VERSION) {
                 return Optional.of(manifest);
             }
         } catch (Exception e) {

diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/model/Manifest.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/manager/model/Manifest.java
@@ -14,5 +14,5 @@ public record Manifest(
         String artifactId,
         String artifactDescription,
         Boolean isManifestDeprecated,
-        @JsonSetter(nulls = Nulls.AS_EMPTY) List<ArtifactVersion> versions) {
+        @JsonSetter(nulls = Nulls.AS_EMPTY) List<ManifestArtifactVersion> versions) {
 }
diff --git a/...nq/lsp/manager/model/ArtifactVersion.java → ...anager/model/ManifestArtifactVersion.java b/...nq/lsp/manager/model/ArtifactVersion.java → ...anager/model/ManifestArtifactVersion.java
@@ -9,7 +9,7 @@
 import com.fasterxml.jackson.annotation.JsonSetter;
 import com.fasterxml.jackson.annotation.Nulls;
 
-public record ArtifactVersion(
+public record ManifestArtifactVersion(
         @JsonProperty(required = true) String serverVersion,
         boolean isDelisted,
         Runtime runtime,

diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/configuration/DefaultPluginStoreTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/configuration/DefaultPluginStoreTest.java
@@ -69,6 +69,7 @@ final void testPutAndGetKeySuccess(final String key) throws Exception {
         assertEquals(value, pluginStore.get(key));
         verifyNoInteractions(mockLogger);
     }
+
     @Test
     void testPutOverridingValue() throws BackingStoreException {
         String key = "testKey";

diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/ArtifactUtilsTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/ArtifactUtilsTest.java
@@ -3,9 +3,9 @@
 
 package software.aws.toolkits.eclipse.amazonq.lsp.manager.fetcher;
 
+import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
-import org.osgi.framework.Version;
 
 import java.io.IOException;
 import java.nio.file.FileSystem;
@@ -40,10 +40,19 @@ void testExtractFile(@TempDir final Path tempDir) throws IOException {
 
     @Test
     void testParseVersion() {
-        Version version = ArtifactUtils.parseVersion("1.2.3");
-        assertEquals(1, version.getMajor());
-        assertEquals(2, version.getMinor());
-        assertEquals(3, version.getMicro());
+        ArtifactVersion version = ArtifactUtils.parseVersion("1.2.3");
+        assertEquals(1, version.getMajorVersion());
+        assertEquals(2, version.getMinorVersion());
+        assertEquals(3, version.getIncrementalVersion());
+    }
+
+    @Test
+    void testParseVersionWithQualifier() {
+        ArtifactVersion version = ArtifactUtils.parseVersion("1.2.3-rc.1");
+        assertEquals(1, version.getMajorVersion());
+        assertEquals(2, version.getMinorVersion());
+        assertEquals(3, version.getIncrementalVersion());
+        assertEquals("rc.1", version.getQualifier());
     }
 
     @Test