Merge pull request #68 from swinslow/issue-59-to-master

Fix special IDs for right-side 2.2 Relationships

spdx/identifier.go

@@ -18,9 +18,14 @@ type ElementID string
 // present document.
 // DocElementIDs should NOT contain the mandatory 'DocumentRef-' or
 // 'SPDXRef-' portions.
+// SpecialID is used ONLY if the DocElementID matches a defined set of
+// permitted special values for a particular field, e.g. "NONE" or
+// "NOASSERTION" for the right-hand side of Relationships. If SpecialID
+// is set, DocumentRefID and ElementRefID should be empty (and vice versa).
 type DocElementID struct {
 	DocumentRefID string
 	ElementRefID  ElementID
+	SpecialID     string
 }
 
 // TODO: add equivalents for LicenseRef- identifiers
@@ -36,6 +41,14 @@ func MakeDocElementID(docRef string, eltRef string) DocElementID {
 	}
 }
 
+// MakeDocElementSpecial takes a "special" string (e.g. "NONE" or
+// "NOASSERTION" for the right side of a Relationship), nd returns
+// a DocElementID with it in the SpecialID field. Other fields will
+// be empty.
+func MakeDocElementSpecial(specialID string) DocElementID {
+	return DocElementID{SpecialID: specialID}
+}
+
 // RenderElementID takes an ElementID and returns the string equivalent,
 // with the SPDXRef- prefix reinserted.
 func RenderElementID(eID ElementID) string {
@@ -44,8 +57,12 @@ func RenderElementID(eID ElementID) string {
 
 // RenderDocElementID takes a DocElementID and returns the string equivalent,
 // with the SPDXRef- prefix (and, if applicable, the DocumentRef- prefix)
-// reinserted.
+// reinserted. If a SpecialID is present, it will be rendered verbatim and
+// DocumentRefID and ElementRefID will be ignored.
 func RenderDocElementID(deID DocElementID) string {
+	if deID.SpecialID != "" {
+		return deID.SpecialID
+	}
 	prefix := ""
 	if deID.DocumentRefID != "" {
 		prefix = "DocumentRef-" + deID.DocumentRefID + ":"

tvloader/parser2v2/parse_relationship.go

@@ -35,7 +35,9 @@ func (parser *tvParser2_2) parsePairForRelationship2_2(tag string, value string)
 		}
 		parser.rln.RefA = aID
 		parser.rln.Relationship = strings.TrimSpace(rp[1])
-		bID, err := extractDocElementID(strings.TrimSpace(rp[2]))
+		// NONE and NOASSERTION are permitted on right side
+		permittedSpecial := []string{"NONE", "NOASSERTION"}
+		bID, err := extractDocElementSpecial(strings.TrimSpace(rp[2]), permittedSpecial)
 		if err != nil {
 			return err
 		}

tvloader/parser2v2/parse_relationship_test.go

@@ -152,6 +152,37 @@ func TestParser2_2InvalidRelationshipTagsInvalidRefIDs(t *testing.T) {
 	}
 }
 
+func TestParser2_2SpecialValuesValidForRightSideOfRelationship(t *testing.T) {
+	parser := tvParser2_2{
+		doc: &spdx.Document2_2{},
+		st:  psCreationInfo2_2,
+	}
+
+	// NONE in right side of relationship should pass
+	err := parser.parsePair2_2("Relationship", "SPDXRef-a CONTAINS NONE")
+	if err != nil {
+		t.Errorf("expected nil error for CONTAINS NONE, got %v", err)
+	}
+
+	// NOASSERTION in right side of relationship should pass
+	err = parser.parsePair2_2("Relationship", "SPDXRef-a CONTAINS NOASSERTION")
+	if err != nil {
+		t.Errorf("expected nil error for CONTAINS NOASSERTION, got %v", err)
+	}
+
+	// NONE in left side of relationship should fail
+	err = parser.parsePair2_2("Relationship", "NONE CONTAINS SPDXRef-a")
+	if err == nil {
+		t.Errorf("expected non-nil error for NONE CONTAINS, got nil")
+	}
+
+	// NOASSERTION in left side of relationship should fail
+	err = parser.parsePair2_2("Relationship", "NOASSERTION CONTAINS SPDXRef-a")
+	if err == nil {
+		t.Errorf("expected non-nil error for NOASSERTION CONTAINS, got nil")
+	}
+}
+
 func TestParser2_2FailsToParseUnknownTagInRelationshipSection(t *testing.T) {
 	parser := tvParser2_2{
 		doc: &spdx.Document2_2{},

tvloader/parser2v2/util.go

@@ -73,6 +73,23 @@ func extractDocElementID(value string) (spdx.DocElementID, error) {
 	return spdx.DocElementID{DocumentRefID: docRefID, ElementRefID: spdx.ElementID(eltRefID)}, nil
 }
 
+// used to extract SPDXRef values from an SPDX Identifier, OR "special" strings
+// from a specified set of permitted values. The primary use case for this is
+// the right-hand side of Relationships, where beginning in SPDX 2.2 the values
+// "NONE" and "NOASSERTION" are permitted. If the value does not match one of
+// the specified permitted values, it will fall back to the ordinary
+// DocElementID extractor.
+func extractDocElementSpecial(value string, permittedSpecial []string) (spdx.DocElementID, error) {
+	// check value against special set first
+	for _, sp := range permittedSpecial {
+		if sp == value {
+			return spdx.DocElementID{SpecialID: sp}, nil
+		}
+	}
+	// not found, fall back to regular search
+	return extractDocElementID(value)
+}
+
 // used to extract SPDXRef values only from an SPDX Identifier which can point
 // to this document only. Use extractDocElementID for parsing IDs that can
 // refer either to this document or a different one.

tvloader/parser2v2/util_test.go

@@ -67,13 +67,58 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo
 	}
 	if deID.ElementRefID != spdx.ElementID(wantElt) {
 		if wantElt == "" {
-			t.Errorf("testing %v: want emptyString for ElementRefID, got %v", tst, deID.ElementRefID)
+			t.Errorf("testing %v: want empty string for ElementRefID, got %v", tst, deID.ElementRefID)
 		} else {
 			t.Errorf("testing %v: want %v for ElementRefID, got %v", tst, wantElt, deID.ElementRefID)
 		}
 	}
 }
 
+func TestCanExtractSpecialDocumentIDs(t *testing.T) {
+	permittedSpecial := []string{"NONE", "NOASSERTION"}
+	// test with valid special values
+	helperForExtractDocElementSpecial(t, permittedSpecial, "NONE", false, "", "", "NONE")
+	helperForExtractDocElementSpecial(t, permittedSpecial, "NOASSERTION", false, "", "", "NOASSERTION")
+	// test with valid regular IDs
+	helperForExtractDocElementSpecial(t, permittedSpecial, "SPDXRef-file1", false, "", "file1", "")
+	helperForExtractDocElementSpecial(t, permittedSpecial, "DocumentRef-doc2:SPDXRef-file2", false, "doc2", "file2", "")
+	helperForExtractDocElementSpecial(t, permittedSpecial, "a:SPDXRef-file1", true, "", "", "")
+	helperForExtractDocElementSpecial(t, permittedSpecial, "DocumentRef-doc2", true, "", "", "")
+	// test with invalid other words not on permitted list
+	helperForExtractDocElementSpecial(t, permittedSpecial, "FOO", true, "", "", "")
+}
+
+func helperForExtractDocElementSpecial(t *testing.T, permittedSpecial []string, tst string, wantErr bool, wantDoc string, wantElt string, wantSpecial string) {
+	deID, err := extractDocElementSpecial(tst, permittedSpecial)
+	if err != nil && wantErr == false {
+		t.Errorf("testing %v: expected nil error, got %v", tst, err)
+	}
+	if err == nil && wantErr == true {
+		t.Errorf("testing %v: expected non-nil error, got nil", tst)
+	}
+	if deID.DocumentRefID != wantDoc {
+		if wantDoc == "" {
+			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
+		} else {
+			t.Errorf("testing %v: want %v for DocumentRefID, got %v", tst, wantDoc, deID.DocumentRefID)
+		}
+	}
+	if deID.ElementRefID != spdx.ElementID(wantElt) {
+		if wantElt == "" {
+			t.Errorf("testing %v: want empty string for ElementRefID, got %v", tst, deID.ElementRefID)
+		} else {
+			t.Errorf("testing %v: want %v for ElementRefID, got %v", tst, wantElt, deID.ElementRefID)
+		}
+	}
+	if deID.SpecialID != wantSpecial {
+		if wantSpecial == "" {
+			t.Errorf("testing %v: want empty string for SpecialID, got %v", tst, deID.SpecialID)
+		} else {
+			t.Errorf("testing %v: want %v for SpecialID, got %v", tst, wantSpecial, deID.SpecialID)
+		}
+	}
+}
+
 func TestCanExtractElementRefsOnlyFromID(t *testing.T) {
 	// test with valid ID in this document
 	helperForExtractElementID(t, "SPDXRef-file1", false, "file1")

tvsaver/saver2v2/save_relationship_test.go

@@ -62,3 +62,53 @@ func TestSaver2_2RelationshipOmitsOptionalFieldsIfEmpty(t *testing.T) {
 		t.Errorf("Expected %v, got %v", want.String(), got.String())
 	}
 }
+
+func TestSaver2_2RelationshipCanHaveNONEOnRight(t *testing.T) {
+	rln := &spdx.Relationship2_2{
+		RefA:         spdx.MakeDocElementID("", "PackageA"),
+		RefB:         spdx.MakeDocElementSpecial("NONE"),
+		Relationship: "DEPENDS_ON",
+	}
+
+	// what we want to get, as a buffer of bytes
+	// no trailing blank newline
+	want := bytes.NewBufferString("Relationship: SPDXRef-PackageA DEPENDS_ON NONE\n")
+
+	// render as buffer of bytes
+	var got bytes.Buffer
+	err := renderRelationship2_2(rln, &got)
+	if err != nil {
+		t.Errorf("Expected nil error, got %v", err)
+	}
+
+	// check that they match
+	c := bytes.Compare(want.Bytes(), got.Bytes())
+	if c != 0 {
+		t.Errorf("Expected %v, got %v", want.String(), got.String())
+	}
+}
+
+func TestSaver2_2RelationshipCanHaveNOASSERTIONOnRight(t *testing.T) {
+	rln := &spdx.Relationship2_2{
+		RefA:         spdx.MakeDocElementID("", "PackageA"),
+		RefB:         spdx.MakeDocElementSpecial("NOASSERTION"),
+		Relationship: "DEPENDS_ON",
+	}
+
+	// what we want to get, as a buffer of bytes
+	// no trailing blank newline
+	want := bytes.NewBufferString("Relationship: SPDXRef-PackageA DEPENDS_ON NOASSERTION\n")
+
+	// render as buffer of bytes
+	var got bytes.Buffer
+	err := renderRelationship2_2(rln, &got)
+	if err != nil {
+		t.Errorf("Expected nil error, got %v", err)
+	}
+
+	// check that they match
+	c := bytes.Compare(want.Bytes(), got.Bytes())
+	if c != 0 {
+		t.Errorf("Expected %v, got %v", want.String(), got.String())
+	}
+}