Skip to content

Conversation

indiVar0508
Copy link
Contributor

@indiVar0508 indiVar0508 commented Feb 18, 2025

the following classes' init no longer raise NoPropertiesProvidedException:

  • cyclonedx.model.IdentifiableAction
  • cyclonedx.model.component.Commit
  • cyclonedx.model.component.ComponentEvidence
  • cyclonedx.model.component.Diff
  • cyclonedx.model.component.Pedigree
  • cyclonedx.model.issue.IssueTypeSource
  • cyclonedx.model.vulnerability.VulnerabilityAnalysis
  • cyclonedx.model.vulnerability.VulnerabilityCredits
  • cyclonedx.model.vulnerability.VulnerabilityRating
  • cyclonedx.model.vulnerability.VulnerabilitySource

did a global search and found these file/classes
raising this exception

File - Class
model/init.py - IdentifiableAction
model/component.py - Commit
model/component.py - ComponentEvidence
model/component.py - Diff
model/component.py - Pedigree
model/issue.py - IssueTypeSource
model/model.py - BomTargetVersionRange
model/model.py - VulnerabilityAnalysis
model/model.py - VulnerabilitySource
model/model.py - VulnerabilityReference
model/model.py - VulnerabilityRating
model/model.py - VulnerabilityCredits

fixes #776

@jkowalleck jkowalleck added the enhancement New feature or request label Feb 18, 2025
@jkowalleck
Copy link
Member

jkowalleck commented Feb 18, 2025

did a global search and found these file/classes
raising this exception

regarding your findings,
please read the according specification/standard carefully, to find out whether it is actually allowed to craft empty objects.
latest document found here: https://ecma-international.org/publications-and-standards/standards/ecma-424/

Thank you in advance for all your effort 👍

@jkowalleck jkowalleck changed the title feat: remove NoPropertiesProvidedException [WIP] feat: remove NoPropertiesProvidedException Feb 18, 2025
@jkowalleck jkowalleck changed the title [WIP] feat: remove NoPropertiesProvidedException [WIP] feat: no longer thow NoPropertiesProvidedException Feb 18, 2025
@indiVar0508
Copy link
Contributor Author

regarding your findings, please read the according specification/standard carefully, to find out whether it is actually allowed to craft empty objects. latest document found here: https://ecma-international.org/publications-and-standards/standards/ecma-424/

Thank you in advance for all your effort 👍

I found in class docstring where it seems to mention the spec URL, i followed that and my assessments seems to suggest following

File Class Doc Comment
model/init.py IdentifiableAction Link Yes, Can Remove
model/component.py Commit Link Yes, Can Remove
model/component.py ComponentEvidence Link 1 issue, attributes not updated since 1.5 code has TODO, but everythin is optional so can do?
model/component.py Diff link Yes, Can Remove
model/component.py Pedigree link Yes, Can Remove
model/issue.py IssueTypeSource link Yes, Can remove
model/vulnerability.py BomTargetVersionRange link This prolly can not be empty if initialized
model/vulnerability.py VulnerabilityAnalysis link Yes, Can Remove. But two keys missing firstIssued and lastUpdated
model/vulnerability.py VulnerabilitySource link Yes, Can remove
model/vulnerability.py VulnerabilityReference link Both are necessary its not either attribute, currently check asks for either which might be wrong?
model/vulnerability.py VulnerabilityRating link Yes, Can remove
model/vulnerability.py VulnerabilityCredits link Yes, Can Remove

is this ok?

@jkowalleck
Copy link
Member

jkowalleck commented Feb 19, 2025

re: #786 (comment)

is this ok?


ComponentEvidence - 1 issue, attributes not updated since 1.5 code has TODO, but everythin is optional so can do?

Yes, can do. there are no mandatory attributes/properties.


VulnerabilityReference - Both are necessary its not either attribute, currently check asks for either which might be wrong?

You are right, this should require both. Nice find!
Lets keep the current behavior as is.

What needs to be changed here is to make both attributes/properties non-optional and to have no default value, and remove the now unnecessary raise. This would be a change to public API - a breaking change - which I could see in the upcoming v9.0 release.
Could you create a ticket that for this?


@indiVar0508
Copy link
Contributor Author

Hi Apologies for delay, i got unplanned travel plans over weekend so couldn't continue on this, will continue on this from this week..

I have created a separate issue for mandatory field #790

What are your thoughts about BomTargetVersionRange, this has two behaviour in it, it can be a list as well as individual version, as per XML list can be empty but if we define Version attributes should be specified

did a global search and found these file/classes
raising this exception

| File                   | Class                  | Doc                                                                                            | Comment                                                                                            |
| ---------------------- | ---------------------- | ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------- |
| model/**init**.py      | IdentifiableAction     | [Link](https://cyclonedx.org/docs/1.6/xml/#type_identifiableActionType-doc-panel-collapse)     | Yes, Can Remove                                                                                    |
| model/component.py     | Commit                 | [Link](https://cyclonedx.org/docs/1.6/xml/#type_commitType-instance-table-collapse)            | Yes, Can Remove                                                                                    |
| model/component.py     | ComponentEvidence      | [Link](https://cyclonedx.org/docs/1.6/xml/#type_componentEvidenceType-instance-table-collapse) | 1 issue, attributes not updated since 1.5 code has TODO, but everythin is optional so can do?      |
| model/component.py     | Diff                   | [link](https://cyclonedx.org/docs/1.6/xml/#type_diffType-instance-table-collapse)              | Yes, Can Remove                                                                                    |
| model/component.py     | Pedigree               | [link](https://cyclonedx.org/docs/1.6/xml/#type_pedigreeType-instance-table-collapse)          | Yes, Can Remove                                                                                    |
| model/issue.py         | IssueTypeSource        | [link](https://cyclonedx.org/docs/1.6/xml/#type_issueType-instance-table-collapse)             | Yes, Can remove                                                                                    |
| model/vulnerability.py | BomTargetVersionRange  | [link](https://cyclonedx.org/docs/1.6/xml/#type_vulnerabilityType)                             | This prolly can not be empty if initialized                                                        |
| model/vulnerability.py | VulnerabilityAnalysis  | [link](https://cyclonedx.org/docs/1.6/xml/#type_vulnerabilityType)                             | Yes, Can Remove. But two keys missing `firstIssued` and `lastUpdated`                              |
| model/vulnerability.py | VulnerabilitySource    | [link](https://cyclonedx.org/docs/1.6/xml/#type_vulnerabilitySourceType)                       | Yes, Can remove                                                                                    |
| model/vulnerability.py | VulnerabilityReference | [link](https://cyclonedx.org/docs/1.6/xml/#type_vulnerabilityType)                             | Both are necessary its not either attribute, currently check asks for either which might be wrong? |
| model/vulnerability.py | VulnerabilityRating    | [link](https://cyclonedx.org/docs/1.6/xml/#type_ratingType-instance-table-collapse)            | Yes, Can remove                                                                                    |
| model/vulnerability.py | VulnerabilityCredits   | [link](https://cyclonedx.org/docs/1.6/xml/#type_vulnerabilityType)                             | Yes, Can Remove                                                                                    |
Signed-off-by: Indivar Mishra <indimishra@gmail.com>
found few classes having incorrect url in vulnerability module while
going through them

Signed-off-by: Indivar Mishra <indimishra@gmail.com>
@indiVar0508
Copy link
Contributor Author

hey @jkowalleck

I updated all of them as per above mentioned table with exception of following two

  1. BomTargetVersionRange : I am not sure if it would be straight forward to do it
  2. VulnerabilityReference: we have created a new issue for this

on side note should VulnerabilityAnalysis this also have two new attributes i.e. first_issued & last_issued in accordance to schema documentation

test snapshot

Files skipped (0):
  flake8: OK (2.22=setup[0.02]+cmd[0.00,0.72,0.49,0.98] seconds)
  mypy-current: OK (1.61=setup[0.01]+cmd[0.00,0.75,0.48,0.38] seconds)
  mypy-lowest: OK (1.78=setup[0.01]+cmd[0.00,0.84,0.54,0.39] seconds)
  py313-allExtras: SKIP (0.11 seconds)
  py313-noExtras: SKIP (0.15 seconds)
  py312-allExtras: SKIP (0.30 seconds)
  py312-noExtras: SKIP (0.12 seconds)
  py311-allExtras: SKIP (0.12 seconds)
  py311-noExtras: SKIP (0.11 seconds)
  py310-allExtras: OK (7.98=setup[0.01]+cmd[0.00,0.59,0.49,6.89] seconds)
  py310-noExtras: OK (6.65=setup[0.01]+cmd[0.00,0.72,0.47,5.46] seconds)
  py39-allExtras: SKIP (0.21 seconds)
  py39-noExtras: SKIP (0.10 seconds)
  py38-allExtras: SKIP (0.11 seconds)
  py38-noExtras: SKIP (0.11 seconds)
  bandit: OK (2.41=setup[0.01]+cmd[0.00,0.70,0.46,1.25] seconds)
  congratulations :) (24.13 seconds)

output of CDX_TEST_RECREATE_SNAPSHOTS=1 poetry run tox -e py

----------------------------------------------------------------------
Ran 5243 tests in 5.931s

OK
  py: OK (10.90=setup[0.02]+cmd[0.00,3.50,0.52,6.86] seconds)
  congratulations :) (10.94 seconds)

@indiVar0508 indiVar0508 marked this pull request as ready for review February 24, 2025 17:54
@indiVar0508 indiVar0508 requested a review from a team as a code owner February 24, 2025 17:54
@indiVar0508 indiVar0508 changed the title [WIP] feat: no longer thow NoPropertiesProvidedException feat: avoid raising NoPropertiesProvidedException for optional parameters Feb 24, 2025
@jkowalleck
Copy link
Member

on side note should VulnerabilityAnalysis this also have two new attributes i.e. first_issued & last_issued in accordance to schema documentation

created a ticket for this:

@jkowalleck
Copy link
Member

jkowalleck commented Feb 25, 2025

regarding BomTargetVersionRange:
lets leave it as is. the spec says it requires either a version or a range - via JSON schema's oneOf and XSD's choice

@jkowalleck jkowalleck merged commit 845b8d5 into CycloneDX:main Feb 25, 2025
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: no longer throw unexpected NoPropertiesProvidedException
2 participants