Skip to content

Commit

Permalink
fix: Reason explanation id ambiguity (#1125)
Browse files Browse the repository at this point in the history
Co-authored-by: sergei <sergei.chernov@adyen.com>
  • Loading branch information
seregamorph and sergei committed Feb 22, 2024
1 parent e218292 commit c08a991
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
package com.autonomousapps.jvm

import com.autonomousapps.AbstractFunctionalSpec
import com.autonomousapps.jvm.projects.BundleKmpProject2
import com.autonomousapps.jvm.projects.NestedSubprojectsProject
import com.autonomousapps.utils.Colors
import org.gradle.testkit.runner.BuildResult
import org.gradle.util.GradleVersion
import spock.lang.PendingFeature

import static com.autonomousapps.utils.Runner.build
import static com.autonomousapps.utils.Runner.buildAndFail
import static com.google.common.truth.Truth.assertThat

final class JvmReasonSpec extends AbstractFunctionalSpec {
Expand Down Expand Up @@ -44,6 +47,36 @@ final class JvmReasonSpec extends AbstractFunctionalSpec {
gradleVersion << gradleVersions()
}
def "reason fails when there is dependency filtering ambiguity"() {
given:
def project = new BundleKmpProject2()
gradleProject = project.gradleProject
when:
def result = buildAndFail(gradleVersion, gradleProject.rootDir, ':consumer:reason', '--id', 'com.squareup.okio:oki')
then:
assertThat(result.output).contains("> Coordinates 'com.squareup.okio:oki' matches more than 1 dependency [com.squareup.okio:okio-jvm:3.0.0, com.squareup.okio:okio:3.0.0]")
where:
gradleVersion << [GradleVersion.current()]
}
def "reason matches startsWith when there is no ambiguity"() {
given:
def project = new BundleKmpProject2()
gradleProject = project.gradleProject
when:
def result = build(gradleVersion, gradleProject.rootDir, ':consumer:reason', '--id', 'com.squareup.okio:okio-')
then:
assertThat(result.output).contains("You asked about the dependency 'com.squareup.okio:okio-jvm:3.0.0'.")
where:
gradleVersion << [GradleVersion.current()]
}
private static void outputMatchesForProject(BuildResult result, String id) {
def lines = Colors.decolorize(result.output).readLines()
def asked = lines.find { it.startsWith('You asked about') }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,16 @@ internal fun <T> String.matchesKey(mapEntry: Map.Entry<String, T>): Boolean {
return false
}

internal fun <T> String.equalsKey(mapEntry: Map.Entry<String, T>) =
mapEntry.key.firstCoordinatesKeySegment() == this || mapEntry.key.secondCoordinatesKeySegment() == this
internal fun <T> String.equalsKey(mapEntry: Map.Entry<String, T>): Boolean {
val tokens = mapEntry.key.firstCoordinatesKeySegment().split(":")
if (tokens.size == 3) {
// "groupId:artifactId:version" => "groupId:artifactId"
if ("${tokens[0]}:${tokens[1]}" == this) {
return true
}
}
return mapEntry.key.firstCoordinatesKeySegment() == this || mapEntry.key.secondCoordinatesKeySegment() == this
}

private fun <T> String.startsWithKey(mapEntry: Map.Entry<String, T>) =
mapEntry.key.firstCoordinatesKeySegment().startsWith(this) || mapEntry.key.secondCoordinatesKeySegment()?.startsWith(this) == true
Expand Down
27 changes: 25 additions & 2 deletions src/main/kotlin/com/autonomousapps/tasks/ReasonTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ abstract class ReasonTask @Inject constructor(
}?.gav()

// Guaranteed to find full GAV or throw
val gavKey = dependencyUsages.entries.find(requestedId::matchesKey)?.key
?: annotationProcessorUsages.entries.find(requestedId::matchesKey)?.key
val gavKey = findFilteredDependencyKey(dependencyUsages.entries, requestedId)
?: findFilteredDependencyKey(annotationProcessorUsages.entries, requestedId)
?: findInGraph()
?: throw InvalidUserDataException("There is no dependency with coordinates '$requestedId' in this project.")

Expand Down Expand Up @@ -253,6 +253,29 @@ abstract class ReasonTask @Inject constructor(
}

private fun wasFiltered(): Boolean = finalAdvice == null && unfilteredAdvice != null

internal companion object {
internal fun findFilteredDependencyKey(dependencies: Set<Map.Entry<String, Any>>, requestedId: String): String? {
val filteredKeys = LinkedHashSet<String>()
for (entry in dependencies) {
if (requestedId.equalsKey(entry)) {
// for exact equal - return immediately
return entry.key
}
if (requestedId.matchesKey(entry)) {
filteredKeys.add(entry.key)
}
}
return if (filteredKeys.isEmpty()) {
null
} else if (filteredKeys.size == 1) {
filteredKeys.iterator().next()
} else {
throw InvalidUserDataException("Coordinates '$requestedId' matches more than 1 dependency " +
"${filteredKeys.map { it.secondCoordinatesKeySegment() ?: it }}")
}
}
}
}

interface ExplainModuleAdviceParams : WorkParameters {
Expand Down
82 changes: 82 additions & 0 deletions src/test/kotlin/com/autonomousapps/tasks/ReasonTaskTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package com.autonomousapps.tasks

import com.autonomousapps.model.declaration.Bucket
import com.autonomousapps.model.declaration.Variant
import com.autonomousapps.model.intermediates.Usage
import com.autonomousapps.tasks.ReasonTask.ExplainDependencyAdviceAction.Companion.findFilteredDependencyKey
import org.gradle.api.InvalidUserDataException
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertThrows
import org.junit.jupiter.api.Test

class ReasonTaskTest {

private val usage = Usage(null, null, Variant.MAIN, Bucket.NONE, emptySet())

private val entries = mapOf(
"demo-gradle-multi-module:list|:list" to usage,
"demo-gradle-multi-module:list:sublist|:list:sublist" to usage,
"demo-gradle-multi-module:list-default|:list-default" to usage,
"demo-gradle-multi-module:list-impl|:list-impl" to usage,
"com.squareup.okio:okio-jvm:3.0.0" to usage,
"com.squareup.okio:okio:3.0.0" to usage
).entries

@Test
fun shouldThrowOnProjectModuleAmbiguity() {
val ex = assertThrows(InvalidUserDataException::class.java) { findFilteredDependencyKey(entries, ":li") }
assertEquals("Coordinates ':li' matches more than 1 dependency " +
"[:list, :list:sublist, :list-default, :list-impl]", ex.message)
}

@Test
fun shouldMatchEqualProjectModule() {
val key = findFilteredDependencyKey(entries, ":list")
assertEquals("demo-gradle-multi-module:list|:list", key)
}

@Test
fun shouldMatchPrefixProjectModuleColon() {
val key = findFilteredDependencyKey(entries, ":list:")
assertEquals("demo-gradle-multi-module:list:sublist|:list:sublist", key)
}

@Test
fun shouldMatchPrefixProjectModule() {
val key = findFilteredDependencyKey(entries, ":list-d")
assertEquals("demo-gradle-multi-module:list-default|:list-default", key)
}

@Test
fun shouldThrowOnLibraryAmbiguity() {
val ex = assertThrows(InvalidUserDataException::class.java) {
findFilteredDependencyKey(entries, "com.squareup.okio:oki")
}
assertEquals("Coordinates 'com.squareup.okio:oki' matches more than 1 dependency " +
"[com.squareup.okio:okio-jvm:3.0.0, com.squareup.okio:okio:3.0.0]", ex.message)
}

@Test
fun shouldMatchEqualLibrary() {
val key = findFilteredDependencyKey(entries, "com.squareup.okio:okio")
assertEquals("com.squareup.okio:okio:3.0.0", key)
}

@Test
fun shouldMatchPrefixLibraryColon() {
val key = findFilteredDependencyKey(entries, "com.squareup.okio:okio:")
assertEquals("com.squareup.okio:okio:3.0.0", key)
}

@Test
fun shouldMatchLibraryVersion() {
val key = findFilteredDependencyKey(entries, "com.squareup.okio:okio:3.0.0")
assertEquals("com.squareup.okio:okio:3.0.0", key)
}

@Test
fun shouldMatchPrefixLibrary() {
val key = findFilteredDependencyKey(entries, "com.squareup.okio:okio-")
assertEquals("com.squareup.okio:okio-jvm:3.0.0", key)
}
}

0 comments on commit c08a991

Please sign in to comment.