Skip to content

Commit 2b3abef

Browse files
irvinesundayirvinesunday
authored
Adds nullability check to HasOperations method and updates/refactors tests for OpenApiUrlTreeNode (#604)
* Refactor out common code into re-usable properties This helps with simplifying the tests since we are abstracting out the plumbing work required for some of the tests. Since the code is repetitive, this can be refactored out and reused. * Defensive programming; XML documentation comment update * Add new tests to cover all edge cases; rename two tests appropriately Co-authored-by: Irvine Sunday <[email protected]>
1 parent a008173 commit 2b3abef

File tree

2 files changed

+98
-124
lines changed

2 files changed

+98
-124
lines changed

‎src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,16 @@ public class OpenApiUrlTreeNode
4747
public string Segment { get; private set; }
4848

4949
/// <summary>
50-
/// Flag indicating whether the node's PathItems has operations.
50+
/// Flag indicating whether the node's PathItems dictionary has operations
51+
/// under a given label.
5152
/// </summary>
53+
/// <param name="label">The name of the key for the target operations
54+
/// in the node's PathItems dictionary.</param>
5255
/// <returns>true or false.</returns>
5356
public bool HasOperations(string label)
5457
{
58+
Utils.CheckArgumentNullOrEmpty(label, nameof(label));
59+
5560
if (!(PathItems?.ContainsKey(label) ?? false))
5661
{
5762
return false;
@@ -139,6 +144,8 @@ public OpenApiUrlTreeNode Attach(string path,
139144
string label)
140145
{
141146
Utils.CheckArgumentNullOrEmpty(label, nameof(label));
147+
Utils.CheckArgumentNullOrEmpty(path, nameof(path));
148+
Utils.CheckArgumentNull(pathItem, nameof(pathItem));
142149

143150
if (path.StartsWith(RootPathSegment))
144151
{

‎test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs

Lines changed: 90 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,26 @@ namespace Microsoft.OpenApi.Tests.Services
1111
{
1212
public class OpenApiUrlTreeNodeTests
1313
{
14+
private OpenApiDocument OpenApiDocumentSample_1 => new OpenApiDocument()
15+
{
16+
Paths = new OpenApiPaths()
17+
{
18+
["/"] = new OpenApiPathItem(),
19+
["/houses"] = new OpenApiPathItem(),
20+
["/cars"] = new OpenApiPathItem()
21+
}
22+
};
23+
24+
private OpenApiDocument OpenApiDocumentSample_2 => new OpenApiDocument()
25+
{
26+
Paths = new OpenApiPaths()
27+
{
28+
["/"] = new OpenApiPathItem(),
29+
["/hotels"] = new OpenApiPathItem(),
30+
["/offices"] = new OpenApiPathItem()
31+
}
32+
};
33+
1434
[Fact]
1535
public void CreateUrlSpaceWithoutOpenApiDocument()
1636
{
@@ -64,15 +84,7 @@ public void CreatePathWithoutRootWorks()
6484
[Fact]
6585
public void CreateMultiplePathsWorks()
6686
{
67-
var doc = new OpenApiDocument()
68-
{
69-
Paths = new OpenApiPaths()
70-
{
71-
["/"] = new OpenApiPathItem(),
72-
["/houses"] = new OpenApiPathItem(),
73-
["/cars"] = new OpenApiPathItem()
74-
}
75-
};
87+
var doc = OpenApiDocumentSample_1;
7688

7789
string label = "assets";
7890
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
@@ -89,25 +101,9 @@ public void CreateMultiplePathsWorks()
89101
[Fact]
90102
public void AttachDocumentWorks()
91103
{
92-
var doc1 = new OpenApiDocument()
93-
{
94-
Paths = new OpenApiPaths()
95-
{
96-
["/"] = new OpenApiPathItem(),
97-
["/houses"] = new OpenApiPathItem(),
98-
["/cars"] = new OpenApiPathItem()
99-
}
100-
};
104+
var doc1 = OpenApiDocumentSample_1;
101105

102-
var doc2 = new OpenApiDocument()
103-
{
104-
Paths = new OpenApiPaths()
105-
{
106-
["/"] = new OpenApiPathItem(),
107-
["/hotels"] = new OpenApiPathItem(),
108-
["/offices"] = new OpenApiPathItem()
109-
}
110-
};
106+
var doc2 = OpenApiDocumentSample_2;
111107

112108
var label1 = "personal";
113109
var label2 = "business";
@@ -123,15 +119,7 @@ public void AttachDocumentWorks()
123119
[Fact]
124120
public void AttachPathWorks()
125121
{
126-
var doc = new OpenApiDocument()
127-
{
128-
Paths = new OpenApiPaths()
129-
{
130-
["/"] = new OpenApiPathItem(),
131-
["/houses"] = new OpenApiPathItem(),
132-
["/cars"] = new OpenApiPathItem()
133-
}
134-
};
122+
var doc = OpenApiDocumentSample_1;
135123

136124
var label1 = "personal";
137125
var rootNode = OpenApiUrlTreeNode.Create(doc, label1);
@@ -335,96 +323,10 @@ public void SegmentIsParameterWorks()
335323
Assert.Equal("{apartment-id}", rootNode.Children["houses"].Children["apartments"].Children["{apartment-id}"].Segment);
336324
}
337325

338-
[Fact]
339-
public void ThrowsArgumentExceptionForDuplicateLabels()
340-
{
341-
var doc1 = new OpenApiDocument()
342-
{
343-
Paths = new OpenApiPaths()
344-
{
345-
["/"] = new OpenApiPathItem(),
346-
["/houses"] = new OpenApiPathItem(),
347-
["/cars"] = new OpenApiPathItem()
348-
}
349-
};
350-
351-
var doc2 = new OpenApiDocument()
352-
{
353-
Paths = new OpenApiPaths()
354-
{
355-
["/"] = new OpenApiPathItem(),
356-
["/hotels"] = new OpenApiPathItem(),
357-
["/offices"] = new OpenApiPathItem()
358-
}
359-
};
360-
361-
var label1 = "personal";
362-
var rootNode = OpenApiUrlTreeNode.Create(doc1, label1);
363-
364-
Assert.Throws<ArgumentException>(() => rootNode.Attach(doc2, label1));
365-
}
366-
367-
[Fact]
368-
public void ThrowsArgumentNullExceptionForNullArgumentsInCreateMethod()
369-
{
370-
var doc = new OpenApiDocument()
371-
{
372-
Paths = new OpenApiPaths()
373-
{
374-
["/"] = new OpenApiPathItem(),
375-
["/houses"] = new OpenApiPathItem(),
376-
["/cars"] = new OpenApiPathItem()
377-
}
378-
};
379-
380-
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(doc, ""));
381-
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(doc, null));
382-
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(null, "beta"));
383-
}
384-
385-
[Fact]
386-
public void ThrowsArgumentNullExceptionForNullArgumentsInAttachMethod()
387-
{
388-
var doc1 = new OpenApiDocument()
389-
{
390-
Paths = new OpenApiPaths()
391-
{
392-
["/"] = new OpenApiPathItem(),
393-
["/houses"] = new OpenApiPathItem(),
394-
["/cars"] = new OpenApiPathItem()
395-
}
396-
};
397-
398-
var doc2 = new OpenApiDocument()
399-
{
400-
Paths = new OpenApiPaths()
401-
{
402-
["/"] = new OpenApiPathItem(),
403-
["/hotels"] = new OpenApiPathItem(),
404-
["/offices"] = new OpenApiPathItem()
405-
}
406-
};
407-
408-
var label1 = "personal";
409-
var rootNode = OpenApiUrlTreeNode.Create(doc1, label1);
410-
411-
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(doc2, ""));
412-
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(doc2, null));
413-
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(null, "beta"));
414-
}
415-
416326
[Fact]
417327
public void AdditionalDataWorks()
418328
{
419-
var doc = new OpenApiDocument()
420-
{
421-
Paths = new OpenApiPaths()
422-
{
423-
["/"] = new OpenApiPathItem(),
424-
["/houses"] = new OpenApiPathItem(),
425-
["/cars"] = new OpenApiPathItem()
426-
}
427-
};
329+
var doc = OpenApiDocumentSample_1;
428330

429331
var label = "personal";
430332
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
@@ -476,5 +378,70 @@ public void AdditionalDataWorks()
476378
Assert.Equal("Convertible", item);
477379
});
478380
}
381+
382+
[Fact]
383+
public void ThrowsArgumentExceptionForDuplicateLabels()
384+
{
385+
var doc1 = OpenApiDocumentSample_1;
386+
387+
var doc2 = OpenApiDocumentSample_2;
388+
389+
var label1 = "personal";
390+
var rootNode = OpenApiUrlTreeNode.Create(doc1, label1);
391+
392+
Assert.Throws<ArgumentException>(() => rootNode.Attach(doc2, label1));
393+
}
394+
395+
[Fact]
396+
public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentsInCreateMethod()
397+
{
398+
var doc = OpenApiDocumentSample_1;
399+
400+
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(doc, ""));
401+
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(doc, null));
402+
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(null, "beta"));
403+
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(null, null));
404+
Assert.Throws<ArgumentNullException>(() => OpenApiUrlTreeNode.Create(null, ""));
405+
}
406+
407+
[Fact]
408+
public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentsInAttachMethod()
409+
{
410+
var doc1 = OpenApiDocumentSample_1;
411+
412+
var doc2 = OpenApiDocumentSample_2;
413+
414+
var label1 = "personal";
415+
var rootNode = OpenApiUrlTreeNode.Create(doc1, label1);
416+
417+
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(doc2, ""));
418+
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(doc2, null));
419+
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(null, "beta"));
420+
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(null, null));
421+
Assert.Throws<ArgumentNullException>(() => rootNode.Attach(null, ""));
422+
}
423+
424+
[Fact]
425+
public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentInHasOperationsMethod()
426+
{
427+
var doc = OpenApiDocumentSample_1;
428+
429+
var label = "personal";
430+
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
431+
432+
Assert.Throws<ArgumentNullException>(() => rootNode.HasOperations(null));
433+
Assert.Throws<ArgumentNullException>(() => rootNode.HasOperations(""));
434+
}
435+
436+
[Fact]
437+
public void ThrowsArgumentNullExceptionForNullArgumentInAddAdditionalDataMethod()
438+
{
439+
var doc = OpenApiDocumentSample_1;
440+
441+
var label = "personal";
442+
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
443+
444+
Assert.Throws<ArgumentNullException>(() => rootNode.AddAdditionalData(null));
445+
}
479446
}
480447
}

0 commit comments

Comments
 (0)