Skip to content

Commit 689f8e4

Browse files
MalcolmnixonMalcolmnixon
authored
Add checking of SPDX ID fields (#58)
* Add checking of SPDX ID fields * Minor improvement to regular expression.
1 parent 283d4e9 commit 689f8e4

File tree

7 files changed

+146
-47
lines changed

7 files changed

+146
-47
lines changed

‎src/DemaConsulting.SpdxModel/SpdxElement.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,23 @@
1818
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
1919
// SOFTWARE.
2020

21+
using System.Text.RegularExpressions;
22+
2123
namespace DemaConsulting.SpdxModel;
2224

2325
/// <summary>
2426
/// SPDX Element base class
2527
/// </summary>
2628
public abstract class SpdxElement
2729
{
30+
/// <summary>
31+
/// Regular expression for checking element IDs of the form "SPDXRef-name"
32+
/// </summary>
33+
protected static readonly Regex SpdxRefRegex = new(
34+
"^SPDXRef-[a-zA-Z0-9,-]+$",
35+
RegexOptions.None,
36+
TimeSpan.FromMilliseconds(100));
37+
2838
/// <summary>
2939
/// No Assertion value
3040
/// </summary>

‎src/DemaConsulting.SpdxModel/SpdxFile.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public void Validate(List<string> issues)
194194
issues.Add($"File {FileName} Invalid File Name Field");
195195

196196
// Validate File SPDX Identifier Field
197-
if (!Id.StartsWith("SPDXRef-"))
197+
if (!SpdxRefRegex.IsMatch(Id))
198198
issues.Add($"File {FileName} Invalid SPDX Identifier Field");
199199

200200
// Validate Checksums

‎src/DemaConsulting.SpdxModel/SpdxPackage.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,10 @@ public void Validate(List<string> issues, SpdxDocument? doc, bool ntia = false)
393393
if (Name.Length == 0)
394394
issues.Add("Package Invalid Package Name Field");
395395

396+
// SPDX NTIA Unique Identifier Check
397+
if (!SpdxRefRegex.IsMatch(Id))
398+
issues.Add($"Package {Name} Invalid SPDX Identifier Field");
399+
396400
// Validate Package Download Location Field
397401
if (DownloadLocation.Length == 0)
398402
issues.Add($"Package {Name} Invalid Package Download Location Field");
@@ -434,10 +438,6 @@ public void Validate(List<string> issues, SpdxDocument? doc, bool ntia = false)
434438
if (ntia && string.IsNullOrEmpty(Version))
435439
issues.Add($"NTIA: Package {Name} Missing Version");
436440

437-
// SPDX NTIA Unique Identifier Check
438-
if (ntia && string.IsNullOrEmpty(Id))
439-
issues.Add($"NTIA: Package {Name} Missing Unique Identifier");
440-
441441
// Release Date field
442442
if (!SpdxHelpers.IsValidSpdxDateTime(ReleaseDate))
443443
issues.Add($"Package {Name} Invalid Release Date Field");

‎src/DemaConsulting.SpdxModel/SpdxSnippet.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ public static SpdxSnippet[] Enhance(SpdxSnippet[] array, SpdxSnippet[] others)
192192
public void Validate(List<string> issues)
193193
{
194194
// Validate Snippet SPDX Identifier Field
195-
if (!Id.StartsWith("SPDXRef-"))
195+
if (!SpdxRefRegex.IsMatch(Id))
196196
issues.Add("Snippet Invalid SPDX Identifier Field");
197197

198198
// Validate Snippet From File Field

‎test/DemaConsulting.SpdxModel.Tests/SpdxFileTests.cs‎

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@ namespace DemaConsulting.SpdxModel.Tests;
2727
public class SpdxFileTests
2828
{
2929
/// <summary>
30-
/// Tests the <see cref="SpdxFile.Same"/> comparer.
30+
/// Tests the <see cref="SpdxFile.Same"/> comparer compares files correctly.
3131
/// </summary>
3232
[TestMethod]
33-
public void FileSameComparer()
33+
public void SpdxFile_SameComparer_ComparesCorrectly()
3434
{
35+
// Arrange: Create several SpdxFile instances with different IDs, names, and checksums
3536
var f1 = new SpdxFile
3637
{
3738
FileName = "./file1.txt",
@@ -44,7 +45,6 @@ public void FileSameComparer()
4445
}
4546
]
4647
};
47-
4848
var f2 = new SpdxFile
4949
{
5050
Id = "SPDXRef-File1",
@@ -64,7 +64,6 @@ public void FileSameComparer()
6464
],
6565
Comment = "File 1"
6666
};
67-
6867
var f3 = new SpdxFile
6968
{
7069
FileName = "./file2.txt",
@@ -78,29 +77,30 @@ public void FileSameComparer()
7877
]
7978
};
8079

81-
// Assert files compare to themselves
80+
// Assert: Verify files compare to themselves
8281
Assert.IsTrue(SpdxFile.Same.Equals(f1, f1));
8382
Assert.IsTrue(SpdxFile.Same.Equals(f2, f2));
8483
Assert.IsTrue(SpdxFile.Same.Equals(f3, f3));
8584

86-
// Assert files compare correctly
85+
// Assert: Verify files compare correctly
8786
Assert.IsTrue(SpdxFile.Same.Equals(f1, f2));
8887
Assert.IsTrue(SpdxFile.Same.Equals(f2, f1));
8988
Assert.IsFalse(SpdxFile.Same.Equals(f1, f3));
9089
Assert.IsFalse(SpdxFile.Same.Equals(f3, f1));
9190
Assert.IsFalse(SpdxFile.Same.Equals(f2, f3));
9291
Assert.IsFalse(SpdxFile.Same.Equals(f3, f2));
9392

94-
// Assert same files have identical hashes
93+
// Assert: Verify same files have identical hashes
9594
Assert.AreEqual(SpdxFile.Same.GetHashCode(f1), SpdxFile.Same.GetHashCode(f2));
9695
}
9796

9897
/// <summary>
99-
/// Tests the <see cref="SpdxFile.DeepCopy"/> method.
98+
/// Tests the <see cref="SpdxFile.DeepCopy"/> method successfully creates a deep copy.
10099
/// </summary>
101100
[TestMethod]
102-
public void DeepCopy()
101+
public void SpdxFile_DeepCopy_CreatesEqualButDistinctInstance()
103102
{
103+
// Arrange: Create an SpdxFile instance with checksums and comments
104104
var f1 = new SpdxFile
105105
{
106106
Id = "SPDXRef-File1",
@@ -121,27 +121,28 @@ public void DeepCopy()
121121
Comment = "File 1"
122122
};
123123

124-
// Make deep copy
124+
// Act: Create a deep copy of the SpdxFile instance
125125
var f2 = f1.DeepCopy();
126126

127-
// Assert both objects are equal
127+
// Assert: Verify deep-copy is equal to original
128128
Assert.AreEqual(f1, f2, SpdxFile.Same);
129129
Assert.AreEqual(f1.Id, f2.Id);
130130
Assert.AreEqual(f1.FileName, f2.FileName);
131131
CollectionAssert.AreEquivalent(f1.Checksums, f2.Checksums, SpdxChecksum.Same);
132132
Assert.AreEqual(f1.Comment, f2.Comment);
133133

134-
// Assert separate instances
134+
// Assert: Verify deep-copy has distinct instances
135135
Assert.IsFalse(ReferenceEquals(f1, f2));
136136
Assert.IsFalse(ReferenceEquals(f1.Checksums, f2.Checksums));
137137
}
138138

139139
/// <summary>
140-
/// Tests the <see cref="SpdxFile.Enhance(SpdxFile[], SpdxFile[])"/> method.
140+
/// Tests the <see cref="SpdxFile.Enhance(SpdxFile[], SpdxFile[])"/> method correctly adds or updates information
141141
/// </summary>
142142
[TestMethod]
143-
public void Enhance()
143+
public void SpdxFile_Enhance_AddsOrUpdatesInformationCorrectly()
144144
{
145+
// Arrange: Create an array of SpdxFile objects with one file
145146
var files = new[]
146147
{
147148
new SpdxFile
@@ -158,6 +159,7 @@ public void Enhance()
158159
}
159160
};
160161

162+
// Act: Enhance the files with additional information
161163
files = SpdxFile.Enhance(
162164
files,
163165
[
@@ -194,6 +196,7 @@ public void Enhance()
194196
}
195197
]);
196198

199+
// Assert: Verify the files array has been enhanced correctly
197200
Assert.AreEqual(2, files.Length);
198201
Assert.AreEqual("SPDXRef-File1", files[0].Id);
199202
Assert.AreEqual("./file1.txt", files[0].FileName);
@@ -210,7 +213,37 @@ public void Enhance()
210213
}
211214

212215
/// <summary>
213-
/// Tests the <see cref="SpdxFileTypeExtensions.FromText(string)"/> method.
216+
/// Tests that an invalid file ID fails validation.
217+
/// </summary>
218+
[TestMethod]
219+
public void SpdxFile_Validate_ReportsInvalidFileId()
220+
{
221+
// Arrange: Create an SpdxFile instance with an invalid ID format
222+
var spdxFile = new SpdxFile
223+
{
224+
Id = "Invalid_ID",
225+
FileName = "./file1.txt",
226+
Checksums =
227+
[
228+
new SpdxChecksum
229+
{
230+
Algorithm = SpdxChecksumAlgorithm.Sha1,
231+
Value = "85ed0817af83a24ad8da68c2b5094de69833983c"
232+
}
233+
]
234+
};
235+
236+
// Act: Perform validation on the SpdxFile instance.
237+
var issues = new List<string>();
238+
spdxFile.Validate(issues);
239+
240+
// Assert: Verify that the validation fails and the error message includes the invalid ID.
241+
Assert.IsTrue(
242+
issues.Any(issue => issue.Contains("File ./file1.txt Invalid SPDX Identifier Field")));
243+
}
244+
245+
/// <summary>
246+
/// Tests the <see cref="SpdxFileTypeExtensions.FromText(string)"/> method with valid inputs.
214247
/// </summary>
215248
[TestMethod]
216249
public void SpdxFileTypeExtensions_FromText_Valid()
@@ -231,7 +264,7 @@ public void SpdxFileTypeExtensions_FromText_Valid()
231264
}
232265

233266
/// <summary>
234-
/// Tests the <see cref="SpdxFileTypeExtensions.FromText(string)"/> method.
267+
/// Tests the <see cref="SpdxFileTypeExtensions.FromText(string)"/> method with invalid input.
235268
/// </summary>
236269
[TestMethod]
237270
public void SpdxFileTypeExtensions_FromText_Invalid()
@@ -241,7 +274,7 @@ public void SpdxFileTypeExtensions_FromText_Invalid()
241274
}
242275

243276
/// <summary>
244-
/// Tests the <see cref="SpdxFileTypeExtensions.ToText"/> method.
277+
/// Tests the <see cref="SpdxFileTypeExtensions.ToText"/> method with valid inputs
245278
/// </summary>
246279
[TestMethod]
247280
public void SpdxFileTypeExtensions_ToText_Valid()
@@ -260,7 +293,7 @@ public void SpdxFileTypeExtensions_ToText_Valid()
260293
}
261294

262295
/// <summary>
263-
/// Tests the <see cref="SpdxFileTypeExtensions.ToText"/> method.
296+
/// Tests the <see cref="SpdxFileTypeExtensions.ToText"/> method with invalid input.
264297
/// </summary>
265298
[TestMethod]
266299
public void SpdxFileTypeExtensions_ToText_Invalid()

‎test/DemaConsulting.SpdxModel.Tests/SpdxPackageTests.cs‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,61 +21,61 @@
2121
namespace DemaConsulting.SpdxModel.Tests;
2222

2323
/// <summary>
24-
/// Tests for the <see cref="SpdxPackage"/> class.
24+
/// Tests for the <see cref="SpdxPackage"/> class.
2525
/// </summary>
2626
[TestClass]
2727
public class SpdxPackageTests
2828
{
2929
/// <summary>
30-
/// Tests the <see cref="SpdxPackage.Same"/> comparer.
30+
/// Tests the <see cref="SpdxPackage.Same"/> comparer compares packages correctly.
3131
/// </summary>
3232
[TestMethod]
33-
public void PackageSameComparer()
33+
public void SpdxPackage_SameComparer_ComparesCorrectly()
3434
{
35+
// Arrange: Create several SpdxPackage instances with different IDs, names, and versions
3536
var p1 = new SpdxPackage
3637
{
3738
Id = "SPDXRef-Package1",
3839
Name = "DemaConsulting.SpdxModel",
3940
Version = "0.0.0"
4041
};
41-
4242
var p2 = new SpdxPackage
4343
{
4444
Id = "SPDXRef-Package-SpdxModel",
4545
Name = "DemaConsulting.SpdxModel",
4646
Version = "0.0.0"
4747
};
48-
4948
var p3 = new SpdxPackage
5049
{
5150
Id = "SPDXRef-Package1",
5251
Name = "SomePackage",
5352
Version = "1.2.3"
5453
};
5554

56-
// Assert packages compare to themselves
55+
// Assert: Verify packages compare to themselves
5756
Assert.IsTrue(SpdxPackage.Same.Equals(p1, p1));
5857
Assert.IsTrue(SpdxPackage.Same.Equals(p2, p2));
5958
Assert.IsTrue(SpdxPackage.Same.Equals(p3, p3));
6059

61-
// Assert packages compare correctly
60+
// Assert: Verify packages compare correctly
6261
Assert.IsTrue(SpdxPackage.Same.Equals(p1, p2));
6362
Assert.IsTrue(SpdxPackage.Same.Equals(p2, p1));
6463
Assert.IsFalse(SpdxPackage.Same.Equals(p1, p3));
6564
Assert.IsFalse(SpdxPackage.Same.Equals(p3, p1));
6665
Assert.IsFalse(SpdxPackage.Same.Equals(p2, p3));
6766
Assert.IsFalse(SpdxPackage.Same.Equals(p3, p2));
6867

69-
// Assert same packages have identical hashes
68+
// Assert: Verify same packages have identical hashes
7069
Assert.AreEqual(SpdxPackage.Same.GetHashCode(p1), SpdxPackage.Same.GetHashCode(p2));
7170
}
7271

7372
/// <summary>
74-
/// Tests the <see cref="SpdxPackage.DeepCopy"/> method.
73+
/// Tests the <see cref="SpdxPackage.DeepCopy"/> method successfully creates a deep copy.
7574
/// </summary>
7675
[TestMethod]
77-
public void DeepCopy()
76+
public void SpdxPackage_DeepCopy_CreatesEqualButDistinctInstance()
7877
{
78+
// Arrange: Create a SpdxPackage with various properties
7979
var p1 = new SpdxPackage
8080
{
8181
Id = "SPDXRef-Package1",
@@ -110,9 +110,10 @@ public void DeepCopy()
110110
]
111111
};
112112

113+
// Act: Create a deep copy of the SpdxPackage
113114
var p2 = p1.DeepCopy();
114115

115-
// Assert both objects are equal
116+
// Assert: Verify deep-copy is equal to original
116117
Assert.AreEqual(p1, p2, SpdxPackage.Same);
117118
Assert.AreEqual(p1.Id, p2.Id);
118119
Assert.AreEqual(p1.Name, p2.Name);
@@ -124,7 +125,7 @@ public void DeepCopy()
124125
CollectionAssert.AreEquivalent(p1.AttributionText, p2.AttributionText);
125126
CollectionAssert.AreEquivalent(p1.Annotations, p2.Annotations, SpdxAnnotation.Same);
126127

127-
// Assert separate instances
128+
// Assert: Verify deep-copy has distinct instances
128129
Assert.IsFalse(ReferenceEquals(p1, p2));
129130
Assert.IsFalse(ReferenceEquals(p1.HasFiles, p2.HasFiles));
130131
Assert.IsFalse(ReferenceEquals(p1.Checksums, p2.Checksums));
@@ -135,11 +136,12 @@ public void DeepCopy()
135136
}
136137

137138
/// <summary>
138-
/// Tests the <see cref="SpdxPackage.Enhance(SpdxPackage[], SpdxPackage[])"/> method.
139+
/// Tests the <see cref="SpdxPackage.Enhance(SpdxPackage[], SpdxPackage[])"/> method correctly adds or updates packages.
139140
/// </summary>
140141
[TestMethod]
141-
public void Enhance()
142+
public void SpdxPackage_Enhance_AddsOrUpdatesPackagesCorrectly()
142143
{
144+
// Arrange: Set up the initial packages and the packages to enhance with.
143145
var packages = new[]
144146
{
145147
new SpdxPackage
@@ -150,6 +152,7 @@ public void Enhance()
150152
}
151153
};
152154

155+
// Act: Call the Enhance method to add or update packages.
153156
packages = SpdxPackage.Enhance(
154157
packages,
155158
[
@@ -167,6 +170,7 @@ public void Enhance()
167170
}
168171
]);
169172

173+
// Assert: Verify the resulting packages are as expected.
170174
Assert.AreEqual(2, packages.Length);
171175
Assert.AreEqual("SPDXRef-Package1", packages[0].Id);
172176
Assert.AreEqual("DemaConsulting.SpdxModel", packages[0].Name);
@@ -175,4 +179,27 @@ public void Enhance()
175179
Assert.AreEqual("SomePackage", packages[1].Name);
176180
Assert.AreEqual("1.2.3", packages[1].Version);
177181
}
182+
183+
/// <summary>
184+
/// Tests that an invalid package ID fails validation.
185+
/// </summary>
186+
[TestMethod]
187+
public void SpdxPackage_Validate_ReportsInvalidPackageIds()
188+
{
189+
// Arrange: Construct a SpdxPackage with an invalid ID format
190+
var package = new SpdxPackage
191+
{
192+
Id = "Invalid_Id",
193+
Name = "TestPackage",
194+
Version = "1.0.0"
195+
};
196+
197+
// Act: Validate the package
198+
var issues = new List<string>();
199+
package.Validate(issues, null, true);
200+
201+
// Assert: Verify the invalid ID is reported
202+
Assert.IsTrue(
203+
issues.Any(i => i.StartsWith("Package TestPackage Invalid SPDX Identifier Field")));
204+
}
178205
}

0 commit comments

Comments
 (0)