Skip to content

Commit 2a7e87c

Browse files
irvinesundaybaywet
irvinesunday
and
baywet
authored
Use CRUD restrictions annotations for the generation of complex properties' paths (#180)
* Remove Delete operations for complex property paths * Remove Delete operation for complex property paths again * Add setting for toggling use of restrictions to generate complex property paths * Use restrictions annotations to generate complex property paths * Update tests to validate use of restrictions annotations to generate complex property paths * Minor xml summary update * Code refactoring; update xml summary * Update tests * Update integration test files * Minor spacing edits * Apply suggestions from code review * Code review suggestion Co-authored-by: Vincent Biret <[email protected]>
1 parent 6e96f51 commit 2a7e87c

26 files changed

+861
-11982
lines changed

‎src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ private void RetrieveComplexPropertyPaths(IEdmEntityType entityType, ODataPath c
247247
.Where(x => x.Type.IsComplex() ||
248248
x.Type.IsCollection() && x.Type.Definition.AsElementType() is IEdmComplexType))
249249
{
250+
if (!ShouldCreateComplexPropertyPaths(sp, convertSettings)) continue;
251+
250252
currentPath.Push(new ODataComplexPropertySegment(sp));
251253
AppendPath(currentPath.Clone());
252254

@@ -280,6 +282,27 @@ private void RetrieveComplexPropertyPaths(IEdmEntityType entityType, ODataPath c
280282
}
281283
}
282284

285+
/// <summary>
286+
/// Evaluates whether or not to create paths for complex properties.
287+
/// </summary>
288+
/// <param name="complexProperty">The target complex property.</param>
289+
/// <param name="convertSettings">The settings for the current conversion.</param>
290+
/// <returns>true or false.</returns>
291+
private bool ShouldCreateComplexPropertyPaths(IEdmStructuralProperty complexProperty, OpenApiConvertSettings convertSettings)
292+
{
293+
Debug.Assert(complexProperty != null);
294+
Debug.Assert(convertSettings != null);
295+
296+
if (!convertSettings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties)
297+
return true;
298+
299+
bool isReadable = _model.GetRecord<ReadRestrictionsType>(complexProperty, CapabilitiesConstants.ReadRestrictions)?.Readable ?? false;
300+
bool isUpdatable = _model.GetRecord<UpdateRestrictionsType>(complexProperty, CapabilitiesConstants.UpdateRestrictions)?.Updatable ?? false;
301+
bool isInsertable = _model.GetRecord<InsertRestrictionsType>(complexProperty, CapabilitiesConstants.InsertRestrictions)?.Insertable ?? false;
302+
303+
return isReadable || isUpdatable || isInsertable;
304+
}
305+
283306
/// <summary>
284307
/// Retrieves the paths for a media entity stream.
285308
/// </summary>

‎src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,11 @@ public string PathPrefix
225225
/// </summary>
226226
public string InnerErrorComplexTypeName { get; set; } = "InnerError";
227227

228+
/// <summary>
229+
/// Gets/Sets a value indicating whether or not to use restrictions annotations to generate paths for complex properties.
230+
/// </summary>
231+
public bool UseRestrictionAnnotationsToGeneratePathsForComplexProperties { get; set; } = true;
232+
228233
internal OpenApiConvertSettings Clone()
229234
{
230235
var newSettings = new OpenApiConvertSettings
@@ -262,7 +267,8 @@ internal OpenApiConvertSettings Clone()
262267
EnableDeprecationInformation = this.EnableDeprecationInformation,
263268
AddEnumDescriptionExtension = this.AddEnumDescriptionExtension,
264269
ErrorResponsesAsDefault = this.ErrorResponsesAsDefault,
265-
InnerErrorComplexTypeName = this.InnerErrorComplexTypeName
270+
InnerErrorComplexTypeName = this.InnerErrorComplexTypeName,
271+
UseRestrictionAnnotationsToGeneratePathsForComplexProperties = this.UseRestrictionAnnotationsToGeneratePathsForComplexProperties
266272
};
267273

268274
return newSettings;

‎src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyDeleteOperationHandler.cs‎

Lines changed: 0 additions & 90 deletions
This file was deleted.

‎src/Microsoft.OpenApi.OData.Reader/Operation/OperationHandlerProvider.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,8 @@ private readonly IDictionary<ODataPathKind, IDictionary<OperationType, IOperatio
104104
{OperationType.Get, new ComplexPropertyGetOperationHandler() },
105105
{OperationType.Patch, new ComplexPropertyPatchOperationHandler() },
106106
{OperationType.Put, new ComplexPropertyPutOperationHandler() },
107-
{OperationType.Post, new ComplexPropertyPostOperationHandler() },
108-
{OperationType.Delete, new ComplexPropertyDeleteOperationHandler() },
109-
}},
107+
{OperationType.Post, new ComplexPropertyPostOperationHandler() }
108+
}}
110109
};
111110

112111
/// <inheritdoc/>

‎src/Microsoft.OpenApi.OData.Reader/PathItem/ComplexPropertyItemHandler.cs‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,37 @@ internal class ComplexPropertyItemHandler : PathItemHandler
2323
/// <inheritdoc/>
2424
protected override void SetOperations(OpenApiPathItem item)
2525
{
26-
AddOperation(item, OperationType.Get);
26+
bool isReadable = Context.Model.GetRecord<ReadRestrictionsType>(ComplexProperty, CapabilitiesConstants.ReadRestrictions)?.Readable ?? false;
27+
if ((Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties && isReadable) ||
28+
!Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties)
29+
{
30+
AddOperation(item, OperationType.Get);
31+
}
2732

28-
UpdateRestrictionsType update = Context.Model.GetRecord<UpdateRestrictionsType>(ComplexProperty);
29-
if (update != null && update.IsUpdateMethodPut)
33+
UpdateRestrictionsType update = Context.Model.GetRecord<UpdateRestrictionsType>(ComplexProperty, CapabilitiesConstants.UpdateRestrictions);
34+
bool isUpdatable = update?.Updatable ?? false;
35+
if ((Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties && isUpdatable) ||
36+
!Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties)
3037
{
31-
AddOperation(item, OperationType.Put);
32-
}
33-
else
34-
{
35-
AddOperation(item, OperationType.Patch);
38+
if (update != null && update.IsUpdateMethodPut)
39+
{
40+
AddOperation(item, OperationType.Put);
41+
}
42+
else
43+
{
44+
AddOperation(item, OperationType.Patch);
45+
}
3646
}
3747

38-
if(Path.LastSegment is ODataComplexPropertySegment segment)
39-
{
40-
if(segment.Property.Type.IsNullable)
41-
AddOperation(item, OperationType.Delete);
42-
if(segment.Property.Type.IsCollection())
48+
if (Path.LastSegment is ODataComplexPropertySegment segment && segment.Property.Type.IsCollection())
49+
{
50+
bool isInsertable = Context.Model.GetRecord<InsertRestrictionsType>(ComplexProperty, CapabilitiesConstants.InsertRestrictions)?.Insertable ?? false;
51+
if ((Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties && isInsertable) ||
52+
!Context.Settings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties)
53+
{
4354
AddOperation(item, OperationType.Post);
44-
}
55+
}
56+
}
4557
}
4658

4759
/// <inheritdoc/>

‎src/Microsoft.OpenApi.OData.Reader/PublicAPI.Unshipped.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ abstract Microsoft.OpenApi.OData.Edm.ODataSegment.GetAnnotables() -> System.Coll
55
Microsoft.OpenApi.OData.Common.Utils
66
Microsoft.OpenApi.OData.Edm.EdmModelExtensions
77
Microsoft.OpenApi.OData.Edm.EdmTypeExtensions
8+
Microsoft.OpenApi.OData.OpenApiConvertSettings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties.get -> bool
9+
Microsoft.OpenApi.OData.OpenApiConvertSettings.UseRestrictionAnnotationsToGeneratePathsForComplexProperties.set -> void
810
static Microsoft.OpenApi.OData.Edm.EdmTypeExtensions.ShouldPathParameterBeQuoted(this Microsoft.OData.Edm.IEdmType edmType, Microsoft.OpenApi.OData.OpenApiConvertSettings settings) -> bool
911
Microsoft.OpenApi.OData.Edm.IODataPathProvider
1012
Microsoft.OpenApi.OData.Edm.IODataPathProvider.CanFilter(Microsoft.OData.Edm.IEdmElement element) -> bool
@@ -134,8 +136,6 @@ Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUnqualifiedCall.get -> bool
134136
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUnqualifiedCall.set -> void
135137
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUriEscapeFunctionCall.get -> bool
136138
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUriEscapeFunctionCall.set -> void
137-
Microsoft.OpenApi.OData.OpenApiConvertSettings.ExpandNavigationPropertyAttributeName.get -> string
138-
Microsoft.OpenApi.OData.OpenApiConvertSettings.ExpandNavigationPropertyAttributeName.set -> void
139139
Microsoft.OpenApi.OData.OpenApiConvertSettings.IEEE754Compatible.get -> bool
140140
Microsoft.OpenApi.OData.OpenApiConvertSettings.IEEE754Compatible.set -> void
141141
Microsoft.OpenApi.OData.OpenApiConvertSettings.OpenApiConvertSettings() -> void

‎src/Microsoft.OpenApi.OData.Reader/Vocabulary/Capabilities/ReadRestrictionsType.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ internal abstract class ReadRestrictionsBase : IRecord
4646
public string LongDescription { get; private set; }
4747

4848
/// <summary>
49-
/// Test the target supports update.
49+
/// Test the target supports read.
5050
/// </summary>
5151
/// <returns>True/false.</returns>
5252
public bool IsReadable => Readable == null || Readable.Value;

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void GetPathsForGraphBetaModelReturnsAllPaths()
4848

4949
// Assert
5050
Assert.NotNull(paths);
51-
Assert.Equal(26436, paths.Count());
51+
Assert.Equal(12261, paths.Count());
5252
}
5353

5454
[Fact]
@@ -67,7 +67,7 @@ public void GetPathsForGraphBetaModelWithDerivedTypesConstraintReturnsAllPaths()
6767

6868
// Assert
6969
Assert.NotNull(paths);
70-
Assert.Equal(26394, paths.Count());
70+
Assert.Equal(12219, paths.Count());
7171
}
7272

7373
[Fact]

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Generator/OpenApiPathItemGeneratorTests.cs‎

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,17 @@ public void CreatePathItemsReturnsForEmptyModel()
4141
Assert.Empty(pathItems);
4242
}
4343

44-
[Fact]
45-
public void CreatePathItemsReturnsForBasicModel()
44+
[Theory]
45+
[InlineData(true, 10)]
46+
[InlineData(false, 22)]
47+
public void CreatePathItemsReturnsForBasicModel(bool useAnnotationToGeneratePath, int pathCount)
4648
{
4749
// Arrange
4850
IEdmModel model = EdmModelHelper.BasicEdmModel;
4951
OpenApiConvertSettings settings = new OpenApiConvertSettings
5052
{
51-
EnableKeyAsSegment = true
53+
EnableKeyAsSegment = true,
54+
UseRestrictionAnnotationsToGeneratePathsForComplexProperties = useAnnotationToGeneratePath
5255
};
5356
ODataContext context = new ODataContext(model, settings);
5457

@@ -57,34 +60,49 @@ public void CreatePathItemsReturnsForBasicModel()
5760

5861
// Assert
5962
Assert.NotNull(pathItems);
60-
Assert.Equal(22, pathItems.Count);
61-
62-
Assert.Contains("/People", pathItems.Keys);
63-
Assert.Contains("/People/$count", pathItems.Keys);
64-
Assert.Contains("/People/{UserName}", pathItems.Keys);
65-
Assert.Contains("/People/{UserName}/Addresses", pathItems.Keys);
66-
Assert.Contains("/People/{UserName}/Addresses/$count", pathItems.Keys);
67-
Assert.Contains("/People/{UserName}/HomeAddress", pathItems.Keys);
68-
Assert.Contains("/People/{UserName}/HomeAddress/City", pathItems.Keys);
69-
Assert.Contains("/City", pathItems.Keys);
70-
Assert.Contains("/City/$count", pathItems.Keys);
71-
Assert.Contains("/City/{Name}", pathItems.Keys);
72-
Assert.Contains("/CountryOrRegion", pathItems.Keys);
73-
Assert.Contains("/CountryOrRegion/$count", pathItems.Keys);
74-
Assert.Contains("/CountryOrRegion/{Name}", pathItems.Keys);
75-
Assert.Contains("/Me", pathItems.Keys);
76-
Assert.Contains("/Me/Addresses", pathItems.Keys);
77-
Assert.Contains("/Me/Addresses/$count", pathItems.Keys);
78-
Assert.Contains("/Me/HomeAddress", pathItems.Keys);
79-
Assert.Contains("/Me/HomeAddress/City", pathItems.Keys);
80-
Assert.Contains("/Me/WorkAddress", pathItems.Keys);
81-
Assert.Contains("/Me/WorkAddress/City", pathItems.Keys);
63+
Assert.Equal(pathCount, pathItems.Count);
64+
65+
if (useAnnotationToGeneratePath)
66+
{
67+
Assert.Contains("/People", pathItems.Keys);
68+
Assert.Contains("/People/$count", pathItems.Keys);
69+
Assert.Contains("/People/{UserName}", pathItems.Keys);
70+
Assert.Contains("/City", pathItems.Keys);
71+
Assert.Contains("/City/$count", pathItems.Keys);
72+
Assert.Contains("/City/{Name}", pathItems.Keys);
73+
Assert.Contains("/CountryOrRegion", pathItems.Keys);
74+
Assert.Contains("/CountryOrRegion/$count", pathItems.Keys);
75+
Assert.Contains("/CountryOrRegion/{Name}", pathItems.Keys);
76+
Assert.Contains("/Me", pathItems.Keys);
77+
}
78+
else
79+
{
80+
Assert.Contains("/People", pathItems.Keys);
81+
Assert.Contains("/People/$count", pathItems.Keys);
82+
Assert.Contains("/People/{UserName}", pathItems.Keys);
83+
Assert.Contains("/People/{UserName}/Addresses", pathItems.Keys);
84+
Assert.Contains("/People/{UserName}/Addresses/$count", pathItems.Keys);
85+
Assert.Contains("/People/{UserName}/HomeAddress", pathItems.Keys);
86+
Assert.Contains("/People/{UserName}/HomeAddress/City", pathItems.Keys);
87+
Assert.Contains("/City", pathItems.Keys);
88+
Assert.Contains("/City/$count", pathItems.Keys);
89+
Assert.Contains("/City/{Name}", pathItems.Keys);
90+
Assert.Contains("/CountryOrRegion", pathItems.Keys);
91+
Assert.Contains("/CountryOrRegion/$count", pathItems.Keys);
92+
Assert.Contains("/CountryOrRegion/{Name}", pathItems.Keys);
93+
Assert.Contains("/Me", pathItems.Keys);
94+
Assert.Contains("/Me/Addresses", pathItems.Keys);
95+
Assert.Contains("/Me/Addresses/$count", pathItems.Keys);
96+
Assert.Contains("/Me/HomeAddress", pathItems.Keys);
97+
Assert.Contains("/Me/HomeAddress/City", pathItems.Keys);
98+
Assert.Contains("/Me/WorkAddress", pathItems.Keys);
99+
Assert.Contains("/Me/WorkAddress/City", pathItems.Keys);
100+
}
82101
}
83102

84103
[Theory]
85104
[InlineData(true, true, true, "/Customers({ID}):/{param}:")]
86105
[InlineData(true, true, false, "/Customers({ID}):/{param}")]
87-
88106
[InlineData(true, false, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]
89107
[InlineData(true, false, false, "/Customers({ID})/NS.MyFunction(param='{param}')")]
90108
[InlineData(false, true, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]

0 commit comments

Comments
 (0)