Skip to content

Commit 226ce62

Browse files
irvinesundayirvinesunday
authored
Fixes response objects for operations that return single primitive type values (#127)
* Refactor similar code into one function; add fix for responses for primitive types * Add new param to method and add tests for this * Update test files * Address PR review suggestion
1 parent 03b6000 commit 226ce62

File tree

8 files changed

+160
-91
lines changed

8 files changed

+160
-91
lines changed

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

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// ------------------------------------------------------------
55

66
using System.Collections.Generic;
7+
using System.Linq;
78
using Microsoft.OData.Edm;
89
using Microsoft.OpenApi.Models;
910
using Microsoft.OpenApi.OData.Common;
@@ -72,35 +73,76 @@ public static IDictionary<string, OpenApiResponse> CreateResponses(this ODataCon
7273
/// </summary>
7374
/// <param name="context">The OData context.</param>
7475
/// <param name="operationImport">The Edm operation import.</param>
76+
/// <param name="path">The OData path.</param>
7577
/// <returns>The created <see cref="OpenApiResponses"/>.</returns>
76-
public static OpenApiResponses CreateResponses(this ODataContext context, IEdmOperationImport operationImport)
78+
public static OpenApiResponses CreateResponses(this ODataContext context, IEdmOperationImport operationImport, ODataPath path)
7779
{
7880
Utils.CheckArgumentNull(context, nameof(context));
7981
Utils.CheckArgumentNull(operationImport, nameof(operationImport));
82+
Utils.CheckArgumentNull(path, nameof(path));
8083

81-
return context.CreateResponses(operationImport.Operation);
84+
return context.CreateResponses(operationImport.Operation, path);
8285
}
8386

8487
/// <summary>
8588
/// Create the <see cref="OpenApiResponses"/> for a <see cref="IEdmOperation"/>
8689
/// </summary>
8790
/// <param name="context">The OData context.</param>
8891
/// <param name="operation">The Edm operation.</param>
92+
/// <param name="path">The OData path.</param>
8993
/// <returns>The created <see cref="OpenApiResponses"/>.</returns>
90-
public static OpenApiResponses CreateResponses(this ODataContext context, IEdmOperation operation)
94+
public static OpenApiResponses CreateResponses(this ODataContext context, IEdmOperation operation, ODataPath path)
9195
{
9296
Utils.CheckArgumentNull(context, nameof(context));
9397
Utils.CheckArgumentNull(operation, nameof(operation));
98+
Utils.CheckArgumentNull(path, nameof(path));
9499

95-
OpenApiResponses responses = new OpenApiResponses();
100+
OpenApiResponses responses = new();
96101

97102
if (operation.IsAction() && operation.ReturnType == null)
98103
{
99104
responses.Add(Constants.StatusCode204, Constants.StatusCode204.GetResponse());
100105
}
101106
else
102107
{
103-
OpenApiResponse response = new OpenApiResponse
108+
OpenApiSchema schema;
109+
if (operation.ReturnType.IsCollection())
110+
{
111+
// Get the entity type of the previous segment
112+
IEdmEntityType entityType = path.Segments.Reverse().Skip(1)?.Take(1)?.FirstOrDefault()?.EntityType;
113+
schema = new OpenApiSchema
114+
{
115+
Title = entityType == null ? null : $"Collection of {entityType.Name}",
116+
Type = "object",
117+
Properties = new Dictionary<string, OpenApiSchema>
118+
{
119+
{
120+
"value", context.CreateEdmTypeSchema(operation.ReturnType)
121+
}
122+
}
123+
};
124+
}
125+
else if (operation.ReturnType.IsPrimitive())
126+
{
127+
// A property or operation response that is of a primitive type is represented as an object with a single name/value pair,
128+
// whose name is value and whose value is a primitive value.
129+
schema = new OpenApiSchema
130+
{
131+
Type = "object",
132+
Properties = new Dictionary<string, OpenApiSchema>
133+
{
134+
{
135+
"value", context.CreateEdmTypeSchema(operation.ReturnType)
136+
}
137+
}
138+
};
139+
}
140+
else
141+
{
142+
schema = context.CreateEdmTypeSchema(operation.ReturnType);
143+
}
144+
145+
OpenApiResponse response = new()
104146
{
105147
Description = "Success",
106148
Content = new Dictionary<string, OpenApiMediaType>
@@ -109,15 +151,15 @@ public static OpenApiResponses CreateResponses(this ODataContext context, IEdmOp
109151
Constants.ApplicationJsonMediaType,
110152
new OpenApiMediaType
111153
{
112-
Schema = context.CreateEdmTypeSchema(operation.ReturnType)
154+
Schema = schema
113155
}
114156
}
115157
}
116158
};
117159
responses.Add(Constants.StatusCode200, response);
118160
}
119161

120-
// both action & function has the default response.
162+
// Both action & function have the default response.
121163
responses.Add(Constants.StatusCodeDefault, Constants.StatusCodeDefault.GetResponse());
122164

123165
return responses;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ protected override void SetResponses(OpenApiOperation operation)
7979
// describing the structure of the success response by referencing an appropriate schema
8080
// in the global schemas. In addition, it contains a default name/value pair for
8181
// the OData error response referencing the global responses.
82-
operation.Responses = Context.CreateResponses(EdmOperationImport);
82+
operation.Responses = Context.CreateResponses(EdmOperationImport, Path);
8383

8484
base.SetResponses(operation);
8585
}

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

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -166,56 +166,7 @@ protected override void SetParameters(OpenApiOperation operation)
166166
/// <inheritdoc/>
167167
protected override void SetResponses(OpenApiOperation operation)
168168
{
169-
if (EdmOperation.IsAction() && EdmOperation.ReturnType == null)
170-
{
171-
operation.Responses.Add(Constants.StatusCode204, Constants.StatusCode204.GetResponse());
172-
}
173-
else
174-
{
175-
OpenApiSchema schema;
176-
if (EdmOperation.ReturnType.TypeKind() == EdmTypeKind.Collection)
177-
{
178-
// Get the entity type of the previous segment
179-
IEdmEntityType entityType = Path.Segments.Reverse().Skip(1).Take(1).FirstOrDefault().EntityType;
180-
schema = new OpenApiSchema
181-
{
182-
Title = $"Collection of {entityType.Name}",
183-
Type = "object",
184-
Properties = new Dictionary<string, OpenApiSchema>
185-
{
186-
{
187-
"value", Context.CreateEdmTypeSchema(EdmOperation.ReturnType)
188-
}
189-
}
190-
};
191-
}
192-
else
193-
{
194-
schema = Context.CreateEdmTypeSchema(EdmOperation.ReturnType);
195-
}
196-
197-
// function should have a return type.
198-
OpenApiResponse response = new OpenApiResponse
199-
{
200-
Description = "Success",
201-
Content = new Dictionary<string, OpenApiMediaType>
202-
{
203-
{
204-
Constants.ApplicationJsonMediaType,
205-
new OpenApiMediaType
206-
{
207-
Schema = schema
208-
}
209-
}
210-
}
211-
};
212-
213-
operation.Responses.Add(Constants.StatusCode200, response);
214-
}
215-
216-
// both action & function has the default response.
217-
operation.Responses.Add(Constants.StatusCodeDefault, Constants.StatusCodeDefault.GetResponse());
218-
169+
operation.Responses = Context.CreateResponses(EdmOperation, Path);
219170
base.SetResponses(operation);
220171
}
221172

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

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public void CreateResponseForoperationImportThrowArgumentNullContext()
104104
ODataContext context = null;
105105

106106
// Act & Assert
107-
Assert.Throws<ArgumentNullException>("context", () => context.CreateResponses(operationImport: null));
107+
Assert.Throws<ArgumentNullException>("context", () => context.CreateResponses(operationImport: null, path: null));
108108
}
109109

110110
[Fact]
@@ -114,7 +114,19 @@ public void CreateResponseForoperationImportThrowArgumentNullOperationImport()
114114
ODataContext context = new ODataContext(EdmCoreModel.Instance);
115115

116116
// Act & Assert
117-
Assert.Throws<ArgumentNullException>("operationImport", () => context.CreateResponses(operationImport: null));
117+
Assert.Throws<ArgumentNullException>("operationImport", () => context.CreateResponses(operationImport: null, path: null));
118+
}
119+
120+
[Fact]
121+
public void CreateResponseForoperationImportThrowArgumentNullPath()
122+
{
123+
// Arrange
124+
ODataContext context = new ODataContext(EdmCoreModel.Instance);
125+
EdmFunction function = new EdmFunction("NS", "MyFunction", EdmCoreModel.Instance.GetString(false));
126+
EdmFunctionImport functionImport = new EdmFunctionImport(new EdmEntityContainer("NS", "Default"), "MyFunctionImport", function);
127+
128+
// Act & Assert
129+
Assert.Throws<ArgumentNullException>("path", () => context.CreateResponses(operationImport: functionImport, path: null));
118130
}
119131

120132
[Fact]
@@ -124,7 +136,7 @@ public void CreateResponseForOperationThrowArgumentNullContext()
124136
ODataContext context = null;
125137

126138
// Act & Assert
127-
Assert.Throws<ArgumentNullException>("context", () => context.CreateResponses(operation: null));
139+
Assert.Throws<ArgumentNullException>("context", () => context.CreateResponses(operation: null, path: null));
128140
}
129141

130142
[Fact]
@@ -134,7 +146,19 @@ public void CreateResponseForOperationThrowArgumentNullOperation()
134146
ODataContext context = new ODataContext(EdmCoreModel.Instance);
135147

136148
// Act & Assert
137-
Assert.Throws<ArgumentNullException>("operation", () => context.CreateResponses(operation: null));
149+
Assert.Throws<ArgumentNullException>("operation", () => context.CreateResponses(operation: null, path: null));
150+
}
151+
152+
[Fact]
153+
public void CreateResponseForOperationThrowArgumentNullPath()
154+
{
155+
// Arrange
156+
ODataContext context = new ODataContext(EdmCoreModel.Instance);
157+
EdmFunction function = new EdmFunction("NS", "MyFunction", EdmCoreModel.Instance.GetString(false));
158+
EdmFunctionImport functionImport = new EdmFunctionImport(new EdmEntityContainer("NS", "Default"), "MyFunctionImport", function);
159+
160+
// Act & Assert
161+
Assert.Throws<ArgumentNullException>("path", () => context.CreateResponses(operation: function, path: null));
138162
}
139163

140164
[Theory]
@@ -157,13 +181,15 @@ public void CreateResponseForEdmFunctionReturnCorrectResponses(bool isFunctionIm
157181
{
158182
IEdmOperationImport operationImport = model.EntityContainer.OperationImports().First(o => o.Name == operationName);
159183
Assert.NotNull(operationImport); // guard
160-
responses = context.CreateResponses(operationImport);
184+
ODataPath path = new ODataPath(new ODataOperationImportSegment(operationImport));
185+
responses = context.CreateResponses(operationImport, path);
161186
}
162187
else
163188
{
164189
IEdmOperation operation = model.SchemaElements.OfType<IEdmOperation>().First(o => o.Name == operationName);
165190
Assert.NotNull(operation); // guard
166-
responses = context.CreateResponses(operation);
191+
ODataPath path = new ODataPath(new ODataOperationSegment(operation));
192+
responses = context.CreateResponses(operation, path);
167193
}
168194

169195
// Assert
@@ -195,7 +221,7 @@ public void CreateResponseForEdmFunctionReturnCorrectResponses(bool isFunctionIm
195221
Assert.NotNull(mediaType.Schema.AnyOf);
196222
var anyOf = Assert.Single(mediaType.Schema.AnyOf);
197223
Assert.Equal("Microsoft.OData.Service.Sample.TrippinInMemory.Models.Person", anyOf.Reference.Id);
198-
}
224+
}
199225
}
200226

201227
[Theory]
@@ -214,13 +240,15 @@ public void CreateResponseForEdmActionReturnCorrectResponses(string actionName,
214240
{
215241
IEdmOperationImport operationImport = model.EntityContainer.OperationImports().First(o => o.Name == actionName);
216242
Assert.NotNull(operationImport); // guard
217-
responses = context.CreateResponses(operationImport);
243+
ODataPath path = new ODataPath(new ODataOperationImportSegment(operationImport));
244+
responses = context.CreateResponses(operationImport, path);
218245
}
219246
else
220247
{
221248
IEdmOperation operation = model.SchemaElements.OfType<IEdmOperation>().First(o => o.Name == actionName);
222249
Assert.NotNull(operation); // guard
223-
responses = context.CreateResponses(operation);
250+
ODataPath path = new ODataPath(new ODataOperationSegment(operation));
251+
responses = context.CreateResponses(operation, path);
224252
}
225253

226254
// Assert

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/TripService.OpenApi.V2.json

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,8 +1313,13 @@
13131313
"200": {
13141314
"description": "Success",
13151315
"schema": {
1316-
"default": false,
1317-
"type": "boolean"
1316+
"type": "object",
1317+
"properties": {
1318+
"value": {
1319+
"default": false,
1320+
"type": "boolean"
1321+
}
1322+
}
13181323
}
13191324
},
13201325
"default": {
@@ -3000,8 +3005,13 @@
30003005
"200": {
30013006
"description": "Success",
30023007
"schema": {
3003-
"default": false,
3004-
"type": "boolean"
3008+
"type": "object",
3009+
"properties": {
3010+
"value": {
3011+
"default": false,
3012+
"type": "boolean"
3013+
}
3014+
}
30053015
}
30063016
},
30073017
"default": {
@@ -4767,8 +4777,13 @@
47674777
"200": {
47684778
"description": "Success",
47694779
"schema": {
4770-
"default": false,
4771-
"type": "boolean"
4780+
"type": "object",
4781+
"properties": {
4782+
"value": {
4783+
"default": false,
4784+
"type": "boolean"
4785+
}
4786+
}
47724787
}
47734788
},
47744789
"default": {

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/TripService.OpenApi.V2.yaml

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -898,8 +898,11 @@ paths:
898898
'200':
899899
description: Success
900900
schema:
901-
default: false
902-
type: boolean
901+
type: object
902+
properties:
903+
value:
904+
default: false
905+
type: boolean
903906
default:
904907
$ref: '#/responses/error'
905908
x-ms-docs-operation-type: function
@@ -2079,8 +2082,11 @@ paths:
20792082
'200':
20802083
description: Success
20812084
schema:
2082-
default: false
2083-
type: boolean
2085+
type: object
2086+
properties:
2087+
value:
2088+
default: false
2089+
type: boolean
20842090
default:
20852091
$ref: '#/responses/error'
20862092
x-ms-docs-operation-type: function
@@ -3320,8 +3326,11 @@ paths:
33203326
'200':
33213327
description: Success
33223328
schema:
3323-
default: false
3324-
type: boolean
3329+
type: object
3330+
properties:
3331+
value:
3332+
default: false
3333+
type: boolean
33253334
default:
33263335
$ref: '#/responses/error'
33273336
x-ms-docs-operation-type: function

0 commit comments

Comments
 (0)