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.

Sign up for GitHub

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

Jump to bottom

Ensure no duplicates in relationships when shortcut fields are used. #218

Merged
merged 4 commits into from
May 26, 2023
Merged

Ensure no duplicates in relationships when shortcut fields are used. #218

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b16c55
fix duplicate shorthand rels for 2.2
lumjjb May 23, 2023
3f743e1
fix duplicate shorthand rels for 2.3
lumjjb May 23, 2023
6b2ab7a
update yaml tests to ignore order of relationships
lumjjb May 24, 2023
fb0f8d8
address pr comments
lumjjb May 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions spdx/v2/v2_2/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ package v2_2

import (
"encoding/json"
"fmt"

"github.com/anchore/go-struct-converter"
converter "github.com/anchore/go-struct-converter"

"github.com/spdx/tools-golang/spdx/v2/common"
)
Expand Down Expand Up @@ -98,27 +99,46 @@ func (d *Document) UnmarshalJSON(b []byte) error {

*d = Document(d2)

relationshipExists := map[string]bool{}
serializeRel := func(r *Relationship) string {
return fmt.Sprintf("%v-%v->%v", common.RenderDocElementID(r.RefA), r.Relationship, common.RenderDocElementID(r.RefB))
}

// index current list of relationships to ensure no duplication
for _, r := range d.Relationships {
relationshipExists[serializeRel(r)] = true
}

kzantow marked this conversation as resolved.
Show resolved Hide resolved
// build relationships for documentDescribes field
for _, id := range e.DocumentDescribes {
d.Relationships = append(d.Relationships, &Relationship{
r := &Relationship{
RefA: common.DocElementID{
ElementRefID: d.SPDXIdentifier,
},
RefB: id,
Relationship: common.TypeRelationshipDescribe,
})
}

if !relationshipExists[serializeRel(r)] {
d.Relationships = append(d.Relationships, r)
relationshipExists[serializeRel(r)] = true
}
kzantow marked this conversation as resolved.
Show resolved Hide resolved
}

// build relationships for package hasFiles field
for _, p := range d.Packages {
for _, f := range p.hasFiles {
d.Relationships = append(d.Relationships, &Relationship{
r := &Relationship{
RefA: common.DocElementID{
ElementRefID: p.PackageSPDXIdentifier,
},
RefB: f,
Relationship: common.TypeRelationshipContains,
})
}
if !relationshipExists[serializeRel(r)] {
d.Relationships = append(d.Relationships, r)
relationshipExists[serializeRel(r)] = true
}
}

p.hasFiles = nil
Expand Down
10 changes: 10 additions & 0 deletions spdx/v2/v2_2/example/example.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,16 @@ var example = spdx.Document{
RefB: common.MakeDocElementID("", "Package"),
Relationship: "CONTAINS",
},
{
RefA: common.MakeDocElementID("", "Package"),
RefB: common.MakeDocElementID("", "CommonsLangSrc"),
Relationship: "CONTAINS",
},
{
RefA: common.MakeDocElementID("", "Package"),
RefB: common.MakeDocElementID("", "DoapSource"),
Relationship: "CONTAINS",
},
{
RefA: common.MakeDocElementID("", "DOCUMENT"),
RefB: common.MakeDocElementID("spdx-tool-1.2", "ToolsElement"),
Expand Down
177 changes: 142 additions & 35 deletions spdx/v2/v2_2/json/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package json

import (
"bytes"
jsonenc "encoding/json"
"fmt"
"os"
"strings"
Expand All @@ -21,35 +22,6 @@ import (

func TestLoad(t *testing.T) {
want := example.Copy()

want.Relationships = append(want.Relationships, []*spdx.Relationship{
{
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
RefB: common.DocElementID{ElementRefID: "File"},
Relationship: "DESCRIBES",
},
{
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
RefB: common.DocElementID{ElementRefID: "Package"},
Relationship: "DESCRIBES",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "CommonsLangSrc"},
Relationship: "CONTAINS",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "JenaLib"},
Relationship: "CONTAINS",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "DoapSource"},
Relationship: "CONTAINS",
},
}...)

file, err := os.Open("../../../../examples/sample-docs/json/SPDXJSONExample-v2.2.spdx.json")
if err != nil {
panic(fmt.Errorf("error opening File: %s", err))
Expand All @@ -62,8 +34,8 @@ func TestLoad(t *testing.T) {
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after parsing YAML example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after parsing JSON example: %s", diff)
return
}
}
Expand Down Expand Up @@ -91,8 +63,8 @@ func Test_Write(t *testing.T) {
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after writing and re-parsing JSON example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after writing and re-parsing JSON example: %s", diff)
return
}
}
Expand Down Expand Up @@ -149,7 +121,7 @@ func Test_ShorthandFields(t *testing.T) {
}
}

require.Equal(t, spdx.Document{
want := spdx.Document{
SPDXVersion: spdx.Version,
DataLicense: spdx.DataLicense,
SPDXIdentifier: "DOCUMENT",
Expand Down Expand Up @@ -200,7 +172,136 @@ func Test_ShorthandFields(t *testing.T) {
Relationship: common.TypeRelationshipContains,
},
},
}, doc)
}

if diff := cmp.Diff(want, doc, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after parsing JSON example: %s", cmp.Diff(want, doc, cmpopts.IgnoreUnexported(spdx.Package{})))
lumjjb marked this conversation as resolved.
Show resolved Hide resolved
return
}

}

func Test_ShorthandFieldsNoDuplicates(t *testing.T) {
contents := `{
"spdxVersion": "SPDX-2.2",
"dataLicense": "CC0-1.0",
"SPDXID": "SPDXRef-DOCUMENT",
"name": "SPDX-Tools-v2.0",
"documentDescribes": [
"SPDXRef-Container"
],
"packages": [
{
"name": "Container",
"SPDXID": "SPDXRef-Container"
},
{
"name": "Package-1",
"SPDXID": "SPDXRef-Package-1",
"versionInfo": "1.1.1",
"hasFiles": [
"SPDXRef-File-1",
"SPDXRef-File-2"
]
},
{
"name": "Package-2",
"SPDXID": "SPDXRef-Package-2",
"versionInfo": "2.2.2"
}
],
"files": [
{
"fileName": "./f1",
"SPDXID": "SPDXRef-File-1"
},
{
"fileName": "./f2",
"SPDXID": "SPDXRef-File-2"
}
],
"relationships": [
{
"spdxElementId": "SPDXRef-Package-1",
"relationshipType": "CONTAINS",
"relatedSpdxElement": "SPDXRef-File-1"
},
{
"spdxElementId": "SPDXRef-Package-1",
"relationshipType": "CONTAINS",
"relatedSpdxElement": "SPDXRef-File-2"
}
]
}`

doc := spdx.Document{}
err := json.ReadInto(strings.NewReader(contents), &doc)

require.NoError(t, err)

id := func(s string) common.DocElementID {
return common.DocElementID{
ElementRefID: common.ElementID(s),
}
}

want := spdx.Document{
SPDXVersion: spdx.Version,
DataLicense: spdx.DataLicense,
SPDXIdentifier: "DOCUMENT",
DocumentName: "SPDX-Tools-v2.0",
Packages: []*spdx.Package{
{
PackageName: "Container",
PackageSPDXIdentifier: "Container",
FilesAnalyzed: true,
},
{
PackageName: "Package-1",
PackageSPDXIdentifier: "Package-1",
PackageVersion: "1.1.1",
FilesAnalyzed: true,
},
{
PackageName: "Package-2",
PackageSPDXIdentifier: "Package-2",
PackageVersion: "2.2.2",
FilesAnalyzed: true,
},
},
Files: []*spdx.File{
{
FileName: "./f1",
FileSPDXIdentifier: "File-1",
},
{
FileName: "./f2",
FileSPDXIdentifier: "File-2",
},
},
Relationships: []*spdx.Relationship{
{
RefA: id("DOCUMENT"),
RefB: id("Container"),
Relationship: common.TypeRelationshipDescribe,
},
{
RefA: id("Package-1"),
RefB: id("File-1"),
Relationship: common.TypeRelationshipContains,
},
{
RefA: id("Package-1"),
RefB: id("File-2"),
Relationship: common.TypeRelationshipContains,
},
},
}

if diff := cmp.Diff(want, doc, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after parsing JSON example: %s", diff)
return
}
}

func Test_JsonEnums(t *testing.T) {
Expand Down Expand Up @@ -344,3 +445,9 @@ func Test_JsonEnums(t *testing.T) {
},
}, doc)
}

func relationshipLess(a, b *spdx.Relationship) bool {
aStr, _ := jsonenc.Marshal(a)
bStr, _ := jsonenc.Marshal(b)
return string(aStr) < string(bStr)
}
44 changes: 11 additions & 33 deletions spdx/v2/v2_2/yaml/yaml_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ package yaml

import (
"bytes"
jsonenc "encoding/json"
"fmt"
"os"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/spdx/tools-golang/spdx/v2/common"
spdx "github.com/spdx/tools-golang/spdx/v2/v2_2"
"github.com/spdx/tools-golang/spdx/v2/v2_2/example"
"github.com/spdx/tools-golang/yaml"
Expand All @@ -20,34 +20,6 @@ import (
func Test_Read(t *testing.T) {
want := example.Copy()

want.Relationships = append(want.Relationships, []*spdx.Relationship{
{
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
RefB: common.DocElementID{ElementRefID: "File"},
Relationship: "DESCRIBES",
},
{
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
RefB: common.DocElementID{ElementRefID: "Package"},
Relationship: "DESCRIBES",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "CommonsLangSrc"},
Relationship: "CONTAINS",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "JenaLib"},
Relationship: "CONTAINS",
},
{
RefA: common.DocElementID{ElementRefID: "Package"},
RefB: common.DocElementID{ElementRefID: "DoapSource"},
Relationship: "CONTAINS",
},
}...)

file, err := os.Open("../../../../examples/sample-docs/yaml/SPDXYAMLExample-2.2.spdx.yaml")
if err != nil {
panic(fmt.Errorf("error opening File: %s", err))
Expand All @@ -60,8 +32,8 @@ func Test_Read(t *testing.T) {
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after parsing YAML example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after parsing YAML example: %s", diff)
return
}
}
Expand All @@ -88,8 +60,14 @@ func Test_Write(t *testing.T) {
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after writing and re-parsing YAML example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
t.Errorf("got incorrect struct after writing and re-parsing YAML example: %s", diff)
return
}
}

func relationshipLess(a, b *spdx.Relationship) bool {
aStr, _ := jsonenc.Marshal(a)
bStr, _ := jsonenc.Marshal(b)
return string(aStr) < string(bStr)
}
Loading