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: support cyclonedx 1.5 - ValidationResult management #2

Merged

Conversation

mrizzi
Copy link

@mrizzi mrizzi commented Sep 4, 2024

It looks like the changes in CycloneDX/cyclonedx-rust-cargo#639 to the validation error management introduced a hierarchy of errors through ValidationErrorsKind:

  • ValidationErrorsKind::Struct and ValidationErrorsKind::List refer to further inner (and hence nested) ValidationResult
  • ValidationErrorsKind::Field, ValidationErrorsKind::Enum and ValidationErrorsKind::Custom refer to the "final" validation error, i.e. ValidationError

Previously, in case of validation failure, i.e. ValidationResult::Failed, the failure's reasons were in a flat Vec structure containing FailureReason that allowed us to easily run through them for implementing the workaround for CycloneDX/cyclonedx-rust-cargo#737

This change in the validation errors management explains why the test parse_cyclonedx_valid_14_newline failed in PR-1721.
In fact the ValidationResult that came out of validation is something like:

ValidationResult { inner: {"components": Struct(ValidationResult { inner: {"inner": List({27: ValidationResult { inner: {"licenses": Struct(ValidationResult { inner: {"inner": List({0: ValidationResult { inner: {"license": Struct(ValidationResult { inner: {"license_identifier": Struct(ValidationResult { inner: {"Name": Enum(ValidationError { message: "NormalizedString contains invalid characters \\r \\n \\t or \\r\\n" })} })} })} }})} })} }})} })} }

In this PR I've added a recursive approach to traverse any ValidationResult in order to collect all of the distinct error messages.

With this fix, the tests are fine with no changes (also the parse_cyclonedx_invalid_serial_number one that failed on PR-1721 as well):

$ cargo test -p bombastic-model 
    Finished test [unoptimized + debuginfo] target(s) in 0.23s
     Running unittests src/lib.rs (target/debug/deps/bombastic_model-a5df717e66ea8ca4)

running 5 tests
test data::tests::parse_cyclonedx_without_serial_number ... ok
test data::tests::parse_cyclonedx_invalid_serial_number ... ok
test data::tests::parse_cyclonedx_valid_14 ... ok
test data::tests::parse_cyclonedx_valid_14_newline ... ok
test data::tests::parse_cyclonedx_valid_13 ... ok

Copy link

@dejanb dejanb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@mrizzi mrizzi merged commit 0fa3e59 into ctron:feature/cyclonedx_15_1 Sep 5, 2024
@mrizzi mrizzi deleted the ctron-feature/cyclonedx_15_1 branch September 5, 2024 08:59
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.

2 participants