From 72453d8dad21f68798bc69d0c34167299562fe74 Mon Sep 17 00:00:00 2001
From: Philip Wedemann <22521688+hfhbd@users.noreply.github.com>
Date: Fri, 29 Mar 2024 14:58:04 +0100
Subject: [PATCH] Fail if invalid spdx is passed to allow (#314)

* Fail when allow() contains an SPDX ID

* Remove provider

* Remove unused object factory

---------

Co-authored-by: hfhbd <hfhbd@users.noreply.github.com>
---
 .../app/cash/licensee/pluginExtension.kt      | 37 ++++++++++++++++---
 .../allow-with-invalid-spdx/build.gradle      |  8 ++++
 .../allow-with-invalid-spdx/settings.gradle   |  7 ++++
 .../licensee/LicenseePluginFixtureTest.kt     | 10 +++++
 4 files changed, 56 insertions(+), 6 deletions(-)
 create mode 100644 src/test/fixtures/allow-with-invalid-spdx/build.gradle
 create mode 100644 src/test/fixtures/allow-with-invalid-spdx/settings.gradle

diff --git a/src/main/kotlin/app/cash/licensee/pluginExtension.kt b/src/main/kotlin/app/cash/licensee/pluginExtension.kt
index 3d5f2330..622f87d1 100644
--- a/src/main/kotlin/app/cash/licensee/pluginExtension.kt
+++ b/src/main/kotlin/app/cash/licensee/pluginExtension.kt
@@ -18,14 +18,16 @@ package app.cash.licensee
 import app.cash.licensee.LicenseeExtension.AllowDependencyOptions
 import app.cash.licensee.LicenseeExtension.IgnoreDependencyOptions
 import java.util.Optional
+import javax.inject.Inject
 import org.gradle.api.Action
 import org.gradle.api.Named
+import org.gradle.api.NamedDomainObjectCollection
 import org.gradle.api.NamedDomainObjectContainer
 import org.gradle.api.artifacts.Dependency
 import org.gradle.api.provider.MapProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
-import org.gradle.api.provider.SetProperty
+import org.gradle.api.provider.ProviderFactory
 
 @Suppress("unused") // Public API for Gradle build scripts.
 interface LicenseeExtension {
@@ -245,8 +247,19 @@ internal abstract class IgnoredCoordinate : Named {
   abstract val ignoredDatas: MapProperty<String, IgnoredData>
 }
 
-internal abstract class MutableLicenseeExtension : LicenseeExtension {
-  internal abstract val allowedIdentifiers: SetProperty<String>
+internal abstract class SpdxId(private val spdxId: String) : Named {
+  override fun getName(): String = spdxId
+  init {
+    requireNotNull(SpdxLicenses.embedded.findByIdentifier(spdxId)) {
+      "$name is not a valid SPDX id."
+    }
+  }
+}
+
+internal abstract class MutableLicenseeExtension @Inject constructor(
+  private val providers: ProviderFactory,
+) : LicenseeExtension {
+  internal abstract val allowedIdentifiers: NamedDomainObjectContainer<SpdxId>
   internal abstract val allowedUrls: MapProperty<String, Optional<String>>
   internal abstract val allowedDependencies: MapProperty<DependencyCoordinates, Optional<String>>
   internal abstract val ignoredGroupIds: MapProperty<String, IgnoredData>
@@ -273,9 +286,11 @@ internal abstract class MutableLicenseeExtension : LicenseeExtension {
   }
 
   fun toLicenseValidationConfig(): Provider<ValidationConfig> {
-    return allowedIdentifiers.zip(allowedUrls, allowedDependencies) { allowedIdentifiers, allowedUrls, allowedDependencies ->
+    return allowedIdentifiers.elements(providers) {
+      it.name
+    }.zip(allowedUrls, allowedDependencies) { allowedIdentifiers, allowedUrls, allowedDependencies ->
       ValidationConfig(
-        allowedIdentifiers.toSet(),
+        allowedIdentifiers,
         allowedUrls.mapValues {
           it.value.orElse(null)
         },
@@ -287,7 +302,7 @@ internal abstract class MutableLicenseeExtension : LicenseeExtension {
   }
 
   override fun allow(spdxId: String) {
-    allowedIdentifiers.add(spdxId)
+    allowedIdentifiers.register(spdxId)
   }
 
   override fun allowUrl(url: String, options: Action<LicenseeExtension.AllowUrlOptions>) {
@@ -397,3 +412,13 @@ private fun <T> NamedDomainObjectContainer<T>.configure(name: String, config: Ac
     register(name, config)
   }
 }
+
+// https://github.com/gradle/gradle/issues/28043
+inline fun <reified T : Any, R> NamedDomainObjectCollection<T>.elements(
+  providers: ProviderFactory,
+  crossinline transform: (T) -> R,
+): Provider<out Set<R>> {
+  return providers.provider {
+    mapTo(mutableSetOf(), transform)
+  }
+}
diff --git a/src/test/fixtures/allow-with-invalid-spdx/build.gradle b/src/test/fixtures/allow-with-invalid-spdx/build.gradle
new file mode 100644
index 00000000..bc50465f
--- /dev/null
+++ b/src/test/fixtures/allow-with-invalid-spdx/build.gradle
@@ -0,0 +1,8 @@
+plugins {
+  id("java-library")
+  alias(libs.plugins.licensee)
+}
+
+licensee {
+  allow("ASDF")
+}
diff --git a/src/test/fixtures/allow-with-invalid-spdx/settings.gradle b/src/test/fixtures/allow-with-invalid-spdx/settings.gradle
new file mode 100644
index 00000000..379dbf56
--- /dev/null
+++ b/src/test/fixtures/allow-with-invalid-spdx/settings.gradle
@@ -0,0 +1,7 @@
+pluginManagement {
+  includeBuild("../../test-build-logic")
+}
+
+plugins {
+  id("licenseeTests")
+}
diff --git a/src/test/kotlin/app/cash/licensee/LicenseePluginFixtureTest.kt b/src/test/kotlin/app/cash/licensee/LicenseePluginFixtureTest.kt
index df399c92..abdca42e 100644
--- a/src/test/kotlin/app/cash/licensee/LicenseePluginFixtureTest.kt
+++ b/src/test/kotlin/app/cash/licensee/LicenseePluginFixtureTest.kt
@@ -205,6 +205,16 @@ class LicenseePluginFixtureTest {
     )
   }
 
+  @Test fun invalidSpdxFails(
+    @TestParameter("allow-with-invalid-spdx") fixtureName: String,
+  ) {
+    val fixtureDir = File(fixturesDir, fixtureName)
+    val result = createRunner(fixtureDir).buildAndFail()
+    assertThat(result.output).contains(
+      "ASDF is not a valid SPDX id.",
+    )
+  }
+
   @Test fun pluginMissingOnRootFails(
     @TestParameter("plugin-missing-on-root-fails") fixtureName: String,
   ) {