Cleanups for v1.0 release (#78)

* Handle null component.moduleVersion
* Track project dependencies more correctly
* Update list of tested versions to include 8.3.1
* Improve filtering of unsupported versions from test
* Bump org.junit.jupiter:junit-jupiter from 5.10.0 to 5.10.1

.github/workflows/gradle.yml

@@ -30,10 +30,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        # Test latest version 5.x, 6.x and 7.x, plus oldest patched 7.x
-        # Test all patched minor versions of 8.x
+        # Test latest version 5.x, 6.x and 7.x, as well as all patched minor versions of 8.x
         # Latest 8.x is tested in 'quick-check' job
-        gradle-version: [ "5.6.4", "6.9.4", "7.0.2", "7.6.2", "8.0.2", "8.1.1", "8.2.1"]
+        gradle-version: [ "5.6.4", "6.9.4", "7.6.3", "8.0.2", "8.1.1", "8.2.1", "8.3"]
     runs-on: ubuntu-latest
     env:
       GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}

gradle/libs.versions.toml

@@ -17,7 +17,7 @@ github-packageurl = { group = "com.github.package-url", name = "packageurl-java"
 spock-core = { group = "org.spockframework", name = "spock-core", version.ref = "spock" }
 spock-junit4 = { group = "org.spockframework", name = "spock-junit4", version.ref = "spock" }
 junit-junit4 = { group = "junit", name = "junit", version = "4.13.2" }
-junit-jupiter = { group = "org.junit.jupiter", name = "junit-jupiter", version = "5.10.0" }
+junit-jupiter = { group = "org.junit.jupiter", name = "junit-jupiter", version = "5.10.1" }
 
 groovy-json = { group = "org.codehaus.groovy", name = "groovy-json", version = "3.0.19" }
 json-schema-validator = { group = "com.networknt", name = "json-schema-validator", version = "1.0.87" }

plugin-test/src/test/groovy/org/gradle/github/dependencygraph/SampleProjectDependencyExtractorTest.groovy

@@ -1,10 +1,11 @@
 package org.gradle.github.dependencygraph
 
 import org.apache.commons.io.FileUtils
+import org.gradle.util.GradleVersion
 import spock.lang.IgnoreIf
 
-@IgnoreIf({ System.getProperty("testGradleVersion") == "5.6.4" })
 // Samples aren't designed to run on Gradle 5.x
+@IgnoreIf({ GradleVersion.version(testGradleVersion) < GradleVersion.version("6.0") })
 class SampleProjectDependencyExtractorTest extends BaseExtractorTest {
     def setup() {
         applyDependencyGraphPlugin()
@@ -65,9 +66,8 @@ class SampleProjectDependencyExtractorTest extends BaseExtractorTest {
         ])
     }
 
-    // Temporarily disable test that hangs on Gradle < 7.6
-    // TODO: Re-enable this test
-    @IgnoreIf({ System.getProperty("testGradleVersion") != null })
+    // Test hangs on Gradle < 7 (but cannot reproduce this outside of TestKit)
+    @IgnoreIf({ GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0") })
     def "check java-included-builds sample"() {
         def sampleDir = new File("../sample-projects/java-included-builds")
 

plugin/src/main/kotlin/org/gradle/dependencygraph/extractor/DependencyExtractor.kt

@@ -1,6 +1,7 @@
 package org.gradle.dependencygraph.extractor
 
 import org.gradle.api.GradleException
+import org.gradle.api.artifacts.component.ProjectComponentIdentifier
 import org.gradle.api.artifacts.result.ResolvedComponentResult
 import org.gradle.api.artifacts.result.ResolvedDependencyResult
 import org.gradle.api.internal.artifacts.DefaultProjectComponentIdentifier
@@ -144,8 +145,9 @@ abstract class DependencyExtractor :
         }
         val projectIdentityPath = (rootComponent.id as? DefaultProjectComponentIdentifier)?.identityPath?.path
 
-        // TODO: At this point, any resolution not bound to a particular project will be assigned to the root "build :"
+        // At this point, any resolution not bound to a particular project will be assigned to the root "build :"
         // This is because `details.buildPath` is always ':', which isn't correct in a composite build.
+        // This is inconsequential for GitHub Dependency Graph, since all dependencies are mapped to a single manifest.
         // It is possible to do better. By tracking the current build operation context, we can assign more precisely.
         // See the Gradle Enterprise Build Scan Plugin: `ConfigurationResolutionCapturer_5_0`
         val rootPath = projectIdentityPath ?: details.buildPath
@@ -159,17 +161,17 @@ abstract class DependencyExtractor :
         val rootSource = DependencySource(rootId, rootPath)
         val resolvedConfiguration = ResolvedConfiguration(rootSource, details.configurationName)
 
-        for (directDependency in getResolvedDependencies(rootComponent)) {
+        for (dependencyComponent in getResolvedDependencies(rootComponent)) {
             val directDep = createComponentNode(
-                componentId(directDependency),
+                componentId(dependencyComponent),
                 rootSource,
                 true,
-                directDependency,
+                dependencyComponent,
                 repositoryLookup
             )
             resolvedConfiguration.addDependency(directDep)
 
-            walkComponentDependencies(directDependency, directDep.source, repositoryLookup, resolvedConfiguration)
+            walkComponentDependencies(dependencyComponent, directDep.source, repositoryLookup, resolvedConfiguration)
         }
 
         resolvedConfigurations.add(resolvedConfiguration)
@@ -184,11 +186,11 @@ abstract class DependencyExtractor :
         val componentSource = getSource(component, parentSource)
         val direct = componentSource != parentSource
 
-        val dependencyComponents = getResolvedDependencies(component)
-        for (dependencyComponent in dependencyComponents) {
+        for (dependencyComponent in getResolvedDependencies(component)) {
             val dependencyId = componentId(dependencyComponent)
             if (!resolvedConfiguration.hasDependency(dependencyId)) {
-                val dependencyNode = createComponentNode(dependencyId, componentSource, direct, dependencyComponent, repositoryLookup)
+                val dependencyNode =
+                    createComponentNode(dependencyId, componentSource, direct, dependencyComponent, repositoryLookup)
                 resolvedConfiguration.addDependency(dependencyNode)
 
                 walkComponentDependencies(dependencyComponent, componentSource, repositoryLookup, resolvedConfiguration)
@@ -208,13 +210,15 @@ abstract class DependencyExtractor :
         return component.dependencies.filterIsInstance<ResolvedDependencyResult>().map { it.selected }.filter { it != component }
     }
 
-    private fun createComponentNode(componentId: String, source: DependencySource, direct: Boolean, component: ResolvedComponentResult, repositoryLookup: RepositoryUrlLookup): ResolvedDependency {
+    private fun createComponentNode(componentId: String, source: DependencySource, isDirectDependency: Boolean, component: ResolvedComponentResult, repositoryLookup: RepositoryUrlLookup): ResolvedDependency {
         val componentDependencies = component.dependencies.filterIsInstance<ResolvedDependencyResult>().map { componentId(it.selected) }
         val repositoryUrl = repositoryLookup.doLookup(component)
+        val isProjectDependency = component.id is ProjectComponentIdentifier
         return ResolvedDependency(
             componentId,
             source,
-            direct,
+            isDirectDependency,
+            isProjectDependency,
             coordinates(component),
             repositoryUrl,
             componentDependencies
@@ -226,13 +230,12 @@ abstract class DependencyExtractor :
     }
 
     private fun coordinates(component: ResolvedComponentResult): DependencyCoordinates {
-        // TODO: Consider and handle null moduleVersion
-        val moduleVersionIdentifier = component.moduleVersion!!
-        return DependencyCoordinates(
-            moduleVersionIdentifier.group,
-            moduleVersionIdentifier.name,
-            moduleVersionIdentifier.version
-        )
+        val mv = component.moduleVersion
+        return if (mv != null) {
+            DependencyCoordinates(mv.group, mv.name, mv.version)
+        } else {
+            DependencyCoordinates("unknown", "unknown", "unknown")
+        }
     }
 
     private class RepositoryUrlLookup(

plugin/src/main/kotlin/org/gradle/dependencygraph/model/ResolvedDependency.kt

@@ -7,7 +7,8 @@ private const val DEFAULT_MAVEN_REPOSITORY_URL = "https://repo.maven.apache.org/
 data class ResolvedDependency(
     val id: String,
     val source: DependencySource,
-    val direct: Boolean,
+    val isDirect: Boolean,
+    val isProject: Boolean,
     val coordinates: DependencyCoordinates,
     val repositoryUrl: String?,
     val dependencies: List<String>
@@ -17,7 +18,7 @@ data class ResolvedDependency(
         PackageURLBuilder
             .aPackageURL()
             .withType("maven")
-            .withNamespace(coordinates.group.ifEmpty { coordinates.module }) // TODO: This is a sign of broken mapping from component -> PURL
+            .withNamespace(coordinates.group.ifEmpty { coordinates.module })
             .withName(coordinates.module)
             .withVersion(coordinates.version)
             .also {

plugin/src/main/kotlin/org/gradle/forceresolve/ForceDependencyResolutionPlugin.kt

@@ -7,7 +7,6 @@ import org.gradle.api.invocation.Gradle
 import org.gradle.api.tasks.TaskProvider
 import org.gradle.util.GradleVersion
 
-// TODO: Rename these
 private const val RESOLVE_PROJECT_TASK = "ForceDependencyResolutionPlugin_resolveProjectDependencies"
 private const val RESOLVE_ALL_TASK = "ForceDependencyResolutionPlugin_resolveAllDependencies"
 
@@ -40,10 +39,10 @@ class ForceDependencyResolutionPlugin : Plugin<Gradle> {
     private fun getResolveProjectDependenciesTaskFactory(): ResolveProjectDependenciesTaskFactory {
         val gradleVersion = GradleVersion.current()
         val gradle8 = GradleVersion.version("8.0")
-        if (gradleVersion >= gradle8) {
-            return ResolveProjectDependenciesTaskFactory.Current
+        return if (gradleVersion >= gradle8) {
+            ResolveProjectDependenciesTaskFactory.Current
         } else {
-            return ResolveProjectDependenciesTaskFactory.Legacy
+            ResolveProjectDependenciesTaskFactory.Legacy
         }
     }
 

plugin/src/main/kotlin/org/gradle/github/dependencygraph/GitHubRepositorySnapshotBuilder.kt

@@ -24,7 +24,7 @@ class GitHubRepositorySnapshotBuilder(
         for (resolutionRoot in resolvedConfigurations) {
             for (dependency in resolutionRoot.allDependencies) {
                 // Ignore project dependencies (transitive deps of projects will be reported with project)
-                if (isProject(dependency)) continue
+                if (dependency.isProject) continue
 
                 dependencyCollector.addResolved(dependency)
             }
@@ -51,11 +51,6 @@ class GitHubRepositorySnapshotBuilder(
         }
     }
 
-    // TODO:DAZ Model this better
-    private fun isProject(dependency: ResolvedDependency): Boolean {
-        return dependency.id.startsWith("project ")
-    }
-
     fun buildSnapshot(manifest: GitHubManifest): GitHubRepositorySnapshot {
         return GitHubRepositorySnapshot(
             job = job,
@@ -90,7 +85,7 @@ class GitHubRepositorySnapshotBuilder(
         }
 
         private fun relationship(component: ResolvedDependency) =
-            if (component.direct) GitHubDependency.Relationship.direct else GitHubDependency.Relationship.indirect
+            if (component.isDirect) GitHubDependency.Relationship.direct else GitHubDependency.Relationship.indirect
 
         private class GitHubDependencyBuilder(val package_url: String) {
             var relationship: GitHubDependency.Relationship = GitHubDependency.Relationship.indirect