Skip to content

Commit 7b5e2ee

Browse files
author
Anivar A Aravind
committed
Implement review feedback: omit empty fields and proper error wrapping
- Use %w for proper error wrapping as suggested by @anderseknert - Omit empty optional fields as suggested by @srenatus - Update tests to match new behavior - This makes the API more idiomatic to Rego where checking for field existence is clearer than checking for empty strings Signed-off-by: Anivar A Aravind <[email protected]>
1 parent 7c0cf8b commit 7b5e2ee

File tree

2 files changed

+34
-31
lines changed

2 files changed

+34
-31
lines changed

v1/topdown/purl.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,35 @@ func builtinPurlParse(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Ter
3131

3232
purl, err := packageurl.FromString(string(str))
3333
if err != nil {
34-
return fmt.Errorf("invalid PURL %q: %s", str, err)
34+
return fmt.Errorf("invalid PURL %q: %w", str, err)
3535
}
3636

37-
// Handle qualifiers - always return an object (empty if no qualifiers)
38-
qualifiers := ast.NewObject()
39-
for _, q := range purl.Qualifiers {
40-
qualifiers.Insert(ast.StringTerm(q.Key), ast.StringTerm(q.Value))
41-
}
42-
43-
// Create object with all PURL components in a single operation
37+
// Create object with required fields
4438
obj := ast.NewObject(
4539
[2]*ast.Term{ast.InternedTerm("type"), ast.StringTerm(purl.Type)},
4640
[2]*ast.Term{ast.InternedTerm("name"), ast.StringTerm(purl.Name)},
47-
[2]*ast.Term{ast.InternedTerm("namespace"), ast.StringTerm(purl.Namespace)},
48-
[2]*ast.Term{ast.InternedTerm("version"), ast.StringTerm(purl.Version)},
49-
[2]*ast.Term{ast.InternedTerm("subpath"), ast.StringTerm(purl.Subpath)},
50-
[2]*ast.Term{ast.InternedTerm("qualifiers"), ast.NewTerm(qualifiers)},
5141
)
5242

43+
// Add optional fields only if present
44+
if purl.Namespace != "" {
45+
obj.Insert(ast.InternedTerm("namespace"), ast.StringTerm(purl.Namespace))
46+
}
47+
if purl.Version != "" {
48+
obj.Insert(ast.InternedTerm("version"), ast.StringTerm(purl.Version))
49+
}
50+
if purl.Subpath != "" {
51+
obj.Insert(ast.InternedTerm("subpath"), ast.StringTerm(purl.Subpath))
52+
}
53+
54+
// Add qualifiers only if present
55+
if len(purl.Qualifiers) > 0 {
56+
qualifiers := ast.NewObject()
57+
for _, q := range purl.Qualifiers {
58+
qualifiers.Insert(ast.StringTerm(q.Key), ast.StringTerm(q.Value))
59+
}
60+
obj.Insert(ast.InternedTerm("qualifiers"), ast.NewTerm(qualifiers))
61+
}
62+
5363
return iter(ast.NewTerm(obj))
5464
}
5565

v1/topdown/purl_test.go

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -84,23 +84,18 @@ func TestBuiltinPurlParse(t *testing.T) {
8484
{
8585
input: `"pkg:npm/[email protected]"`,
8686
expected: map[string]interface{}{
87-
"type": "npm",
88-
"namespace": "",
89-
"name": "foobar",
90-
"version": "12.3.1",
91-
"qualifiers": map[string]interface{}{},
92-
"subpath": "",
87+
"type": "npm",
88+
"name": "foobar",
89+
"version": "12.3.1",
9390
},
9491
},
9592
{
9693
input: `"pkg:maven/org.apache.xmlgraphics/[email protected]"`,
9794
expected: map[string]interface{}{
98-
"type": "maven",
99-
"namespace": "org.apache.xmlgraphics",
100-
"name": "batik-anim",
101-
"version": "1.9.1",
102-
"qualifiers": map[string]interface{}{},
103-
"subpath": "",
95+
"type": "maven",
96+
"namespace": "org.apache.xmlgraphics",
97+
"name": "batik-anim",
98+
"version": "1.9.1",
10499
},
105100
},
106101
{
@@ -114,18 +109,16 @@ func TestBuiltinPurlParse(t *testing.T) {
114109
"arch": "i386",
115110
"distro": "fedora-25",
116111
},
117-
"subpath": "",
118112
},
119113
},
120114
{
121115
input: `"pkg:deb/debian/[email protected]#src/curl.c"`,
122116
expected: map[string]interface{}{
123-
"type": "deb",
124-
"namespace": "debian",
125-
"name": "curl",
126-
"version": "7.50.3-1",
127-
"qualifiers": map[string]interface{}{},
128-
"subpath": "src/curl.c",
117+
"type": "deb",
118+
"namespace": "debian",
119+
"name": "curl",
120+
"version": "7.50.3-1",
121+
"subpath": "src/curl.c",
129122
},
130123
},
131124
}

0 commit comments

Comments
 (0)