Resolve two issues related to ExternalDocumentRef

examples/sample-docs/tv/SPDXTagExample-v2.3.spdx

@@ -3,7 +3,7 @@ DataLicense: CC0-1.0
 SPDXID: SPDXRef-DOCUMENT
 DocumentName: SPDX-Tools-v2.0
 DocumentNamespace: http://spdx.org/spdxdocs/spdx-example-444504E0-4F89-41D3-9A0C-0305E82C3301
-ExternalDocumentRef: DocumentRef-DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759
+ExternalDocumentRef: DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759
 DocumentComment: This document was created using SPDX 2.0 using licenses from the web site.
 LicenseListVersion: 3.9
 Creator: Tool: LicenseFind-1.0

spdx/v2/common/identifier.go

@@ -14,6 +14,40 @@ const (
 	documentRefPrefix = "DocumentRef-"
 )
 
+// DocumentID represents the identifier string portion of an SPDX document
+// identifier. DocumentID should be used for reference to a SPDX document.
+// DocumentIDs should NOT contain the mandatory 'DocumentRef-' portion.
+type DocumentID string
+
+// MarshalJSON returns an DocumentRef- prefixed JSON string
+func (d DocumentID) MarshalJSON() ([]byte, error) {
+	return marshal.JSON(prefixDocumentId(d))
+}
+
+// UnmarshalJSON validates DocumentRef- prefixes and removes them when processing DocumentIDs
+func (d *DocumentID) UnmarshalJSON(data []byte) error {
+	// SPDX identifier will simply be a string
+	idStr := string(data)
+	idStr = strings.Trim(idStr, "\"")
+
+	*d = trimDocumentIdPrefix(idStr)
+	return nil
+}
+
+// prefixDocumentId adds the DocumentRef- prefix to an document ID if it does not have one
+func prefixDocumentId(id DocumentID) string {
+	val := string(id)
+	if !strings.HasPrefix(val, spdxRefPrefix) {
+		return documentRefPrefix + val
+	}
+	return val
+}
+
+// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string
+func trimDocumentIdPrefix(id string) DocumentID {
+	return DocumentID(strings.TrimPrefix(id, documentRefPrefix))
+}
+
 // ElementID represents the identifier string portion of an SPDX element
 // identifier. DocElementID should be used for any attributes which can
 // contain identifiers defined in a different SPDX document.
@@ -31,11 +65,7 @@ func (d *ElementID) UnmarshalJSON(data []byte) error {
 	idStr := string(data)
 	idStr = strings.Trim(idStr, "\"")
 
-	e, err := trimElementIdPrefix(idStr)
-	if err != nil {
-		return err
-	}
-	*d = e
+	*d = trimElementIdPrefix(idStr)
 	return nil
 }
 
@@ -48,17 +78,9 @@ func prefixElementId(id ElementID) string {
 	return val
 }
 
-// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string or returns an error if it
-// does not start with SPDXRef-
-func trimElementIdPrefix(id string) (ElementID, error) {
-	// handle SPDXRef-
-	idFields := strings.SplitN(id, spdxRefPrefix, 2)
-	if len(idFields) != 2 {
-		return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id)
-	}
-
-	e := ElementID(idFields[1])
-	return e, nil
+// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string
+func trimElementIdPrefix(id string) ElementID {
+	return ElementID(strings.TrimPrefix(id, spdxRefPrefix))
 }
 
 // DocElementID represents an SPDX element identifier that could be defined
@@ -76,7 +98,7 @@ func trimElementIdPrefix(id string) (ElementID, error) {
 // "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
+	DocumentRefID DocumentID
 	ElementRefID  ElementID
 	SpecialID     string
 }
@@ -85,8 +107,9 @@ type DocElementID struct {
 // This function is also used when marshalling to YAML
 func (d DocElementID) MarshalJSON() ([]byte, error) {
 	if d.DocumentRefID != "" && d.ElementRefID != "" {
-		idStr := prefixElementId(d.ElementRefID)
-		return marshal.JSON(fmt.Sprintf("%s%s:%s", documentRefPrefix, d.DocumentRefID, idStr))
+		dIdStr := prefixDocumentId(d.DocumentRefID)
+		eIdStr := prefixElementId(d.ElementRefID)
+		return marshal.JSON(fmt.Sprintf("%s:%s", dIdStr, eIdStr))
 	} else if d.ElementRefID != "" {
 		return marshal.JSON(prefixElementId(d.ElementRefID))
 	} else if d.SpecialID != "" {
@@ -98,7 +121,7 @@ func (d DocElementID) MarshalJSON() ([]byte, error) {
 
 // UnmarshalJSON takes a SPDX Identifier string parses it into a DocElementID struct.
 // This function is also used when unmarshalling YAML
-func (d *DocElementID) UnmarshalJSON(data []byte) (err error) {
+func (d *DocElementID) UnmarshalJSON(data []byte) error {
 	// SPDX identifier will simply be a string
 	idStr := string(data)
 	idStr = strings.Trim(idStr, "\"")
@@ -112,23 +135,19 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) {
 	var idFields []string
 	// handle DocumentRef- if present
 	if strings.HasPrefix(idStr, documentRefPrefix) {
-		// strip out the "DocumentRef-" so we can get the value
-		idFields = strings.SplitN(idStr, documentRefPrefix, 2)
-		idStr = idFields[1]
-
 		// an SPDXRef can appear after a DocumentRef, separated by a colon
 		idFields = strings.SplitN(idStr, ":", 2)
-		d.DocumentRefID = idFields[0]
 
+		d.DocumentRefID = trimDocumentIdPrefix(idFields[0])
 		if len(idFields) == 2 {
 			idStr = idFields[1]
 		} else {
 			return nil
 		}
 	}
 
-	d.ElementRefID, err = trimElementIdPrefix(idStr)
-	return err
+	d.ElementRefID = trimElementIdPrefix(idStr)
+	return nil
 }
 
 // TODO: add equivalents for LicenseRef- identifiers
@@ -139,7 +158,7 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) {
 // present document.
 func MakeDocElementID(docRef string, eltRef string) DocElementID {
 	return DocElementID{
-		DocumentRefID: docRef,
+		DocumentRefID: DocumentID(docRef),
 		ElementRefID:  ElementID(eltRef),
 	}
 }
@@ -168,7 +187,7 @@ func RenderDocElementID(deID DocElementID) string {
 	}
 	prefix := ""
 	if deID.DocumentRefID != "" {
-		prefix = documentRefPrefix + deID.DocumentRefID + ":"
+		prefix = documentRefPrefix + string(deID.DocumentRefID) + ":"
 	}
 	return prefix + spdxRefPrefix + string(deID.ElementRefID)
 }

spdx/v2/common/identifier_test.go

@@ -103,14 +103,19 @@ func Test_DocElementIDDecoding(t *testing.T) {
 			},
 		},
 		{
-			name:  "DocumentRefID invalid ElementRefID",
-			value: "DocumentRef-a-doc:invalid",
-			err:   true,
+			name:  "DocumentRefID:ElementRefID without spdxref prefix",
+			value: "DocumentRef-a-doc:some-id",
+			expected: DocElementID{
+				DocumentRefID: "a-doc",
+				ElementRefID:  "some-id",
+			},
 		},
 		{
-			name:  "invalid format",
+			name:  "without spdxref prefix",
 			value: "some-id-without-spdxref",
-			err:   true,
+			expected: DocElementID{
+				ElementRefID: "some-id-without-spdxref",
+			},
 		},
 		{
 			name:  "SpecialID NONE",
@@ -203,9 +208,9 @@ func Test_ElementIDDecoding(t *testing.T) {
 			expected: ElementID("some-id"),
 		},
 		{
-			name:  "invalid format",
+			name:  "without prefix",
 			value: "some-id-without-spdxref",
-			err:   true,
+			expected: ElementID("some-id-without-spdxref"),
 		},
 	}
 
@@ -292,9 +297,11 @@ func Test_ElementIDStructDecoding(t *testing.T) {
 			value: `{"id":"SPDXRef-some-id"}`,
 		},
 		{
-			name:  "invalid format",
+			name:  "without prefix",
+			expected: typ{
+				Id: ElementID("some-id"),
+			},
 			value: `{"id":"some-id"}`,
-			err:   true,
 		},
 	}
 

spdx/v2/v2_1/document.go

@@ -18,7 +18,7 @@ type ExternalDocumentRef struct {
 	// DocumentRefID is the ID string defined in the start of the
 	// reference. It should _not_ contain the "DocumentRef-" part
 	// of the mandatory ID string.
-	DocumentRefID string `json:"externalDocumentId"`
+	DocumentRefID common.DocumentID `json:"externalDocumentId"`
 
 	// URI is the URI defined for the external document
 	URI string `json:"spdxDocument"`

spdx/v2/v2_1/tagvalue/reader/parser.go

@@ -86,7 +86,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error {
 			return err
 		}
 		edr := spdx.ExternalDocumentRef{
-			DocumentRefID: documentRefID,
+			DocumentRefID: common.DocumentID(documentRefID),
 			URI:           uri,
 			Checksum:      common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum},
 		}

spdx/v2/v2_1/tagvalue/reader/util.go

@@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) {
 	}
 
 	// we're good
-	return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil
+	return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil
 }
 
 // used to extract SPDXRef values only from an SPDX Identifier which can point

spdx/v2/v2_1/tagvalue/reader/util_test.go

@@ -58,7 +58,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo
 	if err == nil && wantErr == true {
 		t.Errorf("testing %v: expected non-nil error, got nil", tst)
 	}
-	if deID.DocumentRefID != wantDoc {
+	if deID.DocumentRefID != common.DocumentID(wantDoc) {
 		if wantDoc == "" {
 			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
 		} else {

spdx/v2/v2_2/document.go

@@ -21,7 +21,7 @@ type ExternalDocumentRef struct {
 	// DocumentRefID is the ID string defined in the start of the
 	// reference. It should _not_ contain the "DocumentRef-" part
 	// of the mandatory ID string.
-	DocumentRefID string `json:"externalDocumentId"`
+	DocumentRefID common.DocumentID `json:"externalDocumentId"`
 
 	// URI is the URI defined for the external document
 	URI string `json:"spdxDocument"`

spdx/v2/v2_2/example/example.go

@@ -41,7 +41,7 @@ var example = spdx.Document{
 	DocumentComment: "This document was created using SPDX 2.0 using licenses from the web site.",
 	ExternalDocumentReferences: []spdx.ExternalDocumentRef{
 		{
-			DocumentRefID: "DocumentRef-spdx-tool-1.2",
+			DocumentRefID: "spdx-tool-1.2",
 			URI:           "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301",
 			Checksum: common.Checksum{
 				Algorithm: common.SHA1,

spdx/v2/v2_2/rdf/reader/parse_spdx_document.go

@@ -96,7 +96,7 @@ func (parser *rdfParser2_2) getExternalDocumentRefFromNode(node *gordfParser.Nod
 		switch triple.Predicate.ID {
 		case SPDX_EXTERNAL_DOCUMENT_ID:
 			// cardinality: exactly 1
-			edr.DocumentRefID = triple.Object.ID
+			edr.DocumentRefID = common.DocumentID(triple.Object.ID)
 		case SPDX_SPDX_DOCUMENT:
 			// cardinality: exactly 1
 			// assumption: "spdxDocument" property of an external document

spdx/v2/v2_2/rdf/reader/utils.go

@@ -112,7 +112,7 @@ func ExtractDocElementID(value string) (common.DocElementID, error) {
 	}
 
 	// we're good
-	return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil
+	return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil
 }
 
 // used to extract SPDXRef values only from an SPDX Identifier which can point

spdx/v2/v2_2/rdf/reader/utils_test.go

@@ -217,7 +217,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo
 	if err == nil && wantErr == true {
 		t.Errorf("testing %v: expected non-nil error, got nil", tst)
 	}
-	if deID.DocumentRefID != wantDoc {
+	if deID.DocumentRefID != common.DocumentID(wantDoc) {
 		if wantDoc == "" {
 			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
 		} else {

spdx/v2/v2_2/tagvalue/reader/parser.go

@@ -85,7 +85,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error {
 			return err
 		}
 		edr := spdx.ExternalDocumentRef{
-			DocumentRefID: documentRefID,
+			DocumentRefID: common.DocumentID(documentRefID),
 			URI:           uri,
 			Checksum:      common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum},
 		}

spdx/v2/v2_2/tagvalue/reader/util.go

@@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) {
 	}
 
 	// we're good
-	return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil
+	return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil
 }
 
 // used to extract SPDXRef values from an SPDX Identifier, OR "special" strings

spdx/v2/v2_2/tagvalue/reader/util_test.go

@@ -58,7 +58,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo
 	if err == nil && wantErr == true {
 		t.Errorf("testing %v: expected non-nil error, got nil", tst)
 	}
-	if deID.DocumentRefID != wantDoc {
+	if deID.DocumentRefID != common.DocumentID(wantDoc) {
 		if wantDoc == "" {
 			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
 		} else {
@@ -96,7 +96,7 @@ func helperForExtractDocElementSpecial(t *testing.T, permittedSpecial []string,
 	if err == nil && wantErr == true {
 		t.Errorf("testing %v: expected non-nil error, got nil", tst)
 	}
-	if deID.DocumentRefID != wantDoc {
+	if deID.DocumentRefID != common.DocumentID(wantDoc) {
 		if wantDoc == "" {
 			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
 		} else {

spdx/v2/v2_3/document.go

@@ -20,7 +20,7 @@ type ExternalDocumentRef struct {
 	// DocumentRefID is the ID string defined in the start of the
 	// reference. It should _not_ contain the "DocumentRef-" part
 	// of the mandatory ID string.
-	DocumentRefID string `json:"externalDocumentId"`
+	DocumentRefID common.DocumentID `json:"externalDocumentId"`
 
 	// URI is the URI defined for the external document
 	URI string `json:"spdxDocument"`

spdx/v2/v2_3/example/example.go

@@ -41,7 +41,7 @@ var example = spdx.Document{
 	DocumentComment: "This document was created using SPDX 2.0 using licenses from the web site.",
 	ExternalDocumentReferences: []spdx.ExternalDocumentRef{
 		{
-			DocumentRefID: "DocumentRef-spdx-tool-1.2",
+			DocumentRefID: "spdx-tool-1.2",
 			URI:           "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301",
 			Checksum: common.Checksum{
 				Algorithm: common.SHA1,

spdx/v2/v2_3/rdf/reader/parse_spdx_document.go

@@ -96,7 +96,7 @@ func (parser *rdfParser2_3) getExternalDocumentRefFromNode(node *gordfParser.Nod
 		switch triple.Predicate.ID {
 		case SPDX_EXTERNAL_DOCUMENT_ID:
 			// cardinality: exactly 1
-			edr.DocumentRefID = triple.Object.ID
+			edr.DocumentRefID = common.DocumentID(triple.Object.ID)
 		case SPDX_SPDX_DOCUMENT:
 			// cardinality: exactly 1
 			// assumption: "spdxDocument" property of an external document

spdx/v2/v2_3/rdf/reader/utils.go

@@ -112,7 +112,7 @@ func ExtractDocElementID(value string) (common.DocElementID, error) {
 	}
 
 	// we're good
-	return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil
+	return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil
 }
 
 // used to extract SPDXRef values only from an SPDX Identifier which can point

spdx/v2/v2_3/rdf/reader/utils_test.go

@@ -217,7 +217,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo
 	if err == nil && wantErr == true {
 		t.Errorf("testing %v: expected non-nil error, got nil", tst)
 	}
-	if deID.DocumentRefID != wantDoc {
+	if deID.DocumentRefID != common.DocumentID(wantDoc) {
 		if wantDoc == "" {
 			t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID)
 		} else {

spdx/v2/v2_3/tagvalue/reader/parser.go

@@ -84,7 +84,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error {
 			return err
 		}
 		edr := spdx.ExternalDocumentRef{
-			DocumentRefID: documentRefID,
+			DocumentRefID: common.DocumentID(documentRefID),
 			URI:           uri,
 			Checksum:      common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum},
 		}

spdx/v2/v2_3/tagvalue/reader/util.go

@@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) {
 	}
 
 	// we're good
-	return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil
+	return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil
 }
 
 // used to extract SPDXRef values from an SPDX Identifier, OR "special" strings