Skip to content
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

feat: update input file schema to include configurable classifications for signals #2800

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

abraemer
Copy link
Contributor

@abraemer abraemer commented Feb 18, 2025

Summary of changes

Introduces a new field on externalAttributions called classification. It is a positive integer and its meaning (i.e. text label) can be configured via a new mapping classifications. In anticipation of further configurable items in the future, a new top-level key config is introduced with classification being its only subkey currently.

Context and reason for change

We want to replace criticality by something more universally usable. Thus we introduce its successor in this PR and will build on this is in following work.

No further changes are required at this point. Opening files with OpossumUI and saving them preserves the classification information out-of-the-box since the input file is never changed.

Compatibility

This change is backwards compatible: new versions of OpossumUI can open old opossum files. However, new opossum files (with config and/or classification) cannot be opened by old OpossumUI releases.

How can the changes be tested

There should be no functional changes because we just added optional fields. Everything that worked before still works.

@@ -121,6 +133,11 @@
"type": "string",
"description": "Indication on how critical a signal is. Possible values are \"high\" and \"medium\"."
},
"classification": {
"type": "number",
"description": "Signal classification by usage, usually based on the license. Zero corresponds to unrestricted usage ehile higher levels mean more and more restrictions. Exact values and labels need to be configured in the config section.",
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: ehile --> while

@abraemer abraemer force-pushed the update-schema-with-license-classification branch from baec23f to eebf3a0 Compare February 19, 2025 08:39
"classifications": {
"type": "object",
"description": "Mapping of classification values to human-readable labels. See \"classification\" field of \"externalAttribution\".",
"additionalProperties": true
Copy link
Contributor

@vasily-pozdnyakov vasily-pozdnyakov Feb 19, 2025

Choose a reason for hiding this comment

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

this is not necessary ("additionalProperties": true), at least in the exporter the schema validation fails with the property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read a bit more about JSON Schema: We could make this more restrictive by using patternProperties:

    "classifications": {
          ....
          "patternProperties": {
            "^[0-9]+$": {
              "type":"string"
            }
    }

What do you think about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to keep it simple for now. I also tried to test it with the pattern, and it does not work as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, let's proceed this way.

@vasily-pozdnyakov
Copy link
Contributor

I also generated a new .opossum file with the fields and it works fine with your changes.

@abraemer abraemer force-pushed the update-schema-with-license-classification branch from b2791ed to a2f8166 Compare February 20, 2025 12:00
@abraemer abraemer merged commit 1980189 into main Feb 20, 2025
5 checks passed
@abraemer abraemer deleted the update-schema-with-license-classification branch February 20, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants