-
Notifications
You must be signed in to change notification settings - Fork 32
docs: new field of license table #1897
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR adds a new ADR (00009) documenting the decision to introduce a Entity relationship diagram for the new custom_license_refs field in the license tableerDiagram
LICENSE {
int id
text text
text[] spdx_licenses
text[] spdx_license_exceptions
text[] custom_license_refs
}
LICENSE ||--o{ PACKAGE : contains
LICENSE ||--o{ SBOM : used_in
Class diagram for the updated License model with custom_license_refsclassDiagram
class License {
+int id
+string text
+string[] spdx_licenses
+string[] spdx_license_exceptions
+string[] custom_license_refs
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1897 +/- ##
==========================================
+ Coverage 68.14% 68.26% +0.11%
==========================================
Files 365 367 +2
Lines 23123 23216 +93
Branches 23123 23216 +93
==========================================
+ Hits 15757 15848 +91
- Misses 6485 6488 +3
+ Partials 881 880 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
af433c0 to
844f3c1
Compare
|
@sourcery-ai summary |
|
@sourcery-ai guide |
docs/adrs/00009-new-field-license.md
Outdated
| Example: | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the other fields of that table entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`#[sea_orm(table_name = "license")]
pub struct Model {
#[sea_orm(primary_key)]
pub id: Uuid,
pub text: String,
pub spdx_licenses: Option<Vec>,
pub spdx_license_exceptions: Option<Vec>,
}
`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can add those to the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, Please check again.
|
|
||
| `custom_license_refs`: | ||
|
|
||
| ```json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we not store the other information (comment, extracted text)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These pieces of information are already present in licensing_infos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What the relationship between those two tables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current design is not sufficient to support establishing a relationship between these two tables. If we want to build such a relationship, we would need to introduce a mapping-like entity between them. However, this would increase the complexity of both ingestion and querying.
At present, the inclusion of the license name here is actually redundant, and it may also have negative impacts.
It could lead to potential data inconsistencies — for example, if the corresponding data in license_info is deleted, it cannot be deleted here.
However, in our business scenario, we do not delete individual license_info entries; we only delete an entire SBOM. When deleting the entire SBOM, the related records in the license table will naturally be deleted as well, so this design should still be reasonable.
docs/adrs/00009-new-field-license.md
Outdated
| Example: | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me like as if the type doesn't match the declared type above (text[]). This seems to be a map approach, while the column's type is an array of strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you map that JSON object into a Vec<String>?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you map that JSON object into a
Vec<String>?
I do not understand this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not treat it as a JSON string; I just handled it as a plain string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type of the column is Vec<String> in Rust, text[] in SQL. The example provided is a JSON object. How would a JSON object be mapped in a Vec<String>?
| ], | ||
| "filesAnalyzed": false, | ||
| "licenseConcluded": "NOASSERTION", | ||
| "licenseDeclared": "LicenseRef-2 AND LicenseRef-11 AND LicenseRef-BSD", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would that work with SPDX licenses IDs (non-custom)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will parse such license expressions and extract the standard license IDs, storing them in spdx_licenses and spdx_license_exceptions.
844f3c1 to
7bf378e
Compare
7bf378e to
26235da
Compare
26235da to
80b5163
Compare
|
|
||
| In SPDX, there are two types of licenses: licenseDeclared and licenseConcluded, and the representation is mainly in the form of license expressions, which can include custom license references (licenseRef). | ||
|
|
||
| CycloneDX uses only one type, licenseDeclared, but offers three ways to represent it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to CycloneDX 1.6 specifications it has both types https://cyclonedx.org/docs/1.6/json/#components_items_licenses_oneOf_i0_items_license_acknowledgement
Rendered version: https://github.com/bxf12315/trustify/blob/adr-new-field-license/docs/adrs/00009-new-field-license.md
Summary by Sourcery
Document the decision to add a new custom_license_refs text[] column to the license table to store custom license identifiers from CycloneDX name entries and SPDX expressions for improved license usage statistics.
New Features:
Documentation:
Summary by Sourcery
Document a new ADR to introduce a custom_license_refs text[] column in the license table for capturing custom license references from CycloneDX and SPDX to enhance license usage statistics.
New Features:
Documentation: