Skip to content

Commit 73c43d3

Browse files
irvinesundayirvinesunday
authored
Revert "Use CRUD restrictions annotations for the generation of complex properties' paths (#180)"
This reverts commit 2a7e87c.
1 parent 2a7e87c commit 73c43d3

26 files changed

+12011
-890
lines changed

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,6 @@ 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-
252250
currentPath.Push(new ODataComplexPropertySegment(sp));
253251
AppendPath(currentPath.Clone());
254252

@@ -282,27 +280,6 @@ private void RetrieveComplexPropertyPaths(IEdmEntityType entityType, ODataPath c
282280
}
283281
}
284282

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-
306283
/// <summary>
307284
/// Retrieves the paths for a media entity stream.
308285
/// </summary>

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,6 @@ 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-
233228
internal OpenApiConvertSettings Clone()
234229
{
235230
var newSettings = new OpenApiConvertSettings
@@ -267,8 +262,7 @@ internal OpenApiConvertSettings Clone()
267262
EnableDeprecationInformation = this.EnableDeprecationInformation,
268263
AddEnumDescriptionExtension = this.AddEnumDescriptionExtension,
269264
ErrorResponsesAsDefault = this.ErrorResponsesAsDefault,
270-
InnerErrorComplexTypeName = this.InnerErrorComplexTypeName,
271-
UseRestrictionAnnotationsToGeneratePathsForComplexProperties = this.UseRestrictionAnnotationsToGeneratePathsForComplexProperties
265+
InnerErrorComplexTypeName = this.InnerErrorComplexTypeName
272266
};
273267

274268
return newSettings;

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// ------------------------------------------------------------
2+
// Copyright (c) Microsoft Corporation. All rights reserved.
3+
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
4+
// ------------------------------------------------------------
5+
6+
using System.Linq;
7+
using Microsoft.OData.Edm;
8+
using Microsoft.OpenApi.Models;
9+
using Microsoft.OpenApi.OData.Common;
10+
using Microsoft.OpenApi.OData.Edm;
11+
using Microsoft.OpenApi.OData.Generator;
12+
using Microsoft.OpenApi.OData.Vocabulary.Capabilities;
13+
14+
namespace Microsoft.OpenApi.OData.Operation;
15+
16+
internal class ComplexPropertyDeleteOperationHandler : ComplexPropertyBaseOperationHandler
17+
{
18+
/// <inheritdoc />
19+
public override OperationType OperationType => OperationType.Delete;
20+
21+
/// <inheritdoc/>
22+
protected override void SetBasicInfo(OpenApiOperation operation)
23+
{
24+
// Summary
25+
operation.Summary = $"Delete {ComplexPropertySegment.Property.Name} property value";
26+
27+
// Description
28+
operation.Description = Context.Model.GetDescriptionAnnotation(ComplexPropertySegment.Property);
29+
30+
// OperationId
31+
if (Context.Settings.EnableOperationId)
32+
{
33+
string typeName = ComplexPropertySegment.ComplexType.Name;
34+
operation.OperationId = ComplexPropertySegment.Property.Name + "." + typeName + ".Delete" + Utils.UpperFirstChar(typeName);
35+
}
36+
}
37+
38+
/// <inheritdoc/>
39+
protected override void SetParameters(OpenApiOperation operation)
40+
{
41+
base.SetParameters(operation);
42+
43+
operation.Parameters.Add(new OpenApiParameter
44+
{
45+
Name = "If-Match",
46+
In = ParameterLocation.Header,
47+
Description = "ETag",
48+
Schema = new OpenApiSchema
49+
{
50+
Type = "string"
51+
}
52+
});
53+
}
54+
55+
/// <inheritdoc/>
56+
protected override void SetResponses(OpenApiOperation operation)
57+
{
58+
operation.AddErrorResponses(Context.Settings, true);
59+
base.SetResponses(operation);
60+
}
61+
protected override void SetSecurity(OpenApiOperation operation)
62+
{
63+
DeleteRestrictionsType delete = Context.Model.GetRecord<DeleteRestrictionsType>(ComplexPropertySegment.Property, CapabilitiesConstants.DeleteRestrictions);
64+
if (delete == null || delete.Permissions == null)
65+
{
66+
return;
67+
}
68+
69+
operation.Security = Context.CreateSecurityRequirements(delete.Permissions).ToList();
70+
}
71+
72+
protected override void AppendCustomParameters(OpenApiOperation operation)
73+
{
74+
DeleteRestrictionsType delete = Context.Model.GetRecord<DeleteRestrictionsType>(ComplexPropertySegment.Property, CapabilitiesConstants.DeleteRestrictions);
75+
if (delete == null)
76+
{
77+
return;
78+
}
79+
80+
if (delete.CustomHeaders != null)
81+
{
82+
AppendCustomParameters(operation, delete.CustomHeaders, ParameterLocation.Header);
83+
}
84+
85+
if (delete.CustomQueryOptions != null)
86+
{
87+
AppendCustomParameters(operation, delete.CustomQueryOptions, ParameterLocation.Query);
88+
}
89+
}
90+
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ 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-
}}
107+
{OperationType.Post, new ComplexPropertyPostOperationHandler() },
108+
{OperationType.Delete, new ComplexPropertyDeleteOperationHandler() },
109+
}},
109110
};
110111

111112
/// <inheritdoc/>

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

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,37 +23,25 @@ internal class ComplexPropertyItemHandler : PathItemHandler
2323
/// <inheritdoc/>
2424
protected override void SetOperations(OpenApiPathItem item)
2525
{
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-
}
26+
AddOperation(item, OperationType.Get);
3227

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)
28+
UpdateRestrictionsType update = Context.Model.GetRecord<UpdateRestrictionsType>(ComplexProperty);
29+
if (update != null && update.IsUpdateMethodPut)
3730
{
38-
if (update != null && update.IsUpdateMethodPut)
39-
{
40-
AddOperation(item, OperationType.Put);
41-
}
42-
else
43-
{
44-
AddOperation(item, OperationType.Patch);
45-
}
31+
AddOperation(item, OperationType.Put);
32+
}
33+
else
34+
{
35+
AddOperation(item, OperationType.Patch);
4636
}
4737

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-
{
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())
5443
AddOperation(item, OperationType.Post);
55-
}
56-
}
44+
}
5745
}
5846

5947
/// <inheritdoc/>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ 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
108
static Microsoft.OpenApi.OData.Edm.EdmTypeExtensions.ShouldPathParameterBeQuoted(this Microsoft.OData.Edm.IEdmType edmType, Microsoft.OpenApi.OData.OpenApiConvertSettings settings) -> bool
119
Microsoft.OpenApi.OData.Edm.IODataPathProvider
1210
Microsoft.OpenApi.OData.Edm.IODataPathProvider.CanFilter(Microsoft.OData.Edm.IEdmElement element) -> bool
@@ -136,6 +134,8 @@ Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUnqualifiedCall.get -> bool
136134
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUnqualifiedCall.set -> void
137135
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableUriEscapeFunctionCall.get -> bool
138136
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 read.
49+
/// Test the target supports update.
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(12261, paths.Count());
51+
Assert.Equal(26436, paths.Count());
5252
}
5353

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

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

7373
[Fact]

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

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

44-
[Theory]
45-
[InlineData(true, 10)]
46-
[InlineData(false, 22)]
47-
public void CreatePathItemsReturnsForBasicModel(bool useAnnotationToGeneratePath, int pathCount)
44+
[Fact]
45+
public void CreatePathItemsReturnsForBasicModel()
4846
{
4947
// Arrange
5048
IEdmModel model = EdmModelHelper.BasicEdmModel;
5149
OpenApiConvertSettings settings = new OpenApiConvertSettings
5250
{
53-
EnableKeyAsSegment = true,
54-
UseRestrictionAnnotationsToGeneratePathsForComplexProperties = useAnnotationToGeneratePath
51+
EnableKeyAsSegment = true
5552
};
5653
ODataContext context = new ODataContext(model, settings);
5754

@@ -60,49 +57,34 @@ public void CreatePathItemsReturnsForBasicModel(bool useAnnotationToGeneratePath
6057

6158
// Assert
6259
Assert.NotNull(pathItems);
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-
}
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);
10182
}
10283

10384
[Theory]
10485
[InlineData(true, true, true, "/Customers({ID}):/{param}:")]
10586
[InlineData(true, true, false, "/Customers({ID}):/{param}")]
87+
10688
[InlineData(true, false, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]
10789
[InlineData(true, false, false, "/Customers({ID})/NS.MyFunction(param='{param}')")]
10890
[InlineData(false, true, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]

0 commit comments

Comments
 (0)