Skip to content

Commit 2d00ee0

Browse files
committed
refactor(license-fact-provider): Ensure that license texts are not blank
Ensure in all license fact providers that returned license texts are not blank. Use a `LicenseText` value class to model this in the API of `getLicenseText()`. This way all consumers can rely on the text not being blank, and that if a provider violates that constraint, it will fail immediately instead of leading to follow-up errors. Signed-off-by: Martin Nonnenmacher <[email protected]> Signed-off-by: Sebastian Schuberth <[email protected]>
1 parent f5cae5a commit 2d00ee0

File tree

16 files changed

+149
-42
lines changed

16 files changed

+149
-42
lines changed

plugins/license-fact-providers/api/src/main/kotlin/CompositeLicenseFactProvider.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class CompositeLicenseFactProvider(
3030
* providers: the first provider in the list that has a fact for a given license ID will be used.
3131
*/
3232
private val providers: List<LicenseFactProvider>
33-
) : LicenseFactProvider {
33+
) : LicenseFactProvider() {
3434
override val descriptor = PluginDescriptor(
3535
id = "Composite",
3636
displayName = "Composite License Fact Provider",

plugins/license-fact-providers/api/src/main/kotlin/LicenseFactProvider.kt

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,15 @@ package org.ossreviewtoolkit.plugins.licensefactproviders.api
2222
import org.ossreviewtoolkit.plugins.api.Plugin
2323

2424
/** A provider for license facts. */
25-
interface LicenseFactProvider : Plugin {
26-
/** Return the license text for the given [licenseId], or `null` if the license text is not available. */
27-
fun getLicenseText(licenseId: String): String?
25+
abstract class LicenseFactProvider : Plugin {
26+
/** Return the [LicenseText] for the given [licenseId], or `null` if no valid text is available. */
27+
abstract fun getLicenseText(licenseId: String): LicenseText?
28+
29+
/** Return a non-blank license text for the given [licenseId], or `null` if no valid text is available. */
30+
@Deprecated("Java-only API", level = DeprecationLevel.HIDDEN)
31+
@JvmName("getLicenseText")
32+
fun getNonBlankLicenseText(licenseId: String): String? = getLicenseText(licenseId)?.text
2833

2934
/** Return `true´ if this provider has a license text for the given [licenseId]. */
30-
fun hasLicenseText(licenseId: String): Boolean
35+
abstract fun hasLicenseText(licenseId: String): Boolean
3136
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright (C) 2025 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* License-Filename: LICENSE
18+
*/
19+
20+
package org.ossreviewtoolkit.plugins.licensefactproviders.api
21+
22+
/** A value class to represent a valid license text. It guarantees that the text is not blank. */
23+
@JvmInline
24+
value class LicenseText(val text: String) {
25+
init {
26+
require(text.isNotBlank()) { "The license text must not be blank." }
27+
}
28+
}

plugins/license-fact-providers/dir/src/main/kotlin/DirLicenseFactProvider.kt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import org.ossreviewtoolkit.plugins.api.OrtPlugin
2727
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
2828
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProvider
2929
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProviderFactory
30+
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseText
3031
import org.ossreviewtoolkit.utils.ort.ORT_CUSTOM_LICENSE_TEXTS_DIRNAME
3132
import org.ossreviewtoolkit.utils.ort.ortConfigDirectory
3233

@@ -59,16 +60,20 @@ class DefaultDirLicenseFactProvider(descriptor: PluginDescriptor = DefaultDirLic
5960
open class DirLicenseFactProvider(
6061
override val descriptor: PluginDescriptor = DirLicenseFactProviderFactory.descriptor,
6162
config: DirLicenseFactProviderConfig
62-
) : LicenseFactProvider {
63+
) : LicenseFactProvider() {
6364
private val licenseTextDir = File(config.licenseTextDir).also {
6465
if (!it.isDirectory) {
6566
logger.warn { "The license text directory '${it.absolutePath}' does not exist or is not a directory." }
6667
}
6768
}
6869

69-
override fun getLicenseText(licenseId: String) = getLicenseTextFile(licenseId)?.readText()
70+
override fun getLicenseText(licenseId: String) = getLicenseTextFile(licenseId)?.readText()?.let { LicenseText(it) }
7071

7172
override fun hasLicenseText(licenseId: String) = getLicenseTextFile(licenseId) != null
7273

73-
private fun getLicenseTextFile(licenseId: String) = licenseTextDir.resolve(licenseId).takeIf { it.isFile }
74+
private fun getLicenseTextFile(licenseId: String) =
75+
licenseTextDir.resolve(licenseId).takeIf { it.isFile && it.isNotBlank }
7476
}
77+
78+
private val File.isNotBlank: Boolean
79+
get() = useLines { lines -> lines.any { line -> line.any { !it.isWhitespace() } } }

plugins/license-fact-providers/dir/src/test/kotlin/DirLicenseFactProviderTest.kt

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,23 @@ class DirLicenseFactProviderTest : WordSpec({
3333

3434
val provider = DirLicenseFactProviderFactory.create(licenseTextDir = licenseDir.absolutePath)
3535

36-
provider.getLicenseText("LicenseRef-custom-license") shouldBe "custom license text"
36+
provider.getLicenseText("LicenseRef-custom-license")?.text shouldBe "custom license text"
3737
}
3838

3939
"return null if no license text is found" {
4040
val provider = DirLicenseFactProviderFactory.create(licenseTextDir = tempdir().absolutePath)
4141

4242
provider.getLicenseText("LicenseRef-non-existing-license") should beNull()
4343
}
44+
45+
"return null if the license text file is blank" {
46+
val licenseDir = tempdir()
47+
licenseDir.resolve("LicenseRef-blank-license").writeText(" \n ")
48+
49+
val provider = DirLicenseFactProviderFactory.create(licenseTextDir = licenseDir.absolutePath)
50+
51+
provider.getLicenseText("LicenseRef-blank-license") should beNull()
52+
}
4453
}
4554

4655
"hasLicenseText()" should {
@@ -58,5 +67,14 @@ class DirLicenseFactProviderTest : WordSpec({
5867

5968
provider.hasLicenseText("LicenseRef-non-existing-license") shouldBe false
6069
}
70+
71+
"return false if the license text file is blank" {
72+
val licenseDir = tempdir()
73+
licenseDir.resolve("LicenseRef-blank-license").writeText(" \n ")
74+
75+
val provider = DirLicenseFactProviderFactory.create(licenseTextDir = licenseDir.absolutePath)
76+
77+
provider.hasLicenseText("LicenseRef-blank-license") shouldBe false
78+
}
6179
}
6280
})

plugins/license-fact-providers/scancode/src/funTest/kotlin/ScanCodeLicenseFactProviderFunTest.kt

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class ScanCodeLicenseFactProviderFunTest : WordSpec({
3737

3838
val provider = ScanCodeLicenseFactProviderFactory.create(scanCodeLicenseTextDir = licenseDir.absolutePath)
3939

40-
provider.getLicenseText("test") shouldBe "custom license text"
40+
provider.getLicenseText("test")?.text shouldBe "custom license text"
4141
}
4242

4343
"remove YAML front matter from the license text" {
@@ -54,14 +54,14 @@ class ScanCodeLicenseFactProviderFunTest : WordSpec({
5454

5555
val provider = ScanCodeLicenseFactProviderFactory.create(scanCodeLicenseTextDir = licenseDir.absolutePath)
5656

57-
provider.getLicenseText("test") shouldBe "custom license text"
57+
provider.getLicenseText("test")?.text shouldBe "custom license text"
5858
}
5959

6060
"read license texts from the detected ScanCode license directory" {
6161
val provider = ScanCodeLicenseFactProviderFactory.create()
6262

6363
provider.getLicenseText("MIT") shouldNotBeNull {
64-
this should startWith("Permission is hereby granted, free of charge, to any person obtaining")
64+
text should startWith("Permission is hereby granted, free of charge, to any person obtaining")
6565
}
6666
}
6767

@@ -70,6 +70,22 @@ class ScanCodeLicenseFactProviderFunTest : WordSpec({
7070

7171
provider.getLicenseText("UnknownLicense") should beNull()
7272
}
73+
74+
"return null if the license file contains only YAML front matter" {
75+
val licenseDir = tempdir()
76+
(licenseDir / "test.LICENSE").writeText(
77+
"""
78+
|---
79+
|key: value
80+
|---
81+
|
82+
""".trimMargin()
83+
)
84+
85+
val provider = ScanCodeLicenseFactProviderFactory.create(scanCodeLicenseTextDir = licenseDir.absolutePath)
86+
87+
provider.getLicenseText("test") should beNull()
88+
}
7389
}
7490

7591
"hasLicenseText()" should {
@@ -84,5 +100,21 @@ class ScanCodeLicenseFactProviderFunTest : WordSpec({
84100

85101
provider.hasLicenseText("UnknownLicense") shouldBe false
86102
}
103+
104+
"return false if the license file contains only YAML front matter" {
105+
val licenseDir = tempdir()
106+
(licenseDir / "test.LICENSE").writeText(
107+
"""
108+
|---
109+
|key: value
110+
|---
111+
|
112+
""".trimMargin()
113+
)
114+
115+
val provider = ScanCodeLicenseFactProviderFactory.create(scanCodeLicenseTextDir = licenseDir.absolutePath)
116+
117+
provider.hasLicenseText("test") shouldBe false
118+
}
87119
}
88120
})

plugins/license-fact-providers/scancode/src/main/kotlin/ScanCodeLicenseFactProvider.kt

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import org.ossreviewtoolkit.plugins.api.OrtPlugin
2727
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
2828
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProvider
2929
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProviderFactory
30+
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseText
3031
import org.ossreviewtoolkit.utils.common.Os
3132
import org.ossreviewtoolkit.utils.common.realFile
3233

@@ -50,7 +51,7 @@ data class ScanCodeLicenseFactProviderConfig(
5051
class ScanCodeLicenseFactProvider(
5152
override val descriptor: PluginDescriptor = ScanCodeLicenseFactProviderFactory.descriptor,
5253
private val config: ScanCodeLicenseFactProviderConfig
53-
) : LicenseFactProvider {
54+
) : LicenseFactProvider() {
5455
/**
5556
* The directory that contains the ScanCode license texts. This is located using a heuristic based on the path of
5657
* the ScanCode binary.
@@ -118,21 +119,36 @@ class ScanCodeLicenseFactProvider(
118119
"${licenseId.removePrefix("LicenseRef-scancode-").lowercase()}.LICENSE"
119120
}
120121

121-
return scanCodeLicenseTextDir?.resolve(filename)?.takeIf { it.isFile }
122+
return scanCodeLicenseTextDir?.resolve(filename)?.takeIf { it.isFile && it.isNotBlank }
122123
}
123124

124-
override fun getLicenseText(licenseId: String): String? =
125-
getLicenseTextFile(licenseId)?.readText()?.removeYamlFrontMatter()
125+
override fun getLicenseText(licenseId: String) =
126+
getLicenseTextFile(licenseId)?.useLines { lines ->
127+
lines.skipYamlFrontMatter().joinToString("\n").trimEnd()
128+
}?.let {
129+
LicenseText(it)
130+
}
126131

127132
override fun hasLicenseText(licenseId: String): Boolean = getLicenseTextFile(licenseId) != null
128133
}
129134

130-
internal fun String.removeYamlFrontMatter(): String {
131-
val lines = lines()
132-
133-
// Remove any YAML front matter enclosed by "---" from ScanCode license files.
134-
val licenseLines = lines.takeUnless { it.first() == "---" }
135-
?: lines.drop(1).dropWhile { it != "---" }.drop(1)
135+
internal fun Sequence<String>.skipYamlFrontMatter(): Sequence<String> {
136+
var inFrontMatter = false
136137

137-
return licenseLines.dropWhile { it.isEmpty() }.joinToString("\n").trimEnd()
138+
return filterIndexed { index, line ->
139+
if (index == 0 && line == "---") {
140+
inFrontMatter = true
141+
false
142+
} else if (inFrontMatter) {
143+
if (line == "---") inFrontMatter = false
144+
false
145+
} else {
146+
true
147+
}
148+
}.dropWhile {
149+
it.isBlank()
150+
}
138151
}
152+
153+
private val File.isNotBlank: Boolean
154+
get() = useLines { lines -> lines.skipYamlFrontMatter().any { line -> line.any { !it.isWhitespace() } } }

plugins/license-fact-providers/scancode/src/test/kotlin/ScanCodeLicenseFactProviderTest.kt

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import io.kotest.core.spec.style.WordSpec
2323
import io.kotest.matchers.shouldBe
2424

2525
class ScanCodeLicenseFactProviderTest : WordSpec({
26-
"removeYamlFrontMatter" should {
27-
"remove a YAML front matter" {
26+
"skipYamlFrontMatter()" should {
27+
"skip a YAML front matter" {
2828
val text = """
2929
---
3030
key: alasir
@@ -62,10 +62,6 @@ class ScanCodeLicenseFactProviderTest : WordSpec({
6262
""".trimIndent()
6363
}
6464

65-
"remove trailing whitespace" {
66-
"last sentence\n".removeYamlFrontMatter() shouldBe "last sentence"
67-
}
68-
6965
"remove leading empty lines" {
7066
"\nfirst sentence".removeYamlFrontMatter() shouldBe "first sentence"
7167
}
@@ -75,3 +71,5 @@ class ScanCodeLicenseFactProviderTest : WordSpec({
7571
}
7672
}
7773
})
74+
75+
private fun String.removeYamlFrontMatter() = lineSequence().skipYamlFrontMatter().joinToString("\n")

plugins/license-fact-providers/spdx/src/main/kotlin/SpdxLicenseFactProvider.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import org.ossreviewtoolkit.plugins.api.OrtPlugin
2525
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
2626
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProvider
2727
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProviderFactory
28+
import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseText
2829

2930
@OrtPlugin(
3031
id = "SPDX",
@@ -34,8 +35,12 @@ import org.ossreviewtoolkit.plugins.licensefactproviders.api.LicenseFactProvider
3435
)
3536
class SpdxLicenseFactProvider(
3637
override val descriptor: PluginDescriptor = SpdxLicenseFactProviderFactory.descriptor
37-
) : LicenseFactProvider {
38-
override fun getLicenseText(licenseId: String) = getLicenseTextResource(licenseId)?.readText()
38+
) : LicenseFactProvider() {
39+
override fun getLicenseText(licenseId: String) =
40+
getLicenseTextResource(licenseId)?.readText()?.let {
41+
// It can be safely assumed that the license text is not blank as all SPDX license texts are non-blank.
42+
LicenseText(it)
43+
}
3944

4045
override fun hasLicenseText(licenseId: String) = getLicenseTextResource(licenseId) != null
4146

plugins/license-fact-providers/spdx/src/test/kotlin/SpdxLicenseFactProviderTest.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,20 @@ class SpdxLicenseFactProviderTest : WordSpec({
3737

3838
"getLicenseText()" should {
3939
"return the license text for Apache-2.0" {
40-
val text = provider.getLicenseText(SpdxLicense.APACHE_2_0.id)
41-
text should contain("Apache License")
42-
text should haveLength(11357)
40+
val license = provider.getLicenseText(SpdxLicense.APACHE_2_0.id)
41+
license?.text should contain("Apache License")
42+
license?.text should haveLength(11357)
4343
}
4444

4545
"return the license text for all SPDX licenses" {
4646
enumValues<SpdxLicense>().forAll {
47-
provider.getLicenseText(it.id) shouldNot beEmpty()
47+
provider.getLicenseText(it.id)?.text shouldNot beEmpty()
4848
}
4949
}
5050

5151
"return the license text for all SPDX exceptions" {
5252
enumValues<SpdxLicenseException>().forAll {
53-
provider.getLicenseText(it.id) shouldNot beEmpty()
53+
provider.getLicenseText(it.id)?.text shouldNot beEmpty()
5454
}
5555
}
5656

0 commit comments

Comments
 (0)