Skip to content

Commit feae089

Browse files
irvinesundayirvinesunday
authored
[Fix] Returns response status code 2XX for PUT operations of stream properties when UseSuccessStatusCodeRange is enabled (#357)
* Add content identifier as a constant * Refactor method * Add default responses for status codes * Use tuple to retrieve values; rename method and references * Add schema ref to the content property of the response object of 'content' stream properties * Update unit tests * Update release notes * Use constants * Use constant for content
1 parent 9fdbd6e commit feae089

File tree

10 files changed

+130
-55
lines changed

10 files changed

+130
-55
lines changed

‎src/Microsoft.OpenApi.OData.Reader/Common/Constants.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,5 +194,25 @@ internal static class Constants
194194
/// count segment identifier
195195
/// </summary>
196196
public const string CountSegmentIdentifier = "count";
197+
198+
/// <summary>
199+
/// content string
200+
/// </summary>
201+
public const string Content = "content";
202+
203+
/// <summary>
204+
/// Success string
205+
/// </summary>
206+
public const string Success = "Success";
207+
208+
/// <summary>
209+
/// Created string
210+
/// </summary>
211+
public const string Created = "Created";
212+
213+
/// <summary>
214+
/// error string
215+
/// </summary>
216+
public const string Error = "error";
197217
}
198218
}

‎src/Microsoft.OpenApi.OData.Reader/Common/OpenApiOperationExtensions.cs‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,48 +20,49 @@ public static class OpenApiOperationExtensions
2020
/// </summary>
2121
/// <param name="operation">The operation.</param>
2222
/// <param name="settings">The settings.</param>
23-
/// <param name="addNoContent">Whether to add a 204 no content response.</param>
23+
/// <param name="addNoContent">Optional: Whether to add a 204 no content response.</param>
2424
/// <param name="schema">Optional: The OpenAPI schema of the response.</param>
2525
public static void AddErrorResponses(this OpenApiOperation operation, OpenApiConvertSettings settings, bool addNoContent = false, OpenApiSchema schema = null)
2626
{
27-
if (operation == null) {
28-
throw Error.ArgumentNull(nameof(operation));
29-
}
30-
if(settings == null) {
31-
throw Error.ArgumentNull(nameof(settings));
32-
}
33-
34-
if(operation.Responses == null)
27+
Utils.CheckArgumentNull(operation, nameof(operation));
28+
Utils.CheckArgumentNull(settings, nameof(settings));
29+
30+
if (operation.Responses == null)
3531
{
3632
operation.Responses = new();
3733
}
3834

3935
if (addNoContent)
40-
{
41-
if (settings.UseSuccessStatusCodeRange && schema != null)
36+
{
37+
if (settings.UseSuccessStatusCodeRange)
4238
{
43-
OpenApiResponse response = new()
39+
OpenApiResponse response = null;
40+
if (schema != null)
4441
{
45-
Content = new Dictionary<string, OpenApiMediaType>
42+
response = new()
4643
{
44+
Description = Constants.Success,
45+
Content = new Dictionary<string, OpenApiMediaType>
4746
{
48-
Constants.ApplicationJsonMediaType,
49-
new OpenApiMediaType
5047
{
51-
Schema = schema
48+
Constants.ApplicationJsonMediaType,
49+
new OpenApiMediaType
50+
{
51+
Schema = schema
52+
}
5253
}
5354
}
54-
}
55-
};
56-
operation.Responses.Add(Constants.StatusCodeClass2XX, response);
55+
};
56+
}
57+
operation.Responses.Add(Constants.StatusCodeClass2XX, response ?? Constants.StatusCodeClass2XX.GetResponse());
5758
}
5859
else
5960
{
6061
operation.Responses.Add(Constants.StatusCode204, Constants.StatusCode204.GetResponse());
6162
}
62-
}
63+
}
6364

64-
if(settings.ErrorResponsesAsDefault)
65+
if (settings.ErrorResponsesAsDefault)
6566
{
6667
operation.Responses.Add(Constants.StatusCodeDefault, Constants.StatusCodeDefault.GetResponse());
6768
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ private void RetrieveMediaEntityStreamPaths(IEdmEntityType entityType, ODataPath
385385
currentPath.Pop();
386386
}
387387

388-
if (sp.Name.Equals("content", StringComparison.OrdinalIgnoreCase))
388+
if (sp.Name.Equals(Constants.Content, StringComparison.OrdinalIgnoreCase))
389389
{
390390
createValuePath = false;
391391
}

‎src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiResponseGenerator.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,21 @@ internal static class OpenApiResponseGenerator
2727
Reference = new OpenApiReference
2828
{
2929
Type = ReferenceType.Response,
30-
Id = "error"
30+
Id = Constants.Error
3131
}
3232
}
3333
},
3434

35-
{ Constants.StatusCode204, new OpenApiResponse { Description = "Success"} },
35+
{ Constants.StatusCode204, new OpenApiResponse { Description = Constants.Success} },
36+
{ Constants.StatusCode201, new OpenApiResponse { Description = Constants.Created} },
37+
{ Constants.StatusCodeClass2XX, new OpenApiResponse { Description = Constants.Success} },
3638
{ Constants.StatusCodeClass4XX, new OpenApiResponse
3739
{
3840
UnresolvedReference = true,
3941
Reference = new OpenApiReference
4042
{
4143
Type = ReferenceType.Response,
42-
Id = "error"
44+
Id = Constants.Error
4345
}
4446
}
4547
},
@@ -49,7 +51,7 @@ internal static class OpenApiResponseGenerator
4951
Reference = new OpenApiReference
5052
{
5153
Type = ReferenceType.Response,
52-
Id = "error"
54+
Id = Constants.Error
5355
}
5456
}
5557
}

‎src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
<TargetFrameworks>netstandard2.0</TargetFrameworks>
1616
<PackageId>Microsoft.OpenApi.OData</PackageId>
1717
<SignAssembly>true</SignAssembly>
18-
<Version>1.3.0-preview2</Version>
18+
<Version>1.3.0-preview3</Version>
1919
<Description>This package contains the codes you need to convert OData CSDL to Open API Document of Model.</Description>
2020
<Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
2121
<PackageTags>Microsoft OpenApi OData EDM</PackageTags>
@@ -25,6 +25,7 @@
2525
- Skips adding a $count path if a similar count() function path exists #347
2626
- Checks whether path exists before adding it to the paths dictionary #343
2727
- Strips namespace prefix from operation segments and aliases type cast segments #348
28+
- Return response status code 2XX for PUT operations of stream properties when UseSuccessStatusCodeRange is enabled #310
2829
</PackageReleaseNotes>
2930
<AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
3031
<AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,23 +34,23 @@ protected override void SetBasicInfo(OpenApiOperation operation)
3434
// Description
3535
if (LastSegmentIsStreamPropertySegment)
3636
{
37-
IEdmVocabularyAnnotatable annotatable = GetAnnotatableElement();
37+
(_, var property) = GetStreamElements();
3838
string description;
3939

40-
if (annotatable is IEdmNavigationProperty)
40+
if (property is IEdmNavigationProperty)
4141
{
42-
ReadRestrictionsType readRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(annotatable, CapabilitiesConstants.NavigationRestrictions)?
42+
ReadRestrictionsType readRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(property, CapabilitiesConstants.NavigationRestrictions)?
4343
.RestrictedProperties?.FirstOrDefault()?.ReadRestrictions;
4444

4545
description = LastSegmentIsKeySegment
4646
? readRestriction?.ReadByKeyRestrictions?.Description
4747
: readRestriction?.Description
48-
?? Context.Model.GetDescriptionAnnotation(annotatable);
48+
?? Context.Model.GetDescriptionAnnotation(property);
4949
}
5050
else
5151
{
5252
// Structural property
53-
description = Context.Model.GetDescriptionAnnotation(annotatable);
53+
description = Context.Model.GetDescriptionAnnotation(property);
5454
}
5555

5656
operation.Description = description;

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,11 @@ protected IDictionary<string, OpenApiMediaType> GetContentDescription()
145145
};
146146

147147
// Fetch the respective AcceptableMediaTypes
148-
IEdmVocabularyAnnotatable annotatableElement = GetAnnotatableElement();
148+
(_, var property) = GetStreamElements();
149149
IEnumerable<string> mediaTypes = null;
150-
if (annotatableElement != null)
150+
if (property != null)
151151
{
152-
mediaTypes = Context.Model.GetCollection(annotatableElement,
152+
mediaTypes = Context.Model.GetCollection(property,
153153
CoreConstants.AcceptableMediaTypes);
154154
}
155155

@@ -173,13 +173,13 @@ protected IDictionary<string, OpenApiMediaType> GetContentDescription()
173173
}
174174

175175
/// <summary>
176-
/// Gets the annotatable stream property from the path segments.
176+
/// Gets the stream property and entity type declaring the stream property.
177177
/// </summary>
178-
/// <returns>The annotatable stream property.</returns>
179-
protected IEdmVocabularyAnnotatable GetAnnotatableElement()
178+
/// <returns>The stream property and entity type declaring the stream property.</returns>
179+
protected (IEdmEntityType entityType, IEdmProperty property) GetStreamElements()
180180
{
181181
// Only ODataStreamPropertySegment is annotatable
182-
if (!LastSegmentIsStreamPropertySegment) return null;
182+
if (!LastSegmentIsStreamPropertySegment) return (null, null);
183183

184184
// Retrieve the entity type of the segment before the stream property segment
185185
var entityType = Path.Segments.ElementAtOrDefault(Path.Segments.Count - 2).EntityType;
@@ -192,7 +192,7 @@ protected IEdmVocabularyAnnotatable GetAnnotatableElement()
192192
property = GetNavigationProperty(entityType, lastSegmentProp.Identifier);
193193
}
194194

195-
return property;
195+
return (entityType, property);
196196
}
197197

198198
private IEdmStructuralProperty GetStructuralProperty(IEdmEntityType entityType, string identifier)

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

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
44
// ------------------------------------------------------------
55

6+
using System;
67
using System.Linq;
78
using Microsoft.OData.Edm;
89
using Microsoft.OData.Edm.Vocabularies;
@@ -34,20 +35,20 @@ protected override void SetBasicInfo(OpenApiOperation operation)
3435
// Description
3536
if (LastSegmentIsStreamPropertySegment)
3637
{
37-
IEdmVocabularyAnnotatable annotatable = GetAnnotatableElement();
38+
(_, var property) = GetStreamElements();
3839
string description;
3940

40-
if (annotatable is IEdmNavigationProperty)
41+
if (property is IEdmNavigationProperty)
4142
{
42-
UpdateRestrictionsType updateRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(annotatable, CapabilitiesConstants.NavigationRestrictions)?
43+
UpdateRestrictionsType updateRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(property, CapabilitiesConstants.NavigationRestrictions)?
4344
.RestrictedProperties?.FirstOrDefault()?.UpdateRestrictions;
4445

45-
description = updateRestriction?.Description ?? Context.Model.GetDescriptionAnnotation(annotatable);
46+
description = updateRestriction?.Description ?? Context.Model.GetDescriptionAnnotation(property);
4647
}
4748
else
4849
{
4950
// Structural property
50-
description = Context.Model.GetDescriptionAnnotation(annotatable);
51+
description = Context.Model.GetDescriptionAnnotation(property);
5152
}
5253

5354
operation.Description = description;
@@ -77,7 +78,28 @@ protected override void SetRequestBody(OpenApiOperation operation)
7778
/// <inheritdoc/>
7879
protected override void SetResponses(OpenApiOperation operation)
7980
{
80-
operation.AddErrorResponses(Context.Settings, true);
81+
if (LastSegmentIsStreamPropertySegment && Path.LastSegment.Identifier.Equals(Constants.Content, StringComparison.OrdinalIgnoreCase))
82+
{
83+
// Get the entity type declaring this stream property.
84+
(var entityType, _) = GetStreamElements();
85+
86+
OpenApiSchema schema = new()
87+
{
88+
UnresolvedReference = true,
89+
Reference = new OpenApiReference
90+
{
91+
Type = ReferenceType.Schema,
92+
Id = entityType.FullName()
93+
}
94+
};
95+
96+
operation.AddErrorResponses(Context.Settings, addNoContent: true, schema: schema);
97+
}
98+
else
99+
{
100+
operation.AddErrorResponses(Context.Settings, true);
101+
}
102+
81103
base.SetResponses(operation);
82104
}
83105

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityGetOperationHandlerTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ public static IEdmModel GetEdmModel(string annotation)
135135
</Key>
136136
<Property Name=""Id"" Type=""Edm.Int32"" Nullable=""false"" />
137137
<Property Name=""Logo"" Type=""Edm.Stream""/>
138+
<Property Name=""Content"" Type=""Edm.Stream""/>
138139
<Property Name = ""Description"" Type = ""Edm.String"" />
139140
</EntityType>
140141
<EntityType Name=""user"" OpenType=""true"">

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityPutOperationHandlerTests.cs‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ public class MediaEntityPutOperationHandlerTests
1717
private readonly MediaEntityPutOperationHandler _operationalHandler = new MediaEntityPutOperationHandler();
1818

1919
[Theory]
20-
[InlineData(true)]
21-
[InlineData(false)]
22-
public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOperationId)
20+
[InlineData(true, false)]
21+
[InlineData(true, true)]
22+
[InlineData(false, false)]
23+
[InlineData(false, true)]
24+
public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOperationId, bool useSuccessStatusCodeRange)
2325
{
2426
// Arrange
2527
string qualifiedName = CoreConstants.AcceptableMediaTypes;
@@ -33,17 +35,18 @@ public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOper
3335
<Annotation Term=""Org.OData.Core.V1.Description"" String=""The logo image."" />";
3436

3537
// Assert
36-
VerifyMediaEntityPutOperation("", enableOperationId);
37-
VerifyMediaEntityPutOperation(annotation, enableOperationId);
38+
VerifyMediaEntityPutOperation("", enableOperationId, useSuccessStatusCodeRange);
39+
VerifyMediaEntityPutOperation(annotation, enableOperationId, useSuccessStatusCodeRange);
3840
}
3941

40-
private void VerifyMediaEntityPutOperation(string annotation, bool enableOperationId)
42+
private void VerifyMediaEntityPutOperation(string annotation, bool enableOperationId, bool useSuccessStatusCodeRange)
4143
{
4244
// Arrange
4345
IEdmModel model = MediaEntityGetOperationHandlerTests.GetEdmModel(annotation);
4446
OpenApiConvertSettings settings = new OpenApiConvertSettings
4547
{
46-
EnableOperationId = enableOperationId
48+
EnableOperationId = enableOperationId,
49+
UseSuccessStatusCodeRange = useSuccessStatusCodeRange
4750
};
4851

4952
ODataContext context = new ODataContext(model, settings);
@@ -63,13 +66,20 @@ private void VerifyMediaEntityPutOperation(string annotation, bool enableOperati
6366
new ODataNavigationPropertySegment(navProperty),
6467
new ODataStreamContentSegment());
6568

69+
IEdmStructuralProperty sp2 = todo.StructuralProperties().First(c => c.Name == "Content");
70+
ODataPath path3 = new(new ODataNavigationSourceSegment(todos),
71+
new ODataKeySegment(todos.EntityType()),
72+
new ODataStreamPropertySegment(sp2.Name));
73+
6674
// Act
6775
var putOperation = _operationalHandler.CreateOperation(context, path);
6876
var putOperation2 = _operationalHandler.CreateOperation(context, path2);
77+
var putOperation3 = _operationalHandler.CreateOperation(context, path3);
6978

7079
// Assert
7180
Assert.NotNull(putOperation);
7281
Assert.NotNull(putOperation2);
82+
Assert.NotNull(putOperation3);
7383
Assert.Equal("Update Logo for Todo in Todos", putOperation.Summary);
7484
Assert.Equal("Update media content for the navigation property photo in me", putOperation2.Summary);
7585
Assert.NotNull(putOperation.Tags);
@@ -82,10 +92,28 @@ private void VerifyMediaEntityPutOperation(string annotation, bool enableOperati
8292

8393
Assert.NotNull(putOperation.Responses);
8494
Assert.NotNull(putOperation2.Responses);
95+
Assert.NotNull(putOperation3.Responses);
96+
8597
Assert.Equal(2, putOperation.Responses.Count);
8698
Assert.Equal(2, putOperation2.Responses.Count);
87-
Assert.Equal(new[] { "204", "default" }, putOperation.Responses.Select(r => r.Key));
88-
Assert.Equal(new[] { "204", "default" }, putOperation2.Responses.Select(r => r.Key));
99+
Assert.Equal(2, putOperation3.Responses.Count);
100+
101+
var statusCode = (useSuccessStatusCodeRange) ? Constants.StatusCodeClass2XX : Constants.StatusCode204;
102+
Assert.Equal(new[] { statusCode, "default" }, putOperation.Responses.Select(r => r.Key));
103+
Assert.Equal(new[] { statusCode, "default" }, putOperation2.Responses.Select(r => r.Key));
104+
Assert.Equal(new[] { statusCode, "default" }, putOperation3.Responses.Select(r => r.Key));
105+
106+
// Test only for stream properties of identifier 'content'
107+
if (useSuccessStatusCodeRange)
108+
{
109+
var referenceId = putOperation3.Responses[statusCode]?.Content[Constants.ApplicationJsonMediaType]?.Schema?.Reference.Id;
110+
Assert.NotNull(referenceId);
111+
Assert.Equal("microsoft.graph.Todo", referenceId);
112+
}
113+
else
114+
{
115+
Assert.Empty(putOperation3.Responses[statusCode].Content);
116+
}
89117

90118
if (!string.IsNullOrEmpty(annotation))
91119
{

0 commit comments

Comments
 (0)