Bypass download if CLI version matches (#245)

* Bypass download if CLI version matches

This is a bit faster than waiting for a 304 but more importantly it
works if the user customizes the download source to be a place that does
not have the same etag support that Coder does.

The test bypasses Windows since we would need to create an executable
there for the test.  We could split out the exec and the parsing
although I am not sure it is worth the noise.  The part most likely to
break between platforms is probably the exec, not the parsing?

* Make invalid JSON test less obscure

* Properly compare semver

Author: code-asher

src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt

@@ -2,6 +2,7 @@ package com.coder.gateway.sdk
 
 import com.coder.gateway.models.WorkspaceAgentModel
 import com.coder.gateway.views.steps.CoderWorkspacesStepView
+import com.google.gson.Gson
 import com.intellij.openapi.diagnostic.Logger
 import org.zeroturnaround.exec.ProcessExecutor
 import java.io.BufferedInputStream
@@ -276,11 +277,42 @@ class CoderCLIManager @JvmOverloads constructor(
         }
     }
 
+    /**
+     * Version output from the CLI's version command.
+     */
+    private data class Version(
+        val version: String,
+    )
+
     /**
      * Return the binary version.
+     *
+     * Throws if it could not be determined.
      */
-    fun version(): String {
-        return exec("version")
+    fun version(): CoderSemVer {
+        val raw = exec("version", "--output", "json")
+        val json = Gson().fromJson(raw, Version::class.java)
+        if (json?.version == null) {
+            throw InvalidVersionException("No version found in output")
+        }
+        return CoderSemVer.parse(json.version)
+    }
+
+    /**
+     * Returns true if the CLI has the same major/minor/patch version as the
+     * provided version and false if it does not match or the CLI version could
+     * not be determined or the provided version is invalid.
+     */
+    fun matchesVersion(buildVersion: String): Boolean {
+        return try {
+            val cliVersion = version()
+            val matches = cliVersion == CoderSemVer.parse(buildVersion)
+            logger.info("$localBinaryPath version $cliVersion matches $buildVersion: $matches")
+            matches
+        } catch (e: Exception) {
+            logger.info("Unable to determine $localBinaryPath version: ${e.message}")
+            false
+        }
     }
 
     private fun exec(vararg args: String): String {

src/main/kotlin/com/coder/gateway/sdk/CoderSemVer.kt

@@ -14,7 +14,7 @@ class CoderSemVer(private val major: Long = 0, private val minor: Long = 0, priv
 
 
     override fun toString(): String {
-        return "CoderSemVer(major=$major, minor=$minor)"
+        return "CoderSemVer(major=$major, minor=$minor, patch=$patch)"
     }
 
     override fun equals(other: Any?): Boolean {

src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

@@ -467,8 +467,15 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
                 appPropertiesService.setValue(CODER_URL_KEY, deploymentURL.toString())
                 appPropertiesService.setValue(SESSION_TOKEN, token.first)
 
-                this.indicator.text = "Downloading Coder CLI..."
-                cliManager.downloadCLI()
+                // Short-circuit if we already have the expected version.  This
+                // lets us bypass the 304 which is slower and may not be
+                // supported if the binary is downloaded from alternate sources.
+                // For CLIs without the JSON output flag we will fall back to
+                // the 304 method.
+                if (!cliManager.matchesVersion(clientService.buildVersion)) {
+                    this.indicator.text = "Downloading Coder CLI..."
+                    cliManager.downloadCLI()
+                }
 
                 this.indicator.text = "Authenticating Coder CLI..."
                 cliManager.login(token.first)

src/test/groovy/CoderCLIManagerTest.groovy

@@ -1,14 +1,12 @@
 package com.coder.gateway.sdk
 
+import com.google.gson.JsonSyntaxException
 import com.sun.net.httpserver.HttpExchange
 import com.sun.net.httpserver.HttpHandler
 import com.sun.net.httpserver.HttpServer
 import org.zeroturnaround.exec.InvalidExitValueException
 import org.zeroturnaround.exec.ProcessInitException
-import spock.lang.Requires
-import spock.lang.Shared
-import spock.lang.Specification
-import spock.lang.Unroll
+import spock.lang.*
 
 import java.nio.file.Files
 import java.nio.file.Path
@@ -134,10 +132,11 @@ class CoderCLIManagerTest extends Specification {
 
         when:
         def downloaded = ccm.downloadCLI()
+        ccm.version()
 
         then:
         downloaded
-        ccm.version().contains("Coder")
+        noExceptionThrown()
 
         // Make sure login failures propagate correctly.
         when:
@@ -161,7 +160,7 @@ class CoderCLIManagerTest extends Specification {
         // The mock does not serve a binary that works on Windows so do not
         // actually execute.  Checking the contents works just as well as proof
         // that the binary was correctly downloaded anyway.
-        ccm.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url'"
+        ccm.localBinaryPath.toFile().text.contains(url)
 
         cleanup:
         srv.stop(0)
@@ -172,7 +171,7 @@ class CoderCLIManagerTest extends Specification {
         def ccm = new CoderCLIManager(new URL("https://foo"), tmpdir.resolve("does-not-exist"))
 
         when:
-        ccm.version()
+        ccm.login("token")
 
         then:
         thrown(ProcessInitException)
@@ -193,7 +192,7 @@ class CoderCLIManagerTest extends Specification {
         downloaded
         ccm.localBinaryPath.toFile().readBytes() != "cli".getBytes()
         ccm.localBinaryPath.toFile().lastModified() > 0
-        ccm.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url'"
+        ccm.localBinaryPath.toFile().text.contains(url)
 
         cleanup:
         srv.stop(0)
@@ -207,6 +206,7 @@ class CoderCLIManagerTest extends Specification {
         when:
         def downloaded1 = ccm.downloadCLI()
         ccm.localBinaryPath.toFile().setLastModified(0)
+        // Download will be skipped due to a 304.
         def downloaded2 = ccm.downloadCLI()
 
         then:
@@ -231,8 +231,8 @@ class CoderCLIManagerTest extends Specification {
 
         then:
         ccm1.localBinaryPath != ccm2.localBinaryPath
-        ccm1.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url1'"
-        ccm2.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url2'"
+        ccm1.localBinaryPath.toFile().text.contains(url1)
+        ccm2.localBinaryPath.toFile().text.contains(url2)
 
         cleanup:
         srv1.stop(0)
@@ -249,7 +249,7 @@ class CoderCLIManagerTest extends Specification {
 
         then:
         downloaded
-        ccm.localBinaryPath.toFile().text == "#!/bin/sh\necho '${expected.replace("{{url}}", url)}'"
+        ccm.localBinaryPath.toFile().text.contains(expected.replace("{{url}}", url))
 
         cleanup:
         srv.stop(0)
@@ -429,4 +429,83 @@ class CoderCLIManagerTest extends Specification {
                 "malformed-start-after-end",
         ]
     }
+
+    @IgnoreIf({ os.windows })
+    def "parses version"() {
+        given:
+        def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir)
+        Files.createDirectories(ccm.localBinaryPath.parent)
+
+        when:
+        ccm.localBinaryPath.toFile().text = "#!/bin/sh\n$contents"
+        ccm.localBinaryPath.toFile().setExecutable(true)
+
+        then:
+        ccm.version() == expected
+
+        where:
+        contents                                                 | expected
+        """echo '{"version": "1.0.0"}'"""                        | CoderSemVer.parse("1.0.0")
+        """echo '{"version": "1.0.0", "foo": true, "baz": 1}'""" | CoderSemVer.parse("1.0.0")
+    }
+
+    @IgnoreIf({ os.windows })
+    def "fails to parse version"() {
+        given:
+        def ccm = new CoderCLIManager(new URL("https://test.coder.parse-fail.invalid"), tmpdir)
+        Files.createDirectories(ccm.localBinaryPath.parent)
+
+        when:
+        if (contents != null) {
+            ccm.localBinaryPath.toFile().text = "#!/bin/sh\n$contents"
+            ccm.localBinaryPath.toFile().setExecutable(true)
+        }
+        ccm.version()
+
+        then:
+        thrown(expected)
+
+        where:
+        contents                                                 | expected
+        null                                                     | ProcessInitException
+        """echo '{"foo": true, "baz": 1}'"""                     | InvalidVersionException
+        """echo '{"version: '"""                                 | JsonSyntaxException
+        "exit 0"                                                 | InvalidVersionException
+        "exit 1"                                                 | InvalidExitValueException
+    }
+
+    @IgnoreIf({ os.windows })
+    def "checks if version matches"() {
+        given:
+        def ccm = new CoderCLIManager(new URL("https://test.coder.version-matches.invalid"), tmpdir)
+        Files.createDirectories(ccm.localBinaryPath.parent)
+
+        when:
+        if (contents != null) {
+          ccm.localBinaryPath.toFile().text = "#!/bin/sh\n$contents"
+          ccm.localBinaryPath.toFile().setExecutable(true)
+        }
+
+        then:
+        ccm.matchesVersion(build) == matches
+
+        where:
+        contents                                          | build                   | matches
+        null                                              | "v1.0.0"                | false
+        """echo '{"version": "v1.0.0"}'"""                | "v1.0.0"                | true
+        """echo '{"version": "v1.0.0"}'"""                | "v1.0.0-devel+b5b5b5b5" | true
+        """echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""" | "v1.0.0-devel+b5b5b5b5" | true
+        """echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""" | "v1.0.0"                | true
+        """echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""" | "v1.0.0-devel+c6c6c6c6" | true
+        """echo '{"version": "v1.0.0-prod+b5b5b5b5"}'"""  | "v1.0.0-devel+b5b5b5b5" | true
+        """echo '{"version": "v1.0.0"}'"""                | "v1.0.1"                | false
+        """echo '{"version": "v1.0.0"}'"""                | "v1.1.0"                | false
+        """echo '{"version": "v1.0.0"}'"""                | "v2.0.0"                | false
+        """echo '{"version": "v1.0.0"}'"""                | "v0.0.0"                | false
+        """echo '{"version": ""}'"""                      | "v1.0.0"                | false
+        """echo '{"version": "v1.0.0"}'"""                | ""                      | false
+        """echo '{"version'"""                            | "v1.0.0"                | false
+        """exit 0"""                                      | "v1.0.0"                | false
+        """exit 1"""                                      | "v1.0.0"                | false
+    }
 }