From 8d4f9848240461863b67487add02311c0ecac763 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Thu, 3 Jun 2021 15:00:08 +0300 Subject: [PATCH 1/3] 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. --- .../Services/OpenApiUrlTreeNodeTests.cs | 209 +++++++----------- 1 file changed, 86 insertions(+), 123 deletions(-) diff --git a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs index a246c66ff..2faabda36 100644 --- a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs +++ b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs @@ -11,6 +11,26 @@ namespace Microsoft.OpenApi.Tests.Services { public class OpenApiUrlTreeNodeTests { + private OpenApiDocument OpenApiDocumentSample_1 => new OpenApiDocument() + { + Paths = new OpenApiPaths() + { + ["/"] = new OpenApiPathItem(), + ["/houses"] = new OpenApiPathItem(), + ["/cars"] = new OpenApiPathItem() + } + }; + + private OpenApiDocument OpenApiDocumentSample_2 => new OpenApiDocument() + { + Paths = new OpenApiPaths() + { + ["/"] = new OpenApiPathItem(), + ["/hotels"] = new OpenApiPathItem(), + ["/offices"] = new OpenApiPathItem() + } + }; + [Fact] public void CreateUrlSpaceWithoutOpenApiDocument() { @@ -64,15 +84,7 @@ public void CreatePathWithoutRootWorks() [Fact] public void CreateMultiplePathsWorks() { - var doc = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; + var doc = OpenApiDocumentSample_1; string label = "assets"; var rootNode = OpenApiUrlTreeNode.Create(doc, label); @@ -89,25 +101,9 @@ public void CreateMultiplePathsWorks() [Fact] public void AttachDocumentWorks() { - var doc1 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; + var doc1 = OpenApiDocumentSample_1; - var doc2 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/hotels"] = new OpenApiPathItem(), - ["/offices"] = new OpenApiPathItem() - } - }; + var doc2 = OpenApiDocumentSample_2; var label1 = "personal"; var label2 = "business"; @@ -123,15 +119,7 @@ public void AttachDocumentWorks() [Fact] public void AttachPathWorks() { - var doc = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; + var doc = OpenApiDocumentSample_1; var label1 = "personal"; var rootNode = OpenApiUrlTreeNode.Create(doc, label1); @@ -335,96 +323,10 @@ public void SegmentIsParameterWorks() Assert.Equal("{apartment-id}", rootNode.Children["houses"].Children["apartments"].Children["{apartment-id}"].Segment); } - [Fact] - public void ThrowsArgumentExceptionForDuplicateLabels() - { - var doc1 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; - - var doc2 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/hotels"] = new OpenApiPathItem(), - ["/offices"] = new OpenApiPathItem() - } - }; - - var label1 = "personal"; - var rootNode = OpenApiUrlTreeNode.Create(doc1, label1); - - Assert.Throws(() => rootNode.Attach(doc2, label1)); - } - - [Fact] - public void ThrowsArgumentNullExceptionForNullArgumentsInCreateMethod() - { - var doc = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; - - Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, "")); - Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, null)); - Assert.Throws(() => OpenApiUrlTreeNode.Create(null, "beta")); - } - - [Fact] - public void ThrowsArgumentNullExceptionForNullArgumentsInAttachMethod() - { - var doc1 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; - - var doc2 = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/hotels"] = new OpenApiPathItem(), - ["/offices"] = new OpenApiPathItem() - } - }; - - var label1 = "personal"; - var rootNode = OpenApiUrlTreeNode.Create(doc1, label1); - - Assert.Throws(() => rootNode.Attach(doc2, "")); - Assert.Throws(() => rootNode.Attach(doc2, null)); - Assert.Throws(() => rootNode.Attach(null, "beta")); - } - [Fact] public void AdditionalDataWorks() { - var doc = new OpenApiDocument() - { - Paths = new OpenApiPaths() - { - ["/"] = new OpenApiPathItem(), - ["/houses"] = new OpenApiPathItem(), - ["/cars"] = new OpenApiPathItem() - } - }; + var doc = OpenApiDocumentSample_1; var label = "personal"; var rootNode = OpenApiUrlTreeNode.Create(doc, label); @@ -476,5 +378,66 @@ public void AdditionalDataWorks() Assert.Equal("Convertible", item); }); } + + [Fact] + public void ThrowsArgumentExceptionForDuplicateLabels() + { + var doc1 = OpenApiDocumentSample_1; + + var doc2 = OpenApiDocumentSample_2; + + var label1 = "personal"; + var rootNode = OpenApiUrlTreeNode.Create(doc1, label1); + + Assert.Throws(() => rootNode.Attach(doc2, label1)); + } + + [Fact] + public void ThrowsArgumentNullExceptionForNullArgumentsInCreateMethod() + { + var doc = OpenApiDocumentSample_1; + + Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, "")); + Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, null)); + Assert.Throws(() => OpenApiUrlTreeNode.Create(null, "beta")); + } + + [Fact] + public void ThrowsArgumentNullExceptionForNullArgumentsInAttachMethod() + { + var doc1 = OpenApiDocumentSample_1; + + var doc2 = OpenApiDocumentSample_2; + + var label1 = "personal"; + var rootNode = OpenApiUrlTreeNode.Create(doc1, label1); + + Assert.Throws(() => rootNode.Attach(doc2, "")); + Assert.Throws(() => rootNode.Attach(doc2, null)); + Assert.Throws(() => rootNode.Attach(null, "beta")); + } + + [Fact] + public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentInHasOperationsMethod() + { + var doc = OpenApiDocumentSample_1; + + var label = "personal"; + var rootNode = OpenApiUrlTreeNode.Create(doc, label); + + Assert.Throws(() => rootNode.HasOperations(null)); + Assert.Throws(() => rootNode.HasOperations("")); + } + + [Fact] + public void ThrowsArgumentNullExceptionForNullArgumentInAddAdditionalDataMethod() + { + var doc = OpenApiDocumentSample_1; + + var label = "personal"; + var rootNode = OpenApiUrlTreeNode.Create(doc, label); + + Assert.Throws(() => rootNode.AddAdditionalData(null)); + } } } From 87f3daf87b2e7e8d291de243f033cc2e8f173b9b Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Thu, 3 Jun 2021 15:04:37 +0300 Subject: [PATCH 2/3] Defensive programming; XML documentation comment update --- src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs b/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs index 296068914..30a47bdd7 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs @@ -47,11 +47,16 @@ public class OpenApiUrlTreeNode public string Segment { get; private set; } /// - /// Flag indicating whether the node's PathItems has operations. + /// Flag indicating whether the node's PathItems dictionary has operations + /// under a given label. /// + /// The name of the key for the target operations + /// in the node's PathItems dictionary. /// true or false. public bool HasOperations(string label) { + Utils.CheckArgumentNullOrEmpty(label, nameof(label)); + if (!(PathItems?.ContainsKey(label) ?? false)) { return false; @@ -139,6 +144,8 @@ public OpenApiUrlTreeNode Attach(string path, string label) { Utils.CheckArgumentNullOrEmpty(label, nameof(label)); + Utils.CheckArgumentNullOrEmpty(path, nameof(path)); + Utils.CheckArgumentNull(pathItem, nameof(pathItem)); if (path.StartsWith(RootPathSegment)) { From dcc76fec800448a7f56a7c93535d4b57a4ad5eb5 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Mon, 7 Jun 2021 11:18:51 +0300 Subject: [PATCH 3/3] Add new tests to cover all edge cases; rename two tests appropriately --- .../Services/OpenApiUrlTreeNodeTests.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs index 2faabda36..944e6c830 100644 --- a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs +++ b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs @@ -393,17 +393,19 @@ public void ThrowsArgumentExceptionForDuplicateLabels() } [Fact] - public void ThrowsArgumentNullExceptionForNullArgumentsInCreateMethod() + public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentsInCreateMethod() { var doc = OpenApiDocumentSample_1; Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, "")); Assert.Throws(() => OpenApiUrlTreeNode.Create(doc, null)); Assert.Throws(() => OpenApiUrlTreeNode.Create(null, "beta")); + Assert.Throws(() => OpenApiUrlTreeNode.Create(null, null)); + Assert.Throws(() => OpenApiUrlTreeNode.Create(null, "")); } [Fact] - public void ThrowsArgumentNullExceptionForNullArgumentsInAttachMethod() + public void ThrowsArgumentNullExceptionForNullOrEmptyArgumentsInAttachMethod() { var doc1 = OpenApiDocumentSample_1; @@ -415,6 +417,8 @@ public void ThrowsArgumentNullExceptionForNullArgumentsInAttachMethod() Assert.Throws(() => rootNode.Attach(doc2, "")); Assert.Throws(() => rootNode.Attach(doc2, null)); Assert.Throws(() => rootNode.Attach(null, "beta")); + Assert.Throws(() => rootNode.Attach(null, null)); + Assert.Throws(() => rootNode.Attach(null, "")); } [Fact]