fix: url matching algorithm (#140)

* fix: revert to 1.1.0 url matching algorithm
russellbanks · Mar 21, 2023 · f62c2fa · f62c2fa
1 parent 0e261b2
commit f62c2fa
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 44 deletions.
diff --git a/src/main/kotlin/commands/QuickUpdate.kt b/src/main/kotlin/commands/QuickUpdate.kt
@@ -205,8 +205,10 @@ class QuickUpdate : CliktCommand(name = "update") {
                     releaseDate = gitHubDetection?.releaseDate ?: downloadedFile.lastModified
                 )
             } finally {
-                downloadedFile.file.delete()
-                Runtime.getRuntime().removeShutdownHook(downloadedFile.fileDeletionThread)
+                with(downloadedFile) {
+                    file.delete()
+                    removeFileDeletionHook()
+                }
             }
         }
         progressList.forEach(ProgressAnimation::clear)

diff --git a/src/main/kotlin/data/shared/PackageIdentifier.kt b/src/main/kotlin/data/shared/PackageIdentifier.kt
@@ -6,13 +6,9 @@ import commands.interfaces.ValidationRules
 object PackageIdentifier : TextPrompt {
     override val name: String = "Package Identifier"
 
-    const val maxLength = 128
-
-    const val minLength = 4
-
     override val validationRules: ValidationRules = ValidationRules(
-        maxLength = maxLength,
-        minLength = minLength,
+        maxLength = 128,
+        minLength = 4,
         pattern = Regex("^[^.\\s\\\\/:*?\"<>|\\x01-\\x1f]{1,32}(\\.[^.\\s\\\\/:*?\"<>|\\x01-\\x1f]{1,32}){1,7}$"),
         isRequired = true
     )

diff --git a/src/main/kotlin/data/shared/PackageVersion.kt b/src/main/kotlin/data/shared/PackageVersion.kt
@@ -5,16 +5,12 @@ import commands.interfaces.ValidationRules
 import java.math.BigInteger
 
 object PackageVersion : TextPrompt {
-    private const val pattern = "^[^\\\\/:*?\"<>|\\x01-\\x1f]+$"
-    const val maxLength = 128
-    val regex = Regex(pattern)
-
     override val name: String = "Package Version"
 
     override val validationRules: ValidationRules = ValidationRules(
         maxLength = 128,
         minLength = 1,
-        pattern = Regex(pattern),
+        pattern = Regex("^[^\\\\/:*?\"<>|\\x01-\\x1f]+$"),
         isRequired = true
     )
 

diff --git a/src/main/kotlin/data/shared/Url.kt b/src/main/kotlin/data/shared/Url.kt
@@ -102,6 +102,7 @@ object Url {
             }
             val progress = getDownloadProgressBar(installerUrl).apply(ProgressAnimation::start)
             val downloadedFile = client.downloadFile(installerUrl, packageIdentifier, packageVersion, progress)
+            progress.clear()
             val fileAnalyser = FileAnalyser(downloadedFile.file)
             installerType = fileAnalyser.getInstallerType()
             architecture = installerUrl.findArchitecture() ?: fileAnalyser.getArchitecture()
@@ -119,8 +120,10 @@ object Url {
                     terminal = this@downloadInstaller
                 )
             }
-            downloadedFile.file.delete()
-            Runtime.getRuntime().removeShutdownHook(downloadedFile.fileDeletionThread)
+            with(downloadedFile) {
+                file.delete()
+                removeFileDeletionHook()
+            }
         }
     }
 

diff --git a/src/main/kotlin/detection/ParameterUrls.kt b/src/main/kotlin/detection/ParameterUrls.kt
@@ -43,21 +43,45 @@ object ParameterUrls {
         newInstallers: List<InstallerManifest.Installer>,
         previousInstallers: List<InstallerManifest.Installer>
     ): Map<InstallerManifest.Installer, InstallerManifest.Installer> {
-        return previousInstallers.associateWith { previousInstaller ->
-            newInstallers.firstOrNull {
-                it.architecture == previousInstaller.architecture &&
-                    it.installerType == previousInstaller.installerType &&
-                    (it.scope == previousInstaller.scope || it.scope == null)
-            } ?: newInstallers.firstOrNull {
+        val result = hashMapOf<InstallerManifest.Installer, InstallerManifest.Installer>()
+        for (previousInstaller in previousInstallers) {
+            var newInstaller: InstallerManifest.Installer? = newInstallers.firstOrNull {
                 it.architecture == previousInstaller.architecture &&
+                        it.installerType == previousInstaller.installerType &&
+                        it.scope == previousInstaller.scope
+            }
+            if (newInstaller == null) {
+                newInstaller = newInstallers.firstOrNull {
+                    it.architecture == previousInstaller.architecture &&
+                            it.installerType == previousInstaller.installerType &&
+                            it.scope == null
+                }
+            }
+            if (newInstaller == null) {
+                newInstaller = newInstallers.firstOrNull {
+                    it.architecture == previousInstaller.architecture &&
+                            it.installerType == previousInstaller.installerType
+                }
+            }
+            if (newInstaller == null) {
+                newInstaller = newInstallers.firstOrNull {
                     it.installerType == previousInstaller.installerType
-            } ?: newInstallers.firstOrNull {
-                it.installerType == previousInstaller.installerType
-            } ?: newInstallers.firstOrNull {
-                it.architecture == previousInstaller.architecture
-            } ?: newInstallers.firstOrNull {
-                it.installerUrl.getExtension() == previousInstaller.installerUrl.getExtension()
-            } ?: previousInstaller // If no match was found, use the previous installer itself
+                }
+            }
+            if (newInstaller == null) {
+                newInstaller = newInstallers.firstOrNull {
+                    it.architecture == previousInstaller.architecture
+                }
+            }
+            if (newInstaller == null) {
+                newInstaller = newInstallers.firstOrNull {
+                    it.installerUrl.getExtension() == previousInstaller.installerUrl.getExtension()
+                }
+            }
+            if (newInstaller != null) {
+                result[previousInstaller] = newInstaller
+            }
         }
+        return result
     }
 }
diff --git a/src/main/kotlin/network/HttpUtils.kt b/src/main/kotlin/network/HttpUtils.kt
@@ -42,7 +42,11 @@ object HttpUtils {
         return DownloadedFile(file, lastModified, fileDeletionThread)
     }
 
-    data class DownloadedFile(val file: File, val lastModified: LocalDate?, val fileDeletionThread: Thread)
+    data class DownloadedFile(val file: File, val lastModified: LocalDate?, val fileDeletionHook: Thread) {
+        fun removeFileDeletionHook(): Boolean {
+            return Runtime.getRuntime().removeShutdownHook(fileDeletionHook)
+        }
+    }
 
     fun Terminal.getDownloadProgressBar(url: Url): ProgressAnimation {
         return progressAnimation {

diff --git a/src/main/kotlin/network/KtorGitHubConnector.kt b/src/main/kotlin/network/KtorGitHubConnector.kt
@@ -10,6 +10,7 @@ import io.ktor.http.ContentType
 import io.ktor.http.HttpMethod
 import io.ktor.http.contentType
 import io.ktor.util.toMap
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.runBlocking
 import org.kohsuke.github.connector.GitHubConnector
 import org.kohsuke.github.connector.GitHubConnectorRequest
@@ -18,7 +19,7 @@ import org.kohsuke.github.connector.GitHubConnectorResponse.ByteArrayResponse
 import java.io.InputStream
 
 class KtorGitHubConnector(private val client: HttpClient) : GitHubConnector {
-    override fun send(connectorRequest: GitHubConnectorRequest): GitHubConnectorResponse = runBlocking {
+    override fun send(connectorRequest: GitHubConnectorRequest): GitHubConnectorResponse = runBlocking(Dispatchers.IO) {
         val request = client.request(connectorRequest.url()) {
             method = HttpMethod.parse(connectorRequest.method())
             if (connectorRequest.hasBody()) {
@@ -38,7 +39,7 @@ class KtorGitHubConnector(private val client: HttpClient) : GitHubConnector {
         request: GitHubConnectorRequest,
         private val response: HttpResponse
     ) : ByteArrayResponse(request, response.status.value, response.headers.toMap()) {
-        override fun rawBodyStream(): InputStream? = runBlocking {
+        override fun rawBodyStream(): InputStream? = runBlocking(Dispatchers.IO) {
             return@runBlocking response.body()
         }
     }

diff --git a/src/test/kotlin/IdentifierTests.kt b/src/test/kotlin/IdentifierTests.kt
@@ -1,35 +1,38 @@
 import data.shared.PackageIdentifier
-import data.shared.PackageIdentifier.getError
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 
 class IdentifierTests : FunSpec({
     context("package identifier validity tests") {
         test("valid identifier is successful") {
-            getError("Package.Identifier") shouldBe null
+            PackageIdentifier.getError("Package.Identifier") shouldBe null
         }
 
         test("identifier greater than max segments fails") {
-            getError(
+            PackageIdentifier.getError(
                 mutableListOf<String>().apply { repeat(9) { add("A") } }.joinToString(".")
             ) shouldNotBe null
         }
 
         test("identifier greater than max length fails") {
-            getError("A".repeat(PackageIdentifier.maxLength.inc())) shouldNotBe null
+            PackageIdentifier.getError(
+                "A".repeat(PackageIdentifier.validationRules.maxLength?.inc() as Int)
+            ) shouldNotBe null
         }
 
         test("identifier shorter than min length fails") {
-            getError("A".repeat(PackageIdentifier.minLength.dec())) shouldNotBe null
+            PackageIdentifier.getError(
+                "A".repeat(PackageIdentifier.validationRules.minLength?.dec() as Int)
+            ) shouldNotBe null
         }
 
         test("blank or empty string fails") {
-            getError(" ".repeat(10)) shouldNotBe null
+            PackageIdentifier.getError(" ".repeat(10)) shouldNotBe null
         }
 
         test("identifier with only one segments fails") {
-            getError("Identifier") shouldNotBe null
+            PackageIdentifier.getError("Identifier") shouldNotBe null
         }
     }
 })
diff --git a/src/test/kotlin/VersionTests.kt b/src/test/kotlin/VersionTests.kt
@@ -1,5 +1,5 @@
+
 import data.shared.PackageVersion
-import data.shared.PackageVersion.getError
 import data.shared.PackageVersion.getHighestVersion
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.datatest.withData
@@ -53,7 +53,7 @@ class VersionTests : FunSpec({
         }
 
         test("largest version allowed") {
-            val maxVersion = "9".repeat(PackageVersion.maxLength)
+            val maxVersion = "9".repeat(PackageVersion.validationRules.maxLength as Int)
             listOf(maxVersion).getHighestVersion() shouldBe maxVersion
         }
 
@@ -82,19 +82,19 @@ class VersionTests : FunSpec({
             "14.0.1",
             "1.10"
         ) {
-            getError(it) shouldBe null
+            PackageVersion.getError(it) shouldBe null
         }
 
         test("version greater than max length fails") {
-            getError("A".repeat(PackageVersion.maxLength.inc())) shouldNotBe null
+            PackageVersion.getError("A".repeat(PackageVersion.validationRules.maxLength?.inc() as Int)) shouldNotBe null
         }
 
         test("blank or empty string fails") {
-            getError(" ".repeat(10)) shouldNotBe null
+            PackageVersion.getError(" ".repeat(10)) shouldNotBe null
         }
 
         test("invalid version pattern fails") {
-            getError("£$%^&*()") shouldNotBe null
+            PackageVersion.getError("£$%^&*()") shouldNotBe null
         }
     }
 })
diff --git a/src/test/kotlin/utils/GitHubUtilsTests.kt b/src/test/kotlin/utils/GitHubUtilsTests.kt
@@ -14,6 +14,7 @@ import kotlinx.coroutines.delay
 import org.kohsuke.github.GHRepository
 import org.kohsuke.github.GHTree
 import org.kohsuke.github.GHTreeEntry
+import java.io.IOException
 
 class GitHubUtilsTests : FunSpec({
     context("get all versions tests") {
@@ -87,6 +88,11 @@ class GitHubUtilsTests : FunSpec({
             ).mapToMockedGHTreeEntry()
             GitHubUtils.getAllVersions(repository, packageIdentifier) shouldBe listOf("1.2.3")
         }
+
+        test("IOException thrown") {
+            every { repository.getTreeRecursive(any(), 1) } throws IOException()
+            GitHubUtils.getAllVersions(repository, packageIdentifier) shouldBe null
+        }
     }
 
     context("get manifest names") {