tools-golang
tools-golang copied to clipboard
[Optional Upgrade] SpdxItem and SpdxElement
As evident from the UML diagram, classes like Package, File, Snippets, etc derive some of their fields/properties from an abstract class called SpdxItem.
Currently, each of the subclasses defines each of those properties in their own structs. I suggest pulling out the common fields and create a new struct type "SpdxItem" which will hold those common attributes. The subclasses can then use composition to include those fields in their own definition. This will allow the model to have less entry points for common fields allowing
Similar is the argument for SpdxElement
Sample Working:
type SpdxItem struct {
licenseConcluded AnyLicenseInfo
licenseInfoFromFiles SimpleLicenseInfo
copyrightText String
licenseComments String
}
type Package struct {
SpdxItem
summary String
description String
// ...other fields...
}
type File struct {
SpdxItem
fileType FileType
checksum Checksum
// ...other fields...
}
This is an interesting thought, @RishabhBhatnagar. For the future SPDX 3.0 spec, when we implement that in the Golang tools, I definitely agree that we should more closely follow the model.
For the implementation of the current versions 2.1 and 2.2, I am less sure but could be convinced. Would this make it easier to interchange with the RDF parser / writer that you've been working on? If so, I'd definitely be interested in making this change.
With the change you're proposing here, would any of the existing tag-value parsing code need to change? If I understand correctly, I'm assuming not -- the fields from SpdxItem would just get populated into Package, File and Snippet with the same names, so in theory no other code changes should be needed. Is that correct?
One thing to note is that I'm not sure that model diagram is entirely correct, at least about the "licenseInfoFromFiles" field. There is a field similar to this in Snippets, for example, but it does not have this name. So I don't think that would be one to pull up into SpdxItem.
For the implementation of the current versions 2.1 and 2.2, I am less sure but could be convinced. Would this make it easier to interchange with the RDF parser / writer that you've been working on? If so, I'd definitely be interested in making this change.
It doesn't really affect any of the parser or writer (be it rdf or tag-value).
With the change you're proposing here, would any of the existing tag-value parsing code need to change? If I understand correctly, I'm assuming not -- the fields from SpdxItem would just get populated into Package, File and Snippet with the same names, so in theory no other code changes should be needed. Is that correct?
Yes. You are 100% correct. We won't be required to change any code.
One thing to note is that I'm not sure that model diagram is entirely correct, at least about the "licenseInfoFromFiles" field. There is a field similar to this in Snippets, for example, but it does not have this name. So I don't think that would be one to pull up into SpdxItem.
Correct me if I am wrong. licenseInfoInSnippet is the field which is similar to licenseInfoFromFiles that you're talking about. I've been referring to the rdf spec document all the time. Even in that document, we can see Snippets deriving properties from SpdxItem class. It is quite possible that Snippet might not use the licenseInfoFromFiles property and use licenseInfoInSnippet as it's alternative.
My thoughts:
The following are the ways to resolve the issue ambiguity with Snippet.
- not to include the licenseInfoInFiles field in the SpdxItem and let Snippet use licenseInfoInSnippet as it's alternative.
- include the licenseInfoInFiles field in the SpdxItem and let Snippet declare another property for licenseInfoInSnippet. (this will effectively leave licenseInfoInFiles as a redundant field in the Snippet class).
first way will reduce redundancy and second way will let the tools-golang model be close to the actual data model defined by the uml and the rdf spec.
I'm okay with either of the approaches. I'm inclined more to second way as it will stick to the documentation. @swinslow, let me know which approach are you more comfortable with (first, second or scrap the idea).
@RishabhBhatnagar regarding your thoughts on SpdxItem to make it more closer to spdx-spec model, I like them :+1: . If we need this, extending the thoughts you mentioned, can we go for an extra layer of abstraction on top of SpdxItem (let's assume it's the grandparent here), though it's slightly deviating from the UML diagram?
Instead of directly deriving from SpdxItem, File & Snippet structs derive their props (embedding) from SpdxItemFile & SpdxItemSnippet (two direct child struct of SpdxItem with customised fields specific to File & Snippet).
Would you mind if I work on this issue for SpdxItem as well as SpdxElement? Thank you :)
Marking this as part of 0.4.0, so we can consider it together with some other potential API changes for that release.