Improve test and code quality (#73)

* Improve download URL test
* Introduce detekt plugin
* Address detekt violations

foojay-resolver/build.gradle.kts

@@ -4,6 +4,7 @@ plugins {
     `kotlin-dsl`
     signing
     id("com.gradle.plugin-publish") version "1.2.1"
+    id("io.gitlab.arturbosch.detekt") version "1.23.6"
 }
 
 group = "org.gradle.toolchains"
@@ -16,6 +17,12 @@ java {
     }
 }
 
+detekt {
+    buildUponDefaultConfig = true // preconfigure defaults
+    allRules = true
+    config.setFrom(project.rootProject.file("gradle/detekt.yml"))
+}
+
 dependencies {
     implementation("com.google.code.gson:gson:2.10.1")
 }

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/AbstractFoojayToolchainPlugin.kt

@@ -16,4 +16,4 @@ abstract class AbstractFoojayToolchainPlugin: Plugin<Any> {
 
     abstract fun apply(settings: Settings)
 
-}
+}

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/FoojayApi.kt

@@ -16,32 +16,34 @@ import java.nio.charset.StandardCharsets.UTF_8
 import java.util.concurrent.TimeUnit.SECONDS
 
 
+@Suppress("UnstableApiUsage")
 class FoojayApi {
 
-    val CONNECT_TIMEOUT = SECONDS.toMillis(10).toInt()
-    val READ_TIMEOUT = SECONDS.toMillis(20).toInt()
+    companion object {
+        val CONNECT_TIMEOUT = SECONDS.toMillis(10).toInt()
+        val READ_TIMEOUT = SECONDS.toMillis(20).toInt()
 
-    val SCHEMA = "https"
+        const val SCHEMA = "https"
 
-    val ENDPOINT_ROOT = "api.foojay.io/disco/v3.0"
-    val DISTRIBUTIONS_ENDPOINT = "$ENDPOINT_ROOT/distributions"
-    val PACKAGES_ENDPOINT = "$ENDPOINT_ROOT/packages"
+        private const val ENDPOINT_ROOT = "api.foojay.io/disco/v3.0"
+        const val DISTRIBUTIONS_ENDPOINT = "$ENDPOINT_ROOT/distributions"
+        const val PACKAGES_ENDPOINT = "$ENDPOINT_ROOT/packages"
+    }
 
     val distributions = mutableListOf<Distribution>()
 
     fun toUri(links: Links?): URI? = links?.pkg_download_redirect
 
-    fun toLinks(
+    fun toPackage(
             version: JavaLanguageVersion,
             vendor: JvmVendorSpec,
             implementation: JvmImplementation,
             operatingSystem: OperatingSystem,
             architecture: Architecture
-    ): Links? {
+    ): Package? {
         val distributions = match(vendor, implementation, version)
         return distributions.asSequence().mapNotNull { distribution ->
-            val downloadPackage = match(distribution.api_parameter, version, operatingSystem, architecture)
-            downloadPackage?.links
+            match(distribution.api_parameter, version, operatingSystem, architecture)
         }.firstOrNull()
     }
 

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/FoojayToolchainResolver.kt

@@ -12,14 +12,14 @@ abstract class FoojayToolchainResolver: JavaToolchainResolver {
     override fun resolve(request: JavaToolchainRequest): Optional<JavaToolchainDownload> {
         val spec = request.javaToolchainSpec
         val platform = request.buildPlatform
-        val links = api.toLinks(
+        val links = api.toPackage(
             spec.languageVersion.get(),
             spec.vendor.get(),
             spec.implementation.get(),
             platform.operatingSystem,
             platform.architecture
-        )
+        )?.links
         val uri = api.toUri(links)
         return Optional.ofNullable(uri).map(JavaToolchainDownload::fromUri)
     }
-}
+}

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/FoojayToolchainsConventionPlugin.kt

@@ -20,4 +20,4 @@ abstract class FoojayToolchainsConventionPlugin: AbstractFoojayToolchainPlugin()
         }
     }
 
-}
+}

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/FoojayToolchainsPlugin.kt

@@ -11,6 +11,7 @@ import org.gradle.util.GradleVersion
 @Suppress("unused")
 abstract class FoojayToolchainsPlugin: AbstractFoojayToolchainPlugin() {
 
+    @Suppress("TooGenericExceptionThrown")
     override fun apply(settings: Settings) {
         if (GradleVersion.current().baseVersion < GradleVersion.version("7.6")) {
             throw RuntimeException("${FoojayToolchainsPlugin::class.simpleName} needs Gradle version 7.6 or higher")

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/distributions.kt

@@ -1,3 +1,5 @@
+@file:Suppress("UnstableApiUsage")
+
 package org.gradle.toolchains.foojay
 
 import com.google.gson.Gson
@@ -33,6 +35,7 @@ val j9Aliases = mapOf(
  * Given a list of [distributions], return those that match the provided [vendor] and JVM [implementation]. The Java
  * language [version] is only used to remove wrong GraalVM distributions; no general version filtering is done here.
  */
+@Suppress("ReturnCount")
 fun match(
     distributions: List<Distribution>,
     vendor: JvmVendorSpec,
@@ -104,6 +107,7 @@ fun parseDistributions(json: String): List<Distribution> {
 /**
  * The data class for the result objects as returned by [FoojayApi.DISTRIBUTIONS_ENDPOINT].
  */
+@Suppress("ConstructorParameterNaming")
 data class Distribution(
     /**
      * The distribution (vendor) name, e.g. "Temurin", "Oracle OpenJDK", "JetBrains", "GraalVM", ...

foojay-resolver/src/main/kotlin/org/gradle/toolchains/foojay/packages.kt

@@ -1,3 +1,5 @@
+@file:Suppress("ConstructorParameterNaming")
+
 package org.gradle.toolchains.foojay
 
 import com.google.gson.Gson
@@ -62,4 +64,4 @@ data class Links(
 
 private data class PackagesResult(
     val result: List<Package>
-)
+)

foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt

@@ -10,115 +10,45 @@ import org.gradle.jvm.toolchain.internal.DefaultJvmVendorSpec.any
 import org.gradle.platform.Architecture
 import org.gradle.platform.OperatingSystem
 import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.Arguments
+import org.junit.jupiter.params.provider.MethodSource
 import org.junit.jupiter.params.provider.ValueSource
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
+@Suppress("UnstableApiUsage")
 class FoojayApiTest {
 
     private val api = FoojayApi()
 
-    @Test
-    fun `download URI provided correctly`() {
-        assertDownloadUri(
-            javaVersion = 8,
-            vendor = any(),
-            isJ9 = false,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.AARCH64
-        ) // zulu8.X.X.X-ca-fx-jdk8.X.XXX-macosx_aarch64.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 11,
-            vendor = ADOPTIUM,
-            isJ9 = false,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.AARCH64
-        ) // OpenJDK11U-jdk_aarch64_mac_hotspot_11.X.XX.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 11,
-            vendor = GRAAL_VM,
-            isJ9 = false,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.AARCH64
-        ) // graalvm-ce-java11-darwin-aarch64-22.X.X.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = any(),
-            isJ9 = true,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.X86_64
-        ) // ibm-semeru-open-jdk_x64_mac_16.X.X_openj9-0.27.0.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = IBM,
-            isJ9 = true,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.X86_64
-        ) // ibm-semeru-open-jdk_x64_mac_16.X.X_openj9-0.27.0.tar.gz
-
+    @ParameterizedTest(name = "javaVersion: {0}, vendor: {1}, isJ9: {2}, os: {3}, arch: {4}")
+    @MethodSource("getData")
+    fun `download URI provided correctly`(
+        javaVersion: Int,
+        vendor: JvmVendorSpec,
+        isJ9: Boolean,
+        os: OperatingSystem,
+        arch: Architecture
+    ) = assertDownloadUri(javaVersion, vendor, isJ9, os, arch)
+
+    companion object {
         @Suppress("DEPRECATION")
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = IBM_SEMERU,
-            isJ9 = true,
-            os = OperatingSystem.MAC_OS,
-            arch = Architecture.X86_64
-        ) // ibm-semeru-open-jdk_x64_mac_16.X.X_openj9-0.27.0.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = GRAAL_VM,
-            isJ9 = false,
-            os = OperatingSystem.LINUX,
-            arch = Architecture.X86_64
-        ) // graalvm-ce-java16-linux-amd64-21.X.X.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = any(),
-            isJ9 = false,
-            os = OperatingSystem.LINUX,
-            arch = Architecture.X86_64
-        ) // OpenJDK16U-jdk_x64_linux_hotspot_16.X.X.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 16,
-            vendor = any(),
-            isJ9 = true,
-            os = OperatingSystem.LINUX,
-            arch = Architecture.X86_64
-        ) // ibm-semeru-open-jdk_x64_linux_16.X.X_openj9-0.27.0.tar.gz
-
-        assertDownloadUri(
-            javaVersion = 8,
-            vendor = GRAAL_VM,
-            isJ9 = false,
-            os = OperatingSystem.WINDOWS,
-            arch = Architecture.X86_64
-        ) // graalvm-ce-java8-windows-amd64-21.X.X.zip
-
-        assertDownloadUri(
-            javaVersion = 20,
-            vendor = GRAAL_VM,
-            isJ9 = false,
-            os = OperatingSystem.LINUX,
-            arch = Architecture.X86_64
-        ) // graalvm-community-jdk-20.0.1_linux-x64_bin.tar.gz
-
-        assertDownloadUri(
-            expected = Regex(""),
-            javaVersion = 1000,
-            vendor = any(),
-            isJ9 = false,
-            os = OperatingSystem.LINUX,
-            arch = Architecture.X86_64
-        ) // No match because Java 1000 is not beeing released soon
+        @JvmStatic
+        fun getData(): List<Arguments> = listOf(
+          Arguments.of(8, any(), false, OperatingSystem.MAC_OS, Architecture.AARCH64),
+          Arguments.of(11, ADOPTIUM, false, OperatingSystem.MAC_OS, Architecture.AARCH64),
+          Arguments.of(11, GRAAL_VM, false, OperatingSystem.MAC_OS, Architecture.AARCH64),
+          Arguments.of(16, any(), true, OperatingSystem.MAC_OS, Architecture.X86_64),
+          Arguments.of(16, IBM, true, OperatingSystem.MAC_OS, Architecture.X86_64),
+          Arguments.of(16, IBM_SEMERU, true, OperatingSystem.MAC_OS, Architecture.X86_64),
+          Arguments.of(16, GRAAL_VM, false, OperatingSystem.LINUX, Architecture.X86_64),
+          Arguments.of(16, any(), false, OperatingSystem.LINUX, Architecture.X86_64),
+          Arguments.of(16, any(), true, OperatingSystem.LINUX, Architecture.X86_64),
+          Arguments.of(8, GRAAL_VM, false, OperatingSystem.WINDOWS, Architecture.X86_64),
+          Arguments.of(20, GRAAL_VM, false, OperatingSystem.LINUX, Architecture.X86_64),
+        )
     }
 
     @ParameterizedTest(name = "J9 implementation influences vendor resolution (Java {0})")
@@ -173,7 +103,12 @@ class FoojayApiTest {
         assertMatchedDistributions(HEWLETT_PACKARD, VENDOR_SPECIFIC, version)
     }
 
-    private fun assertMatchedDistributions(vendor: JvmVendorSpec, implementation: JvmImplementation, version: Int, vararg expectedDistributions: String) {
+    private fun assertMatchedDistributions(
+        vendor: JvmVendorSpec,
+        implementation: JvmImplementation,
+        version: Int,
+        vararg expectedDistributions: String
+    ) {
         assertEquals(
                 listOf(*expectedDistributions),
                 api.match(vendor, implementation, of(version)).map { it.name },
@@ -202,25 +137,63 @@ class FoojayApiTest {
         assertEquals("jdk", p.package_type)
     }
 
+    @Suppress("LongParameterList")
     private fun assertDownloadUri(
-            expected: Regex = Regex("""https://api.foojay.io/disco/v3.0/ids/[a-z0-9]{32}/redirect"""),
             javaVersion: Int,
             vendor: JvmVendorSpec,
             isJ9: Boolean,
             os: OperatingSystem,
             arch: Architecture
     ) {
-        val links = api.toLinks(
-                of(javaVersion),
-                vendor,
-                if (isJ9) J9 else VENDOR_SPECIFIC,
-                os,
-                arch
+        val actual = api.toPackage(of(javaVersion), vendor, if (isJ9) J9 else VENDOR_SPECIFIC, os, arch)
+        assertNotNull(actual)
+        assertNotNull(actual.links.pkg_download_redirect)
+        assertJavaVersion(javaVersion, actual)
+        assertDistribution(vendor, actual)
+        assertOperatingSystem(os, actual)
+        assertArchitecture(arch, actual)
+    }
+
+    private fun assertJavaVersion(javaVersion: Int, actual: Package) {
+        val actualValue = actual.jdk_version
+        assertEquals(javaVersion, actualValue,
+            "Expected Java version ($javaVersion) doesn't match actual one ($actualValue),  ${moreDetailsAt(actual)}"
+        )
+    }
+
+    private fun assertDistribution(vendor: JvmVendorSpec, actual: Package) {
+        var expectedValue = vendor.toString().replace("_", "").lowercase()
+        if (expectedValue == "ibm") {
+            expectedValue = "semeru"
+        }
+        val actualValue = actual.distribution
+        assertTrue(vendor.matches(actualValue) || actualValue.startsWith(expectedValue),
+            "Expected vendor spec ($expectedValue) doesn't match actual distribution (${actualValue}), ${moreDetailsAt(actual)}"
+        )
+    }
+
+    private fun assertOperatingSystem(os: OperatingSystem, actual: Package) {
+        val expectedValue = os.toString().replace("_", "").lowercase()
+        val actualValue = actual.operating_system
+        assertEquals(expectedValue, actualValue,
+            "Expected operating system ($expectedValue) doesn't match actual one ($actualValue),  ${moreDetailsAt(actual)}"
         )
-        val uriString = api.toUri(links)?.toString() ?: ""
-        assertTrue(expected.matches(uriString), "Expected URI differs from actual, for details see ${links?.pkg_info_uri}")
     }
 
+    private fun assertArchitecture(arch: Architecture, actual: Package) {
+        val expectedValues = when (arch) {
+            Architecture.X86 -> setOf("x32", "i386", "x86")
+            Architecture.X86_64 -> setOf("x64", "x86_64", "amd64", "ia64")
+            Architecture.AARCH64 -> setOf("aarch64", "arm64")
+        }
+        val actualValue = actual.architecture
+        assertTrue(expectedValues.contains(actualValue),
+            "Expected architecture (${arch}) doesn't match actual one ($actualValue),  ${moreDetailsAt(actual)}"
+        )
+    }
+
+    private fun moreDetailsAt(actual: Package?) = "for more details see ${actual?.links?.pkg_info_uri}"
+
     private fun vendorSpec(vendorName: String): JvmVendorSpec = matching(vendorName)
 
 }

gradle/detekt.yml

@@ -0,0 +1,5 @@
+style:
+  MaxLineLength:
+    maxLineLength: 180                    # default is 120
+  WildcardImport:
+    active: false