feat: add base document db annotation, WithSourceDocumentAnnotations and WithRevisedDocumentAnnotations functions #211
Conversation
|
From a design perspective, to reduce duplication, how would you feel about this?
func (backend *Backend) AddDocument(sbomData []byte, ...annotations *ent.Annotation) (*sbom.Document, error) {
// create ent.BackendOptions with the annotations slice
// call backend.Store
}
func (backend *Backend) AddSourceDocument(sbomData []byte) (*sbom.Document, error) {
// create slice of format, hash, and data annotations
// call backend.AddDocument(sbomData, annotations...)
}
func (backend *Backend) CreateBaseDocumentAnnotation(document *sbom.Document) (*ent.Annotation, error) {
baseUUID, err := ent.GenerateUUID(base)
if err != nil {
return nil, fmt.Errorf("%w", err)
}
return &ent.Annotation{
Name: BaseDocumentAnnotation,
Value: string(baseUUID),
IsUnique: true,
}
} |
I like this. I tried to do something similar with Interfaces (any) initially, but I felt like it made it messy and harder to follow so I went with separate functions. I'll make the switch to follow your example |
Running through the logistics of your suggestion. Were you thinking the creation of a 'revised' document would look something like: revision := sbom.NewDocument() // or whatever
baseDocAntn, err := be.CreateBaseDocumentAnnotation(base)
// check err
_, err = be.AddDocument(revision, baseDocAntn)
// check err
err = be.UpdateAliasReference(revision, base)
// check errOne of reasons I thought revision := sbom.NewDocument() // or whatever
_, err := be. AddDocumentRevision(revision, base)
// check errAnd internal to Also reading ahead (AKA I already have this code written, minus the updates from this conversation), when we add a 'latest' document annotation we'll have another function we have to call (to move the latest annotation to the revised doc) when we create a revised document that could be contained in Not sure how many places we'll be eventually creating document revisions, but I'm assuming it'd be in more than one place. Could also switch it up to |
dd0f792 to
d6a8731
Compare
Minder Vulnerability Report ✅Minder analyzed this PR and found it does not add any new vulnerable dependencies.
|
…ionDocument methods
ea95aa8 to
dfc4ba1
Compare
…d-define-base-document-database-uuid-annotation
|
Documenting discussion of this PR in an earlier call: As a future effort, it might make sense to implement a functional options pattern for passing in a set of annotations to func WithSourceDataAnnotation(sbomData []byte) Option {
return func(backend *Backend) {
backend.Annotations = append(backend.Annotations, ent.Annotations{
{
Name: SourceDataAnnotation,
Value: string(sbomData),
IsUnique: true,
},
})
}
}or a combination of predefined annotations for different scenarios |
…d-define-base-document-database-uuid-annotation
chore: comment clarifications Signed-off-by: Allen Shearin <allen.p.shearin@gmail.com>
|
I believe for this commit, leaving it unset will cause the protobom/storage schema to randomly generate one, which won't match any document IDs in the database |
What would I set it to in this situation? Would they all be the new/revised document uuid? |
Should be From protobom/storage // Set each annotation's document ID if not specified.
for _, a := range annotations {
if a.DocumentID == uuid.Nil {
a.DocumentID = id
}
} |
f960596 to
b5ed14f
Compare
2ed7675 to
a079191
Compare
Description
Adds an annotation for storing the uuid of the 'base' document when creating a new, modified document from one imported/fetched into the cache. In the added function the new document is called a revision.
All names are open to suggestions if you feel they don't capture the intent properly.
Also added a AddDocumentRevision function that takes in two sbom documents, the base document and the newly created revision document and setting appropriate annotations, including the base document uuid annotation, and moves the alias 'pointer' to the latest revision of the document, if an alias is present on the original.
Fixes #203
Related to #81
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist