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: prototype 3.0 model #247

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

kzantow
Copy link
Collaborator

@kzantow kzantow commented Aug 1, 2024

This PR implements one possible data model to support SPDX 3, along with functional JSON serialization / deserialization. There are sure to be some rough edges!

The easiest way to understand what usage looks like is to check out the tests -- they perform some basic document creation, along with serializing, deserializing, and re-serializing to verify the documents are the same.

A few rough edges with SPDX 3 in general seem to be:

  • all elements are required to have a creation Info; this PR includes a feature during JSON serialization to set all elements creation info if it is unset, I don't know if this is a great idea but it would make things a lot easier to deal with...
  • all SPDX elements must have an spdxId; I also added a way to do this prior to serialization
  • it could be possible to have multiple SPDX document objects in the same json document
  • having an SPDX document and an SBOM independently is somewhat confusing
  • there's a chicken-and-egg problem between creation info and agent, where both are required and both reference each other, so some helper as I've implemented here would probably be good; maybe it should take different information

This PR does not add anything in the way of data validation, so it is probably pretty easy to create invalid documents. How to handle this is TBD.

This PR also does not add any conversion functionality to/from older versions of SPDX.

Feedback requested!

@kzantow kzantow force-pushed the feat/spdx-3-prototype branch 2 times, most recently from b71353a to 936ae58 Compare August 1, 2024 06:32
@kzantow kzantow changed the title feat: prototype v3_0 model feat: prototype 3.0 model Aug 1, 2024
@kzantow kzantow force-pushed the feat/spdx-3-prototype branch 2 times, most recently from ea44d2b to 14db0d0 Compare August 1, 2024 16:05
Signed-off-by: Keith Zantow <kzantow@gmail.com>
@kzantow kzantow force-pushed the feat/spdx-3-prototype branch from 14db0d0 to 8e9125f Compare August 2, 2024 19:49
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Comment on lines +43 to +45
pkg2 := &spdx.AiAIPackage{}
pkg2.Name = "some-package-2"
pkg2.SoftwarePackageVersion = "2.4.5"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But it's also possible to avoid the nesting by accessing properties like this.

Comment on lines +34 to +39
pkg1 := &spdx.SoftwarePackage{
SoftwareSoftwareArtifact: spdx.SoftwareSoftwareArtifact{Artifact: spdx.Artifact{Element: spdx.Element{
Name: "some-package-1",
}}},
SoftwarePackageVersion: "1.2.3",
}
Copy link
Collaborator Author

@kzantow kzantow Jan 29, 2025

Choose a reason for hiding this comment

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

I'm just going to call out the two least desirable traits of this proposal: moderately deep inheritance heirarchies with embedding are not especially fun to work with in initializers (the other proposal/implementation also uses embedding), since you must nest embedded types like this to set the "parent" type properties.

for _, sbom := range doc.RootElements.SoftwareSbomIter() {
for _, rel := range sbom.RootElements.RelationshipIter() {
if rel.RelationshipType == spdx.RelationshipType_Contains {
spdx.As(rel.From, func(f *spdx.SoftwareFile) {
Copy link
Collaborator Author

@kzantow kzantow Jan 29, 2025

Choose a reason for hiding this comment

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

The second thing is accessing single-instance properties like this. There is no magic typing as is done with the specialized collections that could be done to add methods like rel.From.SoftwareFile(), for example. This means it's required to type assert... but because of the inheritance, you cannot type assert to *spdx.SoftwareFile, or you'll miss any subtypes, so the spdx.As function is a helper that simply switches on the type provided -- in this case *spdx.SoftwareFile switches to test for spdx.AnySoftwareFile interface, and if found calls .asSoftwareFile() to get a reference. This allows the inheritance to work properly at the expense of not really being able to do anything idiomatic here.

Signed-off-by: Keith Zantow <kzantow@gmail.com>
if rel.RelationshipType != spdx.RelationshipType_Contains {
continue
}
_ = spdx.As(rel.From, func(f *spdx.SoftwareFile) any {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The second thing is accessing single-instance properties like this. There is no magic typing as is done with the specialized collections that could be done to add methods like rel.From.SoftwareFile(), for example. This means it's required to type assert... but because of the inheritance, you cannot type assert to *spdx.SoftwareFile, or you'll miss any subtypes, so the spdx.As function is a helper that simply switches on the type provided -- in this case *spdx.SoftwareFile switches to test for spdx.AnySoftwareFile interface, and if found calls .asSoftwareFile() to get a reference. This allows the inheritance to work properly at the expense of not really being able to do anything idiomatic here.

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