Skip to content

fix(freemarker): Remove uncategorized licenses from category filter #10231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,12 @@ class FreemarkerTemplateProcessor(
}

/**
* Return only those [licenses] that are classified under the given [category], or that are not categorized at
* all.
* Return only those [licenses] that are classified under the given [category].
*/
@Suppress("UNUSED") // This function is used in the templates.
fun filterForCategory(licenses: Collection<ResolvedLicense>, category: String): List<ResolvedLicense> =
licenses.filter { resolvedLicense ->
input.licenseClassifications[resolvedLicense.license]?.contains(category) != false
input.licenseClassifications[resolvedLicense.license]?.contains(category) ?: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notice templates still uses filterForCategory(), and IIUC these intends to include exactly the licenses which are either / or:

  1. uncategorized
  2. categorized with the (provided) include-in-notice category.

I suppose that this behavior shall stay as is. So, I think this PR should account for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understand you correctly.
Is your proposal

  1. Create a new function that returns the uncategorized licenses in addition to the proposed change? And additionally adjust the notice template?
    Or is it
  2. Closing this PR and keeping the functionality as-is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understand you correctly.

ORT has the files NOTICE_DEFAULT.ftl and NOTICE_SUMMARY.ftl.
I believe that the output should of these templates should remain unaffected by this PR.
This could be achieved with option #1 above IIUC.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.model.VcsInfo
import org.ossreviewtoolkit.model.VcsType
import org.ossreviewtoolkit.model.config.LicenseChoices
import org.ossreviewtoolkit.model.config.OrtConfiguration
import org.ossreviewtoolkit.model.config.PackageLicenseChoice
import org.ossreviewtoolkit.model.config.RepositoryConfiguration
import org.ossreviewtoolkit.model.licenses.LicenseCategorization
import org.ossreviewtoolkit.model.licenses.LicenseCategory
import org.ossreviewtoolkit.model.licenses.LicenseClassifications
import org.ossreviewtoolkit.model.licenses.LicenseInfoResolver
import org.ossreviewtoolkit.model.licenses.ResolvedLicense
import org.ossreviewtoolkit.model.licenses.ResolvedLicenseInfo
Expand Down Expand Up @@ -585,4 +589,83 @@ class FreeMarkerTemplateProcessorTest : WordSpec({
helper.getPackage(id) shouldBe Package.EMPTY
}
}

"filterForCategory" should {
"correctly filters licenses for the given categories" {
val licenseInCatA = ResolvedLicense(
license = SpdxSingleLicenseExpression.parse("Apache-2.0"),
originalDeclaredLicenses = setOf("Apache-2.0"),
originalExpressions = setOf(
ResolvedOriginalExpression(
SpdxSingleLicenseExpression.parse("Apache-2.0"),
LicenseSource.DECLARED
)
),
locations = emptySet()
)

val licenseInCatB = ResolvedLicense(
license = SpdxSingleLicenseExpression.parse("MIT"),
originalDeclaredLicenses = setOf("MIT"),
originalExpressions = setOf(
ResolvedOriginalExpression(
SpdxSingleLicenseExpression.parse("MIT"),
LicenseSource.DECLARED
)
),
locations = emptySet()
)

val uncategorizedLicense = ResolvedLicense(
license = SpdxSingleLicenseExpression.parse("BSD-3-Clause"),
originalDeclaredLicenses = setOf("BSD-3-Clause"),
originalExpressions = setOf(
ResolvedOriginalExpression(
SpdxSingleLicenseExpression.parse("BSD-3-Clause"),
LicenseSource.DECLARED
)
),
locations = emptySet()
)

val allLicenses = listOf(licenseInCatA, licenseInCatB, uncategorizedLicense)

val licenseClassifications = LicenseClassifications(
categories = listOf(LicenseCategory("categoryA"), LicenseCategory("categoryB")),
categorizations = listOf(
LicenseCategorization(
id = SpdxSingleLicenseExpression.parse("Apache-2.0"),
categories = setOf("categoryA")
),
LicenseCategorization(
id = SpdxSingleLicenseExpression.parse("MIT"),
categories = setOf("categoryB")
)
)
)

val helper = FreemarkerTemplateProcessor.TemplateHelper(
ReporterInput(
ortResult = OrtResult.EMPTY,
ortConfig = OrtConfiguration(),
licenseClassifications = licenseClassifications
)
)

val categoryA = helper.filterForCategory(allLicenses, "categoryA")
categoryA shouldContainExactlyInAnyOrder listOf(licenseInCatA)

val categoryB = helper.filterForCategory(allLicenses, "categoryB")
categoryB shouldContainExactlyInAnyOrder listOf(licenseInCatB)

val nonExistentCategory = helper.filterForCategory(allLicenses, "nonExistent")
nonExistentCategory shouldBe emptyList()

val emptyListResult = helper.filterForCategory(emptyList(), "categoryA")
emptyListResult shouldBe emptyList()

val uncategorizedResult = helper.filterForCategory(listOf(uncategorizedLicense), "categoryA")
uncategorizedResult shouldContainExactlyInAnyOrder listOf()
}
}
})
Loading