Skip to content

Commit 9436718

Browse files
darrelmillerdarrelmiller
committed
Updated walker to avoid validating references twice
1 parent 2988c4b commit 9436718

File tree

5 files changed

+139
-31
lines changed

5 files changed

+139
-31
lines changed

‎Microsoft.OpenApi.sln

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{E546B92F-20A
2323
EndProject
2424
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{6357D7FD-2DE4-4900-ADB9-ABC37052040A}"
2525
EndProject
26-
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.OpenApi.SmokeTests", "test\Microsoft.OpenApi.SmokeTests\Microsoft.OpenApi.SmokeTests.csproj", "{AD79B61D-88CF-497C-9ED5-41AE3867C5AC}"
26+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.OpenApi.SmokeTests", "test\Microsoft.OpenApi.SmokeTests\Microsoft.OpenApi.SmokeTests.csproj", "{AD79B61D-88CF-497C-9ED5-41AE3867C5AC}"
2727
EndProject
2828
Global
2929
GlobalSection(SolutionConfigurationPlatforms) = preSolution

‎src/Microsoft.OpenApi.Readers/Services/OpenApiReferenceResolver.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ public override void Visit(OpenApiSecurityRequirement securityRequirement)
106106
{
107107
foreach (var scheme in securityRequirement.Keys.ToList())
108108
{
109-
ResolveObject(scheme, (resolvedScheme) => {
109+
ResolveObject(scheme, (resolvedScheme) =>
110+
{
110111
// If scheme was unresolved
111112
// copy Scopes and remove old unresolved scheme
112113
var scopes = securityRequirement[scheme];
@@ -176,7 +177,8 @@ private void ResolveTags(IList<OpenApiTag> tags)
176177

177178
if (IsUnresolvedReference(entity))
178179
{
179-
assign(ResolveReference<T>(entity.Reference));
180+
var x = ResolveReference<T>(entity.Reference);
181+
assign(x);
180182
}
181183
}
182184

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ internal void Walk(OpenApiCallback callback)
346346
/// </summary>
347347
internal void Walk(OpenApiTag tag)
348348
{
349-
if (tag == null)
349+
if (tag == null || IsReference(tag))
350350
{
351351
return;
352352
}
@@ -527,7 +527,7 @@ internal void Walk(IList<OpenApiParameter> parameters)
527527
/// </summary>
528528
internal void Walk(OpenApiParameter parameter)
529529
{
530-
if (parameter == null)
530+
if (parameter == null || IsReference(parameter))
531531
{
532532
return;
533533
}
@@ -567,7 +567,7 @@ internal void Walk(OpenApiResponses responses)
567567
/// </summary>
568568
internal void Walk(OpenApiResponse response)
569569
{
570-
if (response == null)
570+
if (response == null || IsReference(response))
571571
{
572572
return;
573573
}
@@ -580,13 +580,12 @@ internal void Walk(OpenApiResponse response)
580580
Walk(response as IOpenApiExtensible);
581581
}
582582

583-
584583
/// <summary>
585584
/// Visits <see cref="OpenApiRequestBody"/> and child objects
586585
/// </summary>
587586
internal void Walk(OpenApiRequestBody requestBody)
588587
{
589-
if (requestBody == null)
588+
if (requestBody == null || IsReference(requestBody))
590589
{
591590
return;
592591
}
@@ -668,7 +667,7 @@ internal void Walk(IDictionary<string, OpenApiMediaType> content)
668667
/// </summary>
669668
internal void Walk(OpenApiMediaType mediaType)
670669
{
671-
if (mediaType == null)
670+
if (mediaType == null)
672671
{
673672
return;
674673
}
@@ -721,7 +720,7 @@ internal void Walk(OpenApiEncoding encoding)
721720
/// </summary>
722721
internal void Walk(OpenApiSchema schema)
723722
{
724-
if (schema == null)
723+
if (schema == null || IsReference(schema))
725724
{
726725
return;
727726
}
@@ -806,7 +805,7 @@ internal void Walk(IOpenApiAny example)
806805
/// </summary>
807806
internal void Walk(OpenApiExample example)
808807
{
809-
if (example == null)
808+
if (example == null || IsReference(example))
810809
{
811810
return;
812811
}
@@ -910,7 +909,7 @@ internal void Walk(IDictionary<string,OpenApiLink> links)
910909
/// </summary>
911910
internal void Walk(OpenApiLink link)
912911
{
913-
if (link == null)
912+
if (link == null || IsReference(link))
914913
{
915914
return;
916915
}
@@ -925,7 +924,7 @@ internal void Walk(OpenApiLink link)
925924
/// </summary>
926925
internal void Walk(OpenApiHeader header)
927926
{
928-
if (header == null)
927+
if (header == null || IsReference(header))
929928
{
930929
return;
931930
}
@@ -957,7 +956,7 @@ internal void Walk(OpenApiSecurityRequirement securityRequirement)
957956
/// </summary>
958957
internal void Walk(OpenApiSecurityScheme securityScheme)
959958
{
960-
if (securityScheme == null)
959+
if (securityScheme == null || IsReference(securityScheme))
961960
{
962961
return;
963962
}
@@ -1023,7 +1022,12 @@ private void Walk(string context, Action walk)
10231022
_visitor.Exit();
10241023
}
10251024

1026-
1027-
1025+
/// <summary>
1026+
/// Identify if an element is just a reference to a component, or an actual component
1027+
/// </summary>
1028+
private bool IsReference(IOpenApiReferenceable referenceable)
1029+
{
1030+
return referenceable.Reference != null && !_visitor.InComponents;
1031+
}
10281032
}
10291033
}

‎src/Microsoft.OpenApi/Validations/OpenApiValidator.cs

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ public void AddError(OpenApiValidatorError error)
119119
/// <param name="item">The object to be validated</param>
120120
public override void Visit(OpenApiSchema item) => Validate(item);
121121

122-
123122
/// <summary>
124123
/// Execute validation rules against an <see cref="OpenApiServer"/>
125124
/// </summary>
@@ -170,23 +169,14 @@ private void Validate<T>(T item)
170169
private void Validate(object item, Type type)
171170
{
172171
if (item == null) return; // Required fields should be checked by higher level objects
173-
var potentialReference = item as IOpenApiReferenceable;
174172

175-
if (potentialReference != null && potentialReference.Reference != null)
173+
// Validate unresolved references as references
174+
var potentialReference = item as IOpenApiReferenceable;
175+
if (potentialReference != null && potentialReference.UnresolvedReference)
176176
{
177-
if (potentialReference.UnresolvedReference)
178-
{
179-
return; // Don't attempt to validate unresolved references
180-
}
181-
else
182-
{
183-
if (potentialReference.Reference != null && !InComponents)
184-
{
185-
// Don't validate references if they are in not in Components to avoid validating on every reference
186-
return;
187-
}
188-
}
177+
type = typeof(IOpenApiReferenceable);
189178
}
179+
190180
var rules = _ruleSet.FindRules(type);
191181
foreach (var rule in rules)
192182
{

‎test/Microsoft.OpenApi.Tests/Validations/OpenApiReferenceValidationTests.cs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Text;
5+
using System.Threading.Tasks;
6+
using Microsoft.OpenApi.Extensions;
7+
using Microsoft.OpenApi.Interfaces;
8+
using Microsoft.OpenApi.Models;
9+
using Microsoft.OpenApi.Validations;
10+
using Xunit;
11+
12+
namespace Microsoft.OpenApi.Tests.Validations
13+
{
14+
public class OpenApiReferenceValidationTests
15+
{
16+
[Fact]
17+
public void ReferencedSchemaShouldOnlyBeValidatedOnce()
18+
{
19+
// Arrange
20+
21+
var sharedSchema = new OpenApiSchema
22+
{
23+
Type = "string",
24+
Reference = new OpenApiReference()
25+
{
26+
Id = "test"
27+
},
28+
UnresolvedReference = false
29+
};
30+
31+
OpenApiDocument document = new OpenApiDocument();
32+
document.Components = new OpenApiComponents()
33+
{
34+
Schemas = new Dictionary<string, OpenApiSchema>()
35+
{
36+
[sharedSchema.Reference.Id] = sharedSchema
37+
}
38+
};
39+
40+
document.Paths = new OpenApiPaths()
41+
{
42+
["/"] = new OpenApiPathItem()
43+
{
44+
Operations = new Dictionary<OperationType,OpenApiOperation>
45+
{
46+
[OperationType.Get] = new OpenApiOperation()
47+
{
48+
Responses = new OpenApiResponses()
49+
{
50+
["200"] = new OpenApiResponse()
51+
{
52+
Content = new Dictionary<string,OpenApiMediaType>()
53+
{
54+
["application/json"] = new OpenApiMediaType()
55+
{
56+
Schema = sharedSchema
57+
}
58+
}
59+
}
60+
}
61+
}
62+
}
63+
}
64+
};
65+
66+
// Act
67+
var errors = document.Validate(new ValidationRuleSet() { new AllwaysFailRule<OpenApiSchema>()});
68+
69+
70+
// Assert
71+
Assert.True(errors.Count() == 1);
72+
}
73+
[Fact]
74+
public void UnResolvedReferencedSchemaShouldNotBeValidated()
75+
{
76+
// Arrange
77+
var sharedSchema = new OpenApiSchema
78+
{
79+
Type = "string",
80+
Reference = new OpenApiReference()
81+
{
82+
Id = "test"
83+
},
84+
UnresolvedReference = true
85+
};
86+
87+
OpenApiDocument document = new OpenApiDocument();
88+
document.Components = new OpenApiComponents()
89+
{
90+
Schemas = new Dictionary<string, OpenApiSchema>()
91+
{
92+
[sharedSchema.Reference.Id] = sharedSchema
93+
}
94+
};
95+
96+
// Act
97+
var errors = document.Validate(new ValidationRuleSet() { new AllwaysFailRule<OpenApiSchema>() });
98+
99+
// Assert
100+
Assert.True(errors.Count() == 0);
101+
}
102+
103+
}
104+
105+
public class AllwaysFailRule<P> : ValidationRule<P> where P: IOpenApiElement
106+
{
107+
public AllwaysFailRule() : base( (c,t) => c.CreateError("x","y"))
108+
{
109+
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)