feat: schema based validation may return iterable of all errors

[e3krisztian]

This PR addresses two issues (at the commit level they are separately implemented):

• improves validation by enriching the ValidationError with the message and path properties, and sensibly limit (shorten) error messages in size via ValidationError.get_squeezed_message() (Validation errors are hard to present safely to the user (missing abstraction) #827)
• extends SchemabasedValidator with iterate_errors() to allow access to all the validation problems (feat: extend validation interface with new method to iterate over all the problems #828)

[jkowalleck]

thanks for working on this feature.

to me, the current implementation looks odd and not very pythonic.

Instead of the current one, I have a design proposal:
add an optional parameter to validate_str that controls the return type a bit.
Something like def validate_str(self, data: str, *, all_errors: boolean=False) -> None | ValidationError | Iterable[ValidationError]:
The implementation would return None if there were no errors at all.
The implementation would return ValidationError if there was at least one error and all_errors is False.
The implementation would return Iterable[ValidationError] if there was at least one error and all_errors is True.

after the implementation is done, we would add proper type hints for each of the implementations with @overload.

what do you think?

[jkowalleck]

thanks for working on this feature.

to me, the current implementation looks odd and not very pythonic.

Instead of the current one, I have a design proposal: add an optional parameter to validate_str that controls the return type a bit. Something like def validate_str(self, data: str, *, all_errors: boolean=False) -> None | ValidationError | Iterable[ValidationError]: The implementation would return None if there were no errors at all. The implementation would return ValidationError if there was at least one error and all_errors is False. The implementation would return Iterable[ValidationError] if there was at least one error and all_errors is True.

after the implementation is done, we would add proper type hints for each of the implementations with @overload.

what do you think?

I am working on the proposed implementation design, and will pullrequest it to your branch once i am done.

PS: pullrequested the proposed changes

• see feat: SchemabasedValidator.validate_str can return an iterator over all errors e3krisztian/cyclonedx-python-lib#1

[jkowalleck]

i see this PR is mixing scopes.

1. it makes validation return an iterable - almost done
2. it adds additional capabilities to the error class - still in discussion.

i like the iterable feature - it brings immediately value for downstream users.
I see the additional capabilities debatable.

this means: we have a mix of scopes, and each scope is potentially moving in a different pace - from a developer/maintainer's perspective.

Therefore, @e3krisztian , I'd ask to remove all the additional error capabilities from this PR, and open a different PR for them.
This enables us to release the "iterable" feature soon, while we discuss/refine the feature of additional error class capabilities.

A quick outlook: i could imagine we might find, in our discussion, that the whole error class shall be redesigned - which would be a breaking change I'd welcome.

[e3krisztian]

@jkowalleck I appreciate the quick reviews and responses!

You are probably right, that this PR had attempted to change too much.

Unfortunately we clearly have a different view of a library. For me it includes providing guarantees, and being responsible for not leaking the underlying implementation abstractions (which here in the XML case is not even a public one!), and if necessary working around/covering/or making explicit the problems in them.

This PR attempted to somewhat address what was missing in my view around the validation interface, but it is clear, that our views of what needs to be abstracted away from the underlying implementations differ, as well as our view on what is "Pythonic code", and thus I find it very hard, and expensive to continue discussing it at code level.

Since the iterate over errors is your implementation, and above you are asking me to remove all the rest, I ask you to take over this PR, and remove or keep whatever you feel right, or close it and open one for your error iteration implementation.

We can continue the discussions about the error interfaces in the original issues, but I no longer intend to open PR-s implementing them.

[jkowalleck]

@jkowalleck I appreciate the quick reviews and responses!

You are probably right, that this PR had attempted to change too much.

Unfortunately we clearly have a different view of a library. For me it includes providing guarantees, and being responsible for not leaking the underlying implementation abstractions (which here in the XML case is not even a public one!), and if necessary working around/covering/or making explicit the problems in them.

Nope. no different views - I share your opinion.
This is why I ask to split the PR, so the two scopes can be handled in their respective pace.
I do not want to lose your ideas of improving the error class. The error class was nothing people cared about much in the past - that's why it is so sloppy today. I appreciate your ideas.

This PR attempted to somewhat address what was missing in my view around the validation interface, but it is clear, that our views of what needs to be abstracted away from the underlying implementations differ, as well as our view on what is "Pythonic code", and thus I find it very hard, and expensive to continue discussing it at code level.

Since the iterate over errors is your implementation, and above you are asking me to remove all the rest, I ask you to take over this PR, and remove or keep whatever you feel right, or close it and open one for your error iteration implementation.

We can continue the discussions about the error interfaces in the original issues, but I no longer intend to open PR-s implementing them.

I see. thats why requirements engineering is usually done in tickets (#827), not PRs ;-)

i will remove the error class changes from this very PR, and probably merge/release the rest soon.
This way we have access to all underlying errors sooner than the more complex improvements on the error class.

Afterwards, I will continue the discussion for error class improvements in #827 - which already has this scope of improving the error class to make it valuable and independent of 3rd party implementations. More details in the tickets soon - your opinion is appreciated very much!

PS: removed debatable parts and started/continued discussion in #827 (comment)

[jkowalleck]

this feature was released via https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v10.3.0