codeconsole commented on code in PR #16528:
URL: https://github.com/apache/grails-core/pull/16528#discussion_r4189892804
##########
grails-forge/grails-forge-core/src/main/java/org/grails/forge/feature/asciidoctor/template/asciidocGradle.rocker.raw:
##########
@@ -45,3 +45,9 @@ asciidoctorj {
"revnumber": "$project.version"
}
+// The Asciidoctor Gradle plugin does not support the configuration cache, so
a build that runs
+// its tasks runs without the cache instead of failing
+tasks.withType(org.asciidoctor.gradle.jvm.AsciidoctorTask).configureEach {
Review Comment:
This breaks every app generated with the `asciidoctor` feature, with or
without the configuration cache. `gradle/asciidoc.gradle` is applied with
`apply from:`, and a script plugin is compiled against the project's base class
loader scope, which doesn't include the classes of the build script's `plugins
{}` block. `org.asciidoctor.gradle.jvm.AsciidoctorTask` doesn't resolve, Groovy
reads it as a property chain, and configuration fails:
```
A problem occurred evaluating script.
> Could not get unknown property 'org' for root project 'ad' of type
org.gradle.api.Project.
```
I reproduced it on Gradle 9.8.0 with `id "org.asciidoctor.jvm.convert"
version "4.0.5"` in `plugins {}` and these three lines in an applied script.
Referring to the task by name avoids the type. With that change `help` passes,
and `asciidoctor` runs with the entry discarded instead of failing:
```suggestion
tasks.named('asciidoctor') {
```
`AsciidoctorSpec` only checks the template text, so it can't catch this. A
spec that generates an app with `asciidoctor` and runs `./gradlew asciidoctor`
would. The guide's "Plugins Without Configuration Cache Support" section points
at these apps as its example. Its own snippet is fine, though, because it goes
in `build.gradle`, where the type is visible.
##########
gradle.properties:
##########
@@ -70,17 +70,17 @@ codenarcVersion=4.0.0
jacocoVersion=0.8.15
pmdVersion=7.27.0
spotbugsPluginVersion=6.4.8
-sonatypeScanPluginVersion=3.1.6
snakeyamlVersion=2.6
# This prevents the Grails Gradle Plugin from unnecessarily excluding
slf4j-simple in the generated POMs
# https://github.com/apache/grails-gradle-plugin/issues/222
slf4jPreventExclusion=true
org.gradle.caching=true
-# Configuration cache is explicitly disabled until known issues are resolved.
-# See: https://github.com/apache/grails-core/issues/15497
-org.gradle.configuration-cache=false
+# A build that runs the same tasks again skips configuration and reuses the
stored task graph.
+# Build logic must stay compatible: task actions may not reach the project,
other tasks or script
+# state, see the gradle-developer skill.
+org.gradle.configuration-cache=true
Review Comment:
With this on, `./gradlew publishAllToMavenLocal` fails. Its `doLast`
(`build.gradle:190`) reads `layout.projectDirectory`. `Task` has no `layout`,
so Groovy resolves it on the build script, which the configuration cache
doesn't allow while a task runs:
```
> Invocation of 'layout' references a Gradle script object from a Groovy
closure at execution time, which is unsupported with the configuration cache.
```
I reproduced it with the task's own code on Gradle 9.8.0. Reading the
directory while the task is configured, and logging through the task, fixes it,
and a second run reuses the entry:
```groovy
def execSupport = objects.newInstance(ExecSupport)
File projectRoot = layout.projectDirectory.asFile
doLast { Task task ->
def javaHome = System.getProperty('java.home')
def gradlew = Os.isFamily(Os.FAMILY_WINDOWS) ? 'gradlew.bat' : 'gradlew'
def runPublish = { File workingDir, String label ->
task.logger.lifecycle("--- Publishing ${label} to Maven Local ---")
// ...
```
Both the gradle-developer and grails-8-upgrade skills tell people to run
this task.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -4718,3 +4718,17 @@ for template renders:
Objects passed with `bean` or `collection` are not part of `model`. See
link:theWebLayer.html#definingInterceptors[Defining Interceptors].
+
+==== 85. Gradle Configuration Cache
Review Comment:
This is the first 8.1-only note, and it lands in "Upgrading from Grails 7 to
Grails 8". That causes two problems:
- 8.0.x is still adding notes to this page (84 is the latest there), so the
next one there becomes a second number 85 when 8.0.x is merged forward.
- Someone upgrading from 7 to 8.0.x reads this page too. "A project upgraded
from Grails 7, or created with Grails 8.0, can enable it by adding the same
property" has them turn the cache on with the 8.0 plugins, which don't support
it.
Could this go in a new `upgrading81x.adoc` ("Upgrading from Grails 8.0 to
Grails 8.1"), as 7.1 and 7.2 have, and say that it needs the 8.1 Grails Gradle
plugins and BOM? The new line in `grails-8-upgrade/SKILL.md` has the same "or
one created with Grails 8.0, can add the same property" wording.
##########
build-logic/docs-core/src/main/groovy/grails/doc/git/FetchTagsTask.groovy:
##########
@@ -27,52 +27,73 @@ import javax.inject.Inject
import groovy.transform.CompileStatic
+import org.gradle.api.DefaultTask
import org.gradle.api.Project
+import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.Property
import org.gradle.api.tasks.CacheableTask
-import org.gradle.api.tasks.Exec
import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.OutputFile
+import org.gradle.api.tasks.TaskAction
+import org.gradle.process.ExecOperations
+import org.gradle.process.ExecSpec
+/**
+ * Writes the tags of the repository, newest first, to {@link #getTagsFile()};
outside a git
+ * repository it writes {@link #getDefaultTag()} instead.
+ */
@CompileStatic
@CacheableTask
-abstract class FetchTagsTask extends Exec {
+abstract class FetchTagsTask extends DefaultTask {
@Input
final Property<LocalDateTime> cacheDate // allows for forcing refreshing
the tags, defaults to once a day
@Input
final Property<String> defaultTag // if no git repo present
+ @Input
+ final Property<Boolean> gitRepository
+
+ @Internal
+ final DirectoryProperty repositoryDir
+
@OutputFile
final RegularFileProperty tagsFile
+ @Inject
+ abstract ExecOperations getExecOperations()
+
@Inject
FetchTagsTask(ObjectFactory objectFactory, Project project) {
group = 'documentation'
cacheDate =
objectFactory.property(LocalDateTime).convention(LocalDate.now().atStartOfDay())
Review Comment:
With the configuration cache on, `LocalDate.now()` is evaluated once, when
the entry is stored. An entry stored on Monday and reused on Thursday keeps
Monday's `cacheDate`, so `fetchTags` stays up to date and tags pushed since
then never reach the version dropdown. A `ValueSource` that returns today's
date is re-checked when an entry is reused, so it would keep the once-a-day
refresh. `git tag -l` is cheap, so making the task `@UntrackedTask` would work
too.
##########
build-logic/docs-core/src/main/groovy/grails/doc/gradle/ConvertAsciidocTask.groovy:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.doc.gradle
+
+import javax.inject.Inject
+
+import groovy.transform.CompileStatic
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.Project
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.file.FileVisitDetails
+import org.gradle.api.model.ObjectFactory
+import org.gradle.api.provider.ListProperty
+import org.gradle.api.provider.MapProperty
+import org.gradle.api.provider.Property
+import org.gradle.api.tasks.CacheableTask
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.InputDirectory
+import org.gradle.api.tasks.Internal
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.util.PatternSet
+import org.gradle.workers.WorkQueue
+import org.gradle.workers.WorkerExecutor
+
+/**
+ * Converts the AsciiDoc documents of a directory to HTML pages.
+ *
+ * <p>Every document matching {@link #getIncludes()} and not {@link
#getExcludes()} - by default
+ * every {@code .adoc} file whose name does not start with an underscore - is
converted on its
+ * own, with its own directory as the base directory, to the same relative
path under
+ * {@link #getOutputDir()}. The {@code images} directory of the source
directory is copied
+ * alongside the pages.</p>
+ *
+ * <p>Like the Asciidoctor Gradle plugin, the task sets {@code revnumber} and
the
+ * {@code gradle-project-name}, {@code gradle-project-group} and {@code
gradle-project-version}
+ * attributes from the project, and {@code includedir} to the source
directory, unless
+ * {@link #getAttributes()} sets them.</p>
+ *
+ * <p>The conversion runs in a forked worker process, like {@link
PublishGuideTask}, so the JRuby
+ * runtime AsciidoctorJ starts never lands in the Gradle daemon.</p>
+ *
+ * @since 8.0
+ */
+@CompileStatic
+@CacheableTask
+abstract class ConvertAsciidocTask extends DefaultTask {
+
+ @InputDirectory
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract DirectoryProperty getSourceDir()
+
+ @Input
+ abstract ListProperty<String> getIncludes()
+
+ @Input
+ abstract ListProperty<String> getExcludes()
+
+ /**
+ * The document attributes. Values are converted to strings before the
documents are converted.
+ */
+ @Input
+ abstract MapProperty<String, Object> getAttributes()
+
+ /** The attributes describing the project, which {@link #getAttributes()}
can override. */
+ @Input
+ abstract MapProperty<String, String> getProjectAttributes()
+
+ @OutputDirectory
+ abstract DirectoryProperty getOutputDir()
+
+ /** The JVM arguments of the worker process the documents are converted
in. */
+ @Internal
+ abstract ListProperty<String> getJvmArgs()
+
+ /** The maximum heap of the worker process the documents are converted in.
*/
+ @Internal
+ abstract Property<String> getMaxHeapSize()
+
+ @Inject
+ abstract WorkerExecutor getWorkerExecutor()
+
+ @Inject
+ abstract FileSystemOperations getFileSystemOperations()
+
+ @Inject
+ abstract ObjectFactory getObjects()
+
+ ConvertAsciidocTask() {
+ group = 'documentation'
+ description = 'Converts the AsciiDoc documents to HTML.'
+ includes.convention(['**/*.adoc'])
+ excludes.convention(['**/_*'])
Review Comment:
Nothing tests the task itself. `AsciidocConverterSpec` covers the static
converter, but none of the behaviour that is meant to match the plugin. The
build stays green if any of these change:
- the `_*` exclude;
- the `images` copy;
- `revnumber`, `gradle-project-*` or `includedir` (neo4j's `index.adoc` uses
it);
- the override order at lines 143-146.
A TestKit fixture with a `_partial.adoc`, a file under `images/`,
`{revnumber}` and an `include::{includedir}/...` would pin those down.
`FetchTagsTask` has no spec either.
##########
build-logic/vulnerability-scan/build.gradle:
##########
@@ -16,26 +16,29 @@
* specific language governing permissions and limitations
* under the License.
*/
+
plugins {
id 'groovy-gradle-plugin'
}
version = '0.0.1'
group = 'org.apache.grails.buildsrc'
-// The Sonatype scan plugin is a fat jar that bundles Guava, Groovy, Jackson
and other libraries without
-// relocating them. It is kept in its own project so that those bundled
classes never reach the settings
-// classpath that every build shares when it requests a plugin from
':build-logic'; there they would shadow
-// the real dependencies of other settings plugins (for example the Guava the
dependency-analysis plugin
-// requires).
-project.layout.projectDirectory.file('../../gradle.properties').asFile.withInputStream
{
- Properties props = new Properties()
- props.load(it)
- project.ext.gradleProperties = props
+apply {
+ from project.layout.projectDirectory.file('../../dependencies.gradle')
}
dependencies {
- implementation
"org.sonatype.gradle.plugins:scan-gradle-plugin:${gradleProperties.sonatypeScanPluginVersion}"
+ testImplementation
"org.spockframework:spock-core:${gradleBomDependencyVersions['gradle-spock.version']}"
+ testImplementation gradleTestKit()
+ testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+}
+
+tasks.named('test', Test) {
Review Comment:
As far as I can tell, no workflow runs the build-logic tests.
`build-health.yml` runs only `:build-logic-root:buildHealth`, and the root
`build` doesn't run an included build's `test`. So these specs only run when
someone runs them locally. The same goes for the new `PublishPluginSpec`,
`GroovydocEnhancerPluginSpec`, `SbomPluginSpec` and `GrailsCodeStylePluginSpec`
cases and `AsciidocConverterSpec`, and `vulnerabilityScan` itself only runs
weekly or on a label. Could a CI job run `./gradlew -p build-logic check`?
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -228,16 +228,14 @@ class GrailsGradlePlugin implements Plugin<Project> {
}
}
+ /**
+ * The main class the {@code findMainClass} task found. The value is read
from the task's output when it is
+ * queried, so the configuration cache keeps it lazy instead of storing
whatever the file held before the task ran.
+ */
protected static Provider<String> getMainClassProvider(Project project) {
- Provider<FindMainClassTask> findMainClassTask =
project.tasks.named('findMainClass', FindMainClassTask)
- project.provider {
- File cacheFile =
findMainClassTask.get().mainClassCacheFile.orNull?.asFile
- if (!cacheFile?.exists()) {
- return null
- }
-
- cacheFile?.text
- }
+ project.tasks.named('findMainClass', FindMainClassTask)
+ .flatMap { FindMainClassTask task -> task.mainClassCacheFile }
+ .map { RegularFile cacheFile -> cacheFile.asFile.exists() ?
cacheFile.asFile.text : null }
Review Comment:
The provider shape this replaces is still used in `afterEvaluate` at lines
1345-1361, which set `springBoot.mainClass` and `ext.mainClassName` to
`project.provider { cacheFile.text }` when no main class is configured. Grails'
own tasks no longer read those, but user scripts and third-party plugins do,
since that's what the extension is for. Under the configuration cache they get
whatever the file held when the entry was stored: null on a clean checkout, or
a previous build's class.
Could both use `getMainClassProvider(project)`? One thing to check first: a
provider mapped from a task output can't be queried before `findMainClass` has
run, so anything calling `springBoot.mainClass.get()` during configuration
would start failing instead of getting null.
##########
build-logic/vulnerability-scan/src/main/groovy/org/apache/grails/buildsrc/scan/VulnerabilityScanExtension.groovy:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.buildsrc.scan
+
+import groovy.transform.CompileStatic
+
+import org.gradle.api.provider.ListProperty
+import org.gradle.api.provider.Property
+
+/**
+ * Configures the {@link VulnerabilityScanTask vulnerability scan} of a
project.
+ */
+@CompileStatic
+abstract class VulnerabilityScanExtension {
+
+ /** The configurations whose dependencies are scanned, {@code
runtimeClasspath} and {@code compileClasspath} unless set. */
+ abstract ListProperty<String> getConfigurations()
+
+ /** The vulnerabilities the scan does not report. */
+ abstract ListProperty<VulnerabilityExclusion> getExclusions()
+
+ /** Whether to only report the vulnerabilities instead of failing the
build. */
+ abstract Property<Boolean> getIgnoreFailures()
+
+ /**
+ * Excludes a vulnerability from the scan.
+ *
+ * @param vulnerabilityId the OSV or Sonatype Guide id of the
vulnerability, or one of its aliases such as its CVE
+ * @param module the {@code group:artifact} it is excluded for, or {@code
null} for every module
+ * @param reason why it is excluded and when the exclusion can be removed
+ */
+ void exclude(String vulnerabilityId, String module, String reason) {
+ exclusions.add(new VulnerabilityExclusion(vulnerabilityId, module,
reason))
Review Comment:
`exclusions` only has a convention (`VulnerabilityScanPlugin.groovy:82`),
and `ListProperty.add` builds on the explicit value, not the convention. So the
first `vulnerabilityScan { exclude(...) }` in a project silently drops the
built-in H2, HdrHistogram and Spring Security exclusions, and that project's
scan starts failing on them. On Gradle 9.8.0, `convention(['a', 'b'])` followed
by `add('c')` gives `[c]`; `append('c')` gives `[a, b, c]`.
```suggestion
exclusions.append(new VulnerabilityExclusion(vulnerabilityId,
module, reason))
```
Nothing calls `exclude` yet, so no test catches it. A
`VulnerabilityScanPluginSpec` case that calls `exclude(...)` and checks that
the report still lists `CVE-2018-14335` would.
##########
build-logic/vulnerability-scan/src/main/groovy/org/apache/grails/buildsrc/scan/VulnerabilityScanReportTask.groovy:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.buildsrc.scan
+
+import groovy.json.JsonSlurper
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.file.ConfigurableFileCollection
+import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.provider.Property
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.OutputFile
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.UntrackedTask
+
+/**
+ * Summarizes the reports of {@link VulnerabilityScanTask vulnerability scans}
in Markdown: each vulnerable
+ * dependency once, with the projects it was found in and the databases that
know of each vulnerability, the
+ * vulnerabilities that were excluded, and the exclusions that no longer match
anything. The last are only listed
+ * when every database was consulted.
+ */
+@CompileStatic
+@UntrackedTask(because = 'It summarizes scans whose results change without the
build changing')
+abstract class VulnerabilityScanReportTask extends DefaultTask {
+
+ /** The reports of the scans; those that do not exist are skipped. */
+ @InputFiles
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract ConfigurableFileCollection getReports()
+
+ /** Truncates the listing to this many bytes; {@code 0} keeps all of it. */
+ @Input
+ abstract Property<Integer> getMaxBytes()
+
+ @OutputFile
+ abstract RegularFileProperty getSummaryFile()
+
+ @TaskAction
+ void summarize() {
+ List<Map<String, Object>> scans = reports.files.findAll { it.isFile()
}.sort().collect { File file ->
Review Comment:
A scan whose lookup fails throws before `scan()` writes
`vulnerabilities.json`, for example when OSV or Sonatype Guide still answers
429/5xx after the retries, or keeps timing out. This line then skips that
project without a trace. With `--continue` the other scans finish, and if
they're clean, the step summary and the sticky comment say "✅ No known
vulnerabilities in the dependencies of N scanned projects" while the job is
red. "Exclusions that match nothing" can also tell us to remove an exclusion
that only matched in the project whose scan failed.
Could `scan()` write `[project: ..., error: e.message]` to its report before
rethrowing when a lookup fails? `render` could then list those projects with
⚠️, never print ✅ while one of them failed, and skip the "match nothing"
section, as it already does when a source wasn't consulted. As it stands, the
workflow comment "--continue lets the summary report the projects whose scan
failed" holds only for scans that failed on findings.
##########
build-logic/vulnerability-scan/src/main/groovy/org/apache/grails/buildsrc/scan/OsvVulnerabilityService.groovy:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.buildsrc.scan
+
+import java.net.http.HttpRequest
+import java.nio.charset.StandardCharsets
+import java.nio.file.Path
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+
+import groovy.json.JsonOutput
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.provider.Property
+import org.gradle.api.services.BuildService
+import org.gradle.api.services.BuildServiceParameters
+
+/**
+ * Looks the vulnerabilities of Maven artifacts up in the <a
href="https://osv.dev";>OSV</a> database.
+ *
+ * <p>Every answer is kept on disk for {@link Parameters#getCacheTtl() a
while}, so builds that run again shortly
+ * after, in this or another checkout sharing the cache directory, do not ask
the API again. Entries older than that
+ * are fetched again.</p>
+ *
+ * <p>The service is shared by the scan tasks of every project and asks the
API one task at a time, which keeps the
+ * build well under the API's rate limits.</p>
+ */
+@CompileStatic
+abstract class OsvVulnerabilityService implements BuildService<Parameters>,
AutoCloseable {
+
+ static final String NAME = 'OSV'
+ static final String DEFAULT_API_URL = 'https://api.osv.dev'
+ static final Duration DEFAULT_CACHE_TTL = Duration.ofHours(1)
+
+ private static final int BATCH_SIZE = 1000
+
+ interface Parameters extends BuildServiceParameters {
+
+ /** The OSV API, {@value OsvVulnerabilityService#DEFAULT_API_URL}
unless a test points it elsewhere. */
+ Property<String> getApiUrl()
+
+ /** Where the answers of the API are kept between builds, shared with
the other databases. */
+ DirectoryProperty getCacheDirectory()
+
+ /** How long an answer is used before the API is asked again. */
+ Property<Duration> getCacheTtl()
+ }
+
+ private final Map<String, List<String>> idsByCoordinate = new
ConcurrentHashMap<>()
+ private final Map<String, Map<String, Object>> vulnerabilitiesById = new
ConcurrentHashMap<>()
+ private final ApiClient client = new ApiClient('OSV API')
+ private ApiCache queryCache
+ private ApiCache vulnerabilityCache
+
+ /**
+ * Returns the vulnerabilities known for each of the given {@code
group:artifact:version} coordinates, as the
+ * OSV records of them.
+ */
+ synchronized Map<String, List<Map<String, Object>>>
vulnerabilities(Collection<String> coordinates) {
+ Collection<String> unique = coordinates.toUnique()
+ List<String> missing = unique.findAll { String coordinate ->
idsFor(coordinate) == null }.toList()
+ if (missing) {
+ queryIds(missing)
+ }
+
+ Map<String, List<Map<String, Object>>> vulnerabilities = [:]
+ for (String coordinate : unique) {
+ vulnerabilities.put(coordinate,
idsByCoordinate.get(coordinate).collect { String id -> vulnerability(id) })
+ }
+ vulnerabilities
+ }
+
+ @Override
+ void close() {
+ client.close()
+ }
+
+ private List<String> idsFor(String coordinate) {
+ List<String> ids = idsByCoordinate.get(coordinate)
+ if (ids == null) {
+ Map<String, Object> cached = queryCache().read(coordinate)
+ if (cached != null) {
+ ids = (List<String>) cached.get('ids')
+ idsByCoordinate.put(coordinate, ids)
+ }
+ }
+ ids
+ }
+
+ private void queryIds(List<String> coordinates) {
+ coordinates.collate(BATCH_SIZE).each { List<String> batch ->
+ Map<String, List<String>> ids = batch.collectEntries { [(it): new
ArrayList<String>()] }
+ Map<String, String> pageTokens = [:]
+ List<String> pending = batch
+ while (pending) {
+ List<Map<String, Object>> queries = pending.collect { String
coordinate -> query(coordinate, pageTokens.get(coordinate)) }
+ List<Map<String, Object>> results =
resultsOf(ApiClient.json(client.send(post('/v1/querybatch', [queries:
queries]))))
+ pageTokens.clear()
+ pending.eachWithIndex { String coordinate, int index ->
+ Map<String, Object> result = results[index] ?: [:]
Review Comment:
If a `querybatch` answer has fewer `results` than queries, or none (which
`resultsOf` turns into `[]`), `results[index]` is null past the end. Every
remaining coordinate is then recorded as clean and cached for an hour (line
129) in the Gradle user home, where every build on the machine reads it. The
Sonatype Guide service already throws when a coordinate isn't reported on. The
same guard here, after line 115:
```groovy
if (results.size() != pending.size()) {
throw new GradleException("The OSV API answered ${pending.size()}
queries with ${results.size()} results")
}
```
##########
build-logic/vulnerability-scan/src/main/groovy/org/apache/grails/buildsrc/scan/ApiClient.groovy:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.buildsrc.scan
+
+import java.net.http.HttpClient
+import java.net.http.HttpRequest
+import java.net.http.HttpResponse
+import java.time.Duration
+
+import groovy.json.JsonSlurper
+import groovy.transform.CompileStatic
+
+import org.gradle.api.GradleException
+import org.gradle.api.logging.Logger
+import org.gradle.api.logging.Logging
+
+/**
+ * Sends the requests to a vulnerability database, retrying those that were
throttled or failed on the way.
+ */
+@CompileStatic
+class ApiClient implements AutoCloseable {
+
+ private static final Logger LOG = Logging.getLogger(ApiClient)
+ private static final int MAX_ATTEMPTS = 4
+ private static final int MAX_ERROR_BODY = 500
+
+ private final String name
+ private HttpClient httpClient
+
+ /**
+ * @param name what the API is called in messages, such as {@code OSV API}
+ */
+ ApiClient(String name) {
+ this.name = name
+ }
+
+ /**
+ * Sends the request and returns the answer, once it is a {@code 200}. A
request that was throttled, failed
+ * with a server error or did not get through is sent again, after the
time the API asks for if it does.
+ *
+ * @param hints what to tell the user when the API answers with one of
these statuses
+ */
+ HttpResponse<String> send(HttpRequest request, Map<Integer, String> hints
= [:]) {
+ IOException failure = null
+ for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
+ try {
+ HttpResponse<String> response = client().send(request,
HttpResponse.BodyHandlers.ofString())
+ int status = response.statusCode()
+ if (status == 200) {
+ return response
+ }
+ if (status != 429 && status < 500) {
+ String body = response.body() ?: ''
+ throw new GradleException("The ${name} answered
${request.method()} ${request.uri()} with ${status}" +
+ "${hints.get(status) ? " (${hints.get(status)})" :
''}: " +
+ "${body.length() > MAX_ERROR_BODY ?
body.substring(0, MAX_ERROR_BODY) + '…' : body}")
+ }
+ LOG.info('The {} answered {} {} with {}, attempt {} of {}',
name, request.method(), request.uri(),
+ status, attempt, MAX_ATTEMPTS)
+ if (attempt < MAX_ATTEMPTS) {
+ sleep(retryDelay(response, attempt))
+ }
+ }
+ catch (IOException e) {
+ failure = e
+ LOG.info('Could not reach the {} at {}, attempt {} of {}',
name, request.uri(), attempt, MAX_ATTEMPTS, e)
+ if (attempt < MAX_ATTEMPTS) {
+ sleep(Duration.ofSeconds(1L << attempt).toMillis())
+ }
+ }
+ }
+ throw new GradleException("The ${name} did not answer
${request.method()} ${request.uri()} after " +
Review Comment:
Two small things in the retry loop:
- When all four attempts were 429s or 5xx, `failure` is null and the message
says the API "did not answer", without the last status or body, so a rate limit
looks like an outage. Keeping the last response and putting its status and body
in the message would tell them apart.
- Groovy's `sleep(long)` ignores interrupts, so cancelling the build waits
out each backoff, which can be up to 60 s per attempt with `Retry-After`.
`Thread.sleep` stops when the build is cancelled.
##########
build-logic/docs-core/src/main/groovy/grails/doc/gradle/ConvertAsciidocTask.groovy:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.doc.gradle
+
+import javax.inject.Inject
+
+import groovy.transform.CompileStatic
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.Project
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.file.FileVisitDetails
+import org.gradle.api.model.ObjectFactory
+import org.gradle.api.provider.ListProperty
+import org.gradle.api.provider.MapProperty
+import org.gradle.api.provider.Property
+import org.gradle.api.tasks.CacheableTask
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.InputDirectory
+import org.gradle.api.tasks.Internal
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.util.PatternSet
+import org.gradle.workers.WorkQueue
+import org.gradle.workers.WorkerExecutor
+
+/**
+ * Converts the AsciiDoc documents of a directory to HTML pages.
+ *
+ * <p>Every document matching {@link #getIncludes()} and not {@link
#getExcludes()} - by default
+ * every {@code .adoc} file whose name does not start with an underscore - is
converted on its
+ * own, with its own directory as the base directory, to the same relative
path under
+ * {@link #getOutputDir()}. The {@code images} directory of the source
directory is copied
+ * alongside the pages.</p>
+ *
+ * <p>Like the Asciidoctor Gradle plugin, the task sets {@code revnumber} and
the
+ * {@code gradle-project-name}, {@code gradle-project-group} and {@code
gradle-project-version}
+ * attributes from the project, and {@code includedir} to the source
directory, unless
+ * {@link #getAttributes()} sets them.</p>
+ *
+ * <p>The conversion runs in a forked worker process, like {@link
PublishGuideTask}, so the JRuby
+ * runtime AsciidoctorJ starts never lands in the Gradle daemon.</p>
+ *
+ * @since 8.0
+ */
+@CompileStatic
+@CacheableTask
Review Comment:
The only file input is `sourceDir`, but 21 of the guide pages include code
from outside it through attributes: `grailsTestExamplesMongodbBaseDir`,
`grailsDataMongodbCoreDir`, neo4j's `exampledir`/`testsdir`, graphql's
`sourcedir`, and others. A change that only touches an included example leaves
the cache key unchanged. The task is then UP-TO-DATE locally or FROM-CACHE on
CI, and the published guide shows the old code. The neo4j and graphql
attributes are also absolute paths in the `@Input` map, which makes those cache
keys machine-specific.
I think the plugin had the same gap, but now that this is our task, could it
take an `@InputFiles @PathSensitive(RELATIVE)` collection of the included
directories? Path-valued attributes could then stay out of the `@Input` map,
relativized the way `PublishGuideTask.getRelativizedPropertiesWithFilePaths()`
does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
