Skip to content

Commit e939624

Browse files
lumjjblumjjb
authored
Merge pull request #218 from spdx/fix-shorthand-dupls
Ensure no duplicates in relationships when shortcut fields are used.
2 parents 00864c0 + fb0f8d8 commit e939624

File tree

7 files changed

+365
-87
lines changed

7 files changed

+365
-87
lines changed

‎spdx/v2/v2_2/document.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ package v2_2
55

66
import (
77
"encoding/json"
8+
"fmt"
89

9-
"github.com/anchore/go-struct-converter"
10+
converter "github.com/anchore/go-struct-converter"
1011

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

99100
*d = Document(d2)
100101

102+
relationshipExists := map[string]bool{}
103+
serializeRel := func(r *Relationship) string {
104+
return fmt.Sprintf("%v-%v->%v", common.RenderDocElementID(r.RefA), r.Relationship, common.RenderDocElementID(r.RefB))
105+
}
106+
107+
// index current list of relationships to ensure no duplication
108+
for _, r := range d.Relationships {
109+
relationshipExists[serializeRel(r)] = true
110+
}
111+
101112
// build relationships for documentDescribes field
102113
for _, id := range e.DocumentDescribes {
103-
d.Relationships = append(d.Relationships, &Relationship{
114+
r := &Relationship{
104115
RefA: common.DocElementID{
105116
ElementRefID: d.SPDXIdentifier,
106117
},
107118
RefB: id,
108119
Relationship: common.TypeRelationshipDescribe,
109-
})
120+
}
121+
122+
if !relationshipExists[serializeRel(r)] {
123+
d.Relationships = append(d.Relationships, r)
124+
relationshipExists[serializeRel(r)] = true
125+
}
110126
}
111127

112128
// build relationships for package hasFiles field
113129
for _, p := range d.Packages {
114130
for _, f := range p.hasFiles {
115-
d.Relationships = append(d.Relationships, &Relationship{
131+
r := &Relationship{
116132
RefA: common.DocElementID{
117133
ElementRefID: p.PackageSPDXIdentifier,
118134
},
119135
RefB: f,
120136
Relationship: common.TypeRelationshipContains,
121-
})
137+
}
138+
if !relationshipExists[serializeRel(r)] {
139+
d.Relationships = append(d.Relationships, r)
140+
relationshipExists[serializeRel(r)] = true
141+
}
122142
}
123143

124144
p.hasFiles = nil

‎spdx/v2/v2_2/example/example.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,16 @@ var example = spdx.Document{
373373
RefB: common.MakeDocElementID("", "Package"),
374374
Relationship: "CONTAINS",
375375
},
376+
{
377+
RefA: common.MakeDocElementID("", "Package"),
378+
RefB: common.MakeDocElementID("", "CommonsLangSrc"),
379+
Relationship: "CONTAINS",
380+
},
381+
{
382+
RefA: common.MakeDocElementID("", "Package"),
383+
RefB: common.MakeDocElementID("", "DoapSource"),
384+
Relationship: "CONTAINS",
385+
},
376386
{
377387
RefA: common.MakeDocElementID("", "DOCUMENT"),
378388
RefB: common.MakeDocElementID("spdx-tool-1.2", "ToolsElement"),

‎spdx/v2/v2_2/json/json_test.go

Lines changed: 142 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package json
44

55
import (
66
"bytes"
7+
jsonenc "encoding/json"
78
"fmt"
89
"os"
910
"strings"
@@ -21,35 +22,6 @@ import (
2122

2223
func TestLoad(t *testing.T) {
2324
want := example.Copy()
24-
25-
want.Relationships = append(want.Relationships, []*spdx.Relationship{
26-
{
27-
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
28-
RefB: common.DocElementID{ElementRefID: "File"},
29-
Relationship: "DESCRIBES",
30-
},
31-
{
32-
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
33-
RefB: common.DocElementID{ElementRefID: "Package"},
34-
Relationship: "DESCRIBES",
35-
},
36-
{
37-
RefA: common.DocElementID{ElementRefID: "Package"},
38-
RefB: common.DocElementID{ElementRefID: "CommonsLangSrc"},
39-
Relationship: "CONTAINS",
40-
},
41-
{
42-
RefA: common.DocElementID{ElementRefID: "Package"},
43-
RefB: common.DocElementID{ElementRefID: "JenaLib"},
44-
Relationship: "CONTAINS",
45-
},
46-
{
47-
RefA: common.DocElementID{ElementRefID: "Package"},
48-
RefB: common.DocElementID{ElementRefID: "DoapSource"},
49-
Relationship: "CONTAINS",
50-
},
51-
}...)
52-
5325
file, err := os.Open("../../../../examples/sample-docs/json/SPDXJSONExample-v2.2.spdx.json")
5426
if err != nil {
5527
panic(fmt.Errorf("error opening File: %s", err))
@@ -62,8 +34,8 @@ func TestLoad(t *testing.T) {
6234
return
6335
}
6436

65-
if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
66-
t.Errorf("got incorrect struct after parsing YAML example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
37+
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
38+
t.Errorf("got incorrect struct after parsing JSON example: %s", diff)
6739
return
6840
}
6941
}
@@ -91,8 +63,8 @@ func Test_Write(t *testing.T) {
9163
return
9264
}
9365

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

152-
require.Equal(t, spdx.Document{
124+
want := spdx.Document{
153125
SPDXVersion: spdx.Version,
154126
DataLicense: spdx.DataLicense,
155127
SPDXIdentifier: "DOCUMENT",
@@ -200,7 +172,136 @@ func Test_ShorthandFields(t *testing.T) {
200172
Relationship: common.TypeRelationshipContains,
201173
},
202174
},
203-
}, doc)
175+
}
176+
177+
if diff := cmp.Diff(want, doc, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
178+
t.Errorf("got incorrect struct after parsing JSON example: %s", diff)
179+
return
180+
}
181+
182+
}
183+
184+
func Test_ShorthandFieldsNoDuplicates(t *testing.T) {
185+
contents := `{
186+
"spdxVersion": "SPDX-2.2",
187+
"dataLicense": "CC0-1.0",
188+
"SPDXID": "SPDXRef-DOCUMENT",
189+
"name": "SPDX-Tools-v2.0",
190+
"documentDescribes": [
191+
"SPDXRef-Container"
192+
],
193+
"packages": [
194+
{
195+
"name": "Container",
196+
"SPDXID": "SPDXRef-Container"
197+
},
198+
{
199+
"name": "Package-1",
200+
"SPDXID": "SPDXRef-Package-1",
201+
"versionInfo": "1.1.1",
202+
"hasFiles": [
203+
"SPDXRef-File-1",
204+
"SPDXRef-File-2"
205+
]
206+
},
207+
{
208+
"name": "Package-2",
209+
"SPDXID": "SPDXRef-Package-2",
210+
"versionInfo": "2.2.2"
211+
}
212+
],
213+
"files": [
214+
{
215+
"fileName": "./f1",
216+
"SPDXID": "SPDXRef-File-1"
217+
},
218+
{
219+
"fileName": "./f2",
220+
"SPDXID": "SPDXRef-File-2"
221+
}
222+
],
223+
"relationships": [
224+
{
225+
"spdxElementId": "SPDXRef-Package-1",
226+
"relationshipType": "CONTAINS",
227+
"relatedSpdxElement": "SPDXRef-File-1"
228+
},
229+
{
230+
"spdxElementId": "SPDXRef-Package-1",
231+
"relationshipType": "CONTAINS",
232+
"relatedSpdxElement": "SPDXRef-File-2"
233+
}
234+
]
235+
}`
236+
237+
doc := spdx.Document{}
238+
err := json.ReadInto(strings.NewReader(contents), &doc)
239+
240+
require.NoError(t, err)
241+
242+
id := func(s string) common.DocElementID {
243+
return common.DocElementID{
244+
ElementRefID: common.ElementID(s),
245+
}
246+
}
247+
248+
want := spdx.Document{
249+
SPDXVersion: spdx.Version,
250+
DataLicense: spdx.DataLicense,
251+
SPDXIdentifier: "DOCUMENT",
252+
DocumentName: "SPDX-Tools-v2.0",
253+
Packages: []*spdx.Package{
254+
{
255+
PackageName: "Container",
256+
PackageSPDXIdentifier: "Container",
257+
FilesAnalyzed: true,
258+
},
259+
{
260+
PackageName: "Package-1",
261+
PackageSPDXIdentifier: "Package-1",
262+
PackageVersion: "1.1.1",
263+
FilesAnalyzed: true,
264+
},
265+
{
266+
PackageName: "Package-2",
267+
PackageSPDXIdentifier: "Package-2",
268+
PackageVersion: "2.2.2",
269+
FilesAnalyzed: true,
270+
},
271+
},
272+
Files: []*spdx.File{
273+
{
274+
FileName: "./f1",
275+
FileSPDXIdentifier: "File-1",
276+
},
277+
{
278+
FileName: "./f2",
279+
FileSPDXIdentifier: "File-2",
280+
},
281+
},
282+
Relationships: []*spdx.Relationship{
283+
{
284+
RefA: id("DOCUMENT"),
285+
RefB: id("Container"),
286+
Relationship: common.TypeRelationshipDescribe,
287+
},
288+
{
289+
RefA: id("Package-1"),
290+
RefB: id("File-1"),
291+
Relationship: common.TypeRelationshipContains,
292+
},
293+
{
294+
RefA: id("Package-1"),
295+
RefB: id("File-2"),
296+
Relationship: common.TypeRelationshipContains,
297+
},
298+
},
299+
}
300+
301+
if diff := cmp.Diff(want, doc, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
302+
t.Errorf("got incorrect struct after parsing JSON example: %s", diff)
303+
return
304+
}
204305
}
205306

206307
func Test_JsonEnums(t *testing.T) {
@@ -344,3 +445,9 @@ func Test_JsonEnums(t *testing.T) {
344445
},
345446
}, doc)
346447
}
448+
449+
func relationshipLess(a, b *spdx.Relationship) bool {
450+
aStr, _ := jsonenc.Marshal(a)
451+
bStr, _ := jsonenc.Marshal(b)
452+
return string(aStr) < string(bStr)
453+
}

‎spdx/v2/v2_2/yaml/yaml_test.go

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ package yaml
44

55
import (
66
"bytes"
7+
jsonenc "encoding/json"
78
"fmt"
89
"os"
910
"testing"
1011

1112
"github.com/google/go-cmp/cmp"
1213
"github.com/google/go-cmp/cmp/cmpopts"
1314

14-
"github.com/spdx/tools-golang/spdx/v2/common"
1515
spdx "github.com/spdx/tools-golang/spdx/v2/v2_2"
1616
"github.com/spdx/tools-golang/spdx/v2/v2_2/example"
1717
"github.com/spdx/tools-golang/yaml"
@@ -20,34 +20,6 @@ import (
2020
func Test_Read(t *testing.T) {
2121
want := example.Copy()
2222

23-
want.Relationships = append(want.Relationships, []*spdx.Relationship{
24-
{
25-
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
26-
RefB: common.DocElementID{ElementRefID: "File"},
27-
Relationship: "DESCRIBES",
28-
},
29-
{
30-
RefA: common.DocElementID{ElementRefID: "DOCUMENT"},
31-
RefB: common.DocElementID{ElementRefID: "Package"},
32-
Relationship: "DESCRIBES",
33-
},
34-
{
35-
RefA: common.DocElementID{ElementRefID: "Package"},
36-
RefB: common.DocElementID{ElementRefID: "CommonsLangSrc"},
37-
Relationship: "CONTAINS",
38-
},
39-
{
40-
RefA: common.DocElementID{ElementRefID: "Package"},
41-
RefB: common.DocElementID{ElementRefID: "JenaLib"},
42-
Relationship: "CONTAINS",
43-
},
44-
{
45-
RefA: common.DocElementID{ElementRefID: "Package"},
46-
RefB: common.DocElementID{ElementRefID: "DoapSource"},
47-
Relationship: "CONTAINS",
48-
},
49-
}...)
50-
5123
file, err := os.Open("../../../../examples/sample-docs/yaml/SPDXYAMLExample-2.2.spdx.yaml")
5224
if err != nil {
5325
panic(fmt.Errorf("error opening File: %s", err))
@@ -60,8 +32,8 @@ func Test_Read(t *testing.T) {
6032
return
6133
}
6234

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

91-
if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
92-
t.Errorf("got incorrect struct after writing and re-parsing YAML example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
63+
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{}), cmpopts.SortSlices(relationshipLess)); len(diff) > 0 {
64+
t.Errorf("got incorrect struct after writing and re-parsing YAML example: %s", diff)
9365
return
9466
}
9567
}
68+
69+
func relationshipLess(a, b *spdx.Relationship) bool {
70+
aStr, _ := jsonenc.Marshal(a)
71+
bStr, _ := jsonenc.Marshal(b)
72+
return string(aStr) < string(bStr)
73+
}

0 commit comments

Comments
 (0)