Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Returns response status code 2XX for PUT operations of stream properties when UseSuccessStatusCodeRange is enabled #357

Merged
merged 9 commits into from
Mar 21, 2023
5 changes: 5 additions & 0 deletions src/Microsoft.OpenApi.OData.Reader/Common/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,5 +194,10 @@ internal static class Constants
/// count segment identifier
/// </summary>
public const string CountSegmentIdentifier = "count";

/// <summary>
/// content string identifier
/// </summary>
public const string Content = "content";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,48 +20,48 @@ public static class OpenApiOperationExtensions
/// </summary>
/// <param name="operation">The operation.</param>
/// <param name="settings">The settings.</param>
/// <param name="addNoContent">Whether to add a 204 no content response.</param>
/// <param name="addNoContent">Optional: Whether to add a 204 no content response.</param>
/// <param name="schema">Optional: The OpenAPI schema of the response.</param>
public static void AddErrorResponses(this OpenApiOperation operation, OpenApiConvertSettings settings, bool addNoContent = false, OpenApiSchema schema = null)
{
if (operation == null) {
throw Error.ArgumentNull(nameof(operation));
}
if(settings == null) {
throw Error.ArgumentNull(nameof(settings));
}

if(operation.Responses == null)
Utils.CheckArgumentNull(operation, nameof(operation));
Utils.CheckArgumentNull(settings, nameof(settings));

if (operation.Responses == null)
{
operation.Responses = new();
}

if (addNoContent)
{
if (settings.UseSuccessStatusCodeRange && schema != null)
{
if (settings.UseSuccessStatusCodeRange)
{
OpenApiResponse response = new()
OpenApiResponse response = null;
if (schema != null)
{
Content = new Dictionary<string, OpenApiMediaType>
response = new()
{
Content = new Dictionary<string, OpenApiMediaType>
{
Constants.ApplicationJsonMediaType,
new OpenApiMediaType
{
Schema = schema
Constants.ApplicationJsonMediaType,
new OpenApiMediaType
{
Schema = schema
}
}
}
}
};
operation.Responses.Add(Constants.StatusCodeClass2XX, response);
};
}
operation.Responses.Add(Constants.StatusCodeClass2XX, response ?? Constants.StatusCodeClass2XX.GetResponse());
}
else
{
operation.Responses.Add(Constants.StatusCode204, Constants.StatusCode204.GetResponse());
}
}
}

if(settings.ErrorResponsesAsDefault)
if (settings.ErrorResponsesAsDefault)
{
operation.Responses.Add(Constants.StatusCodeDefault, Constants.StatusCodeDefault.GetResponse());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ internal static class OpenApiResponseGenerator
},

{ Constants.StatusCode204, new OpenApiResponse { Description = "Success"} },
{ Constants.StatusCode201, new OpenApiResponse { Description = "Created"} },
{ Constants.StatusCodeClass2XX, new OpenApiResponse { Description = "Success"} },
baywet marked this conversation as resolved.
Show resolved Hide resolved
{ Constants.StatusCodeClass4XX, new OpenApiResponse
{
UnresolvedReference = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<PackageId>Microsoft.OpenApi.OData</PackageId>
<SignAssembly>true</SignAssembly>
<Version>1.3.0-preview2</Version>
<Version>1.3.0-preview3</Version>
<Description>This package contains the codes you need to convert OData CSDL to Open API Document of Model.</Description>
<Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
<PackageTags>Microsoft OpenApi OData EDM</PackageTags>
Expand All @@ -25,6 +25,7 @@
- Skips adding a $count path if a similar count() function path exists #347
- Checks whether path exists before adding it to the paths dictionary #343
- Strips namespace prefix from operation segments and aliases type cast segments #348
- Return response status code 2XX for PUT operations of stream properties when UseSuccessStatusCodeRange is enabled #310
</PackageReleaseNotes>
<AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
<AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,23 @@ protected override void SetBasicInfo(OpenApiOperation operation)
// Description
if (LastSegmentIsStreamPropertySegment)
{
IEdmVocabularyAnnotatable annotatable = GetAnnotatableElement();
(_, var property) = GetStreamElements();
string description;

if (annotatable is IEdmNavigationProperty)
if (property is IEdmNavigationProperty)
{
ReadRestrictionsType readRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(annotatable, CapabilitiesConstants.NavigationRestrictions)?
ReadRestrictionsType readRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(property, CapabilitiesConstants.NavigationRestrictions)?
.RestrictedProperties?.FirstOrDefault()?.ReadRestrictions;

description = LastSegmentIsKeySegment
? readRestriction?.ReadByKeyRestrictions?.Description
: readRestriction?.Description
?? Context.Model.GetDescriptionAnnotation(annotatable);
?? Context.Model.GetDescriptionAnnotation(property);
}
else
{
// Structural property
description = Context.Model.GetDescriptionAnnotation(annotatable);
description = Context.Model.GetDescriptionAnnotation(property);
}

operation.Description = description;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ protected IDictionary<string, OpenApiMediaType> GetContentDescription()
};

// Fetch the respective AcceptableMediaTypes
IEdmVocabularyAnnotatable annotatableElement = GetAnnotatableElement();
(_, var property) = GetStreamElements();
IEnumerable<string> mediaTypes = null;
if (annotatableElement != null)
if (property != null)
{
mediaTypes = Context.Model.GetCollection(annotatableElement,
mediaTypes = Context.Model.GetCollection(property,
CoreConstants.AcceptableMediaTypes);
}

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

/// <summary>
/// Gets the annotatable stream property from the path segments.
/// Gets the stream property and entity type declaring the stream property.
/// </summary>
/// <returns>The annotatable stream property.</returns>
protected IEdmVocabularyAnnotatable GetAnnotatableElement()
/// <returns>The stream property and entity type declaring the stream property.</returns>
protected (IEdmEntityType entityType, IEdmProperty property) GetStreamElements()
{
// Only ODataStreamPropertySegment is annotatable
if (!LastSegmentIsStreamPropertySegment) return null;
if (!LastSegmentIsStreamPropertySegment) return (null, null);

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

return property;
return (entityType, property);
}

private IEdmStructuralProperty GetStructuralProperty(IEdmEntityType entityType, string identifier)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// ------------------------------------------------------------

using System;
using System.Linq;
using Microsoft.OData.Edm;
using Microsoft.OData.Edm.Vocabularies;
Expand Down Expand Up @@ -34,20 +35,20 @@ protected override void SetBasicInfo(OpenApiOperation operation)
// Description
if (LastSegmentIsStreamPropertySegment)
{
IEdmVocabularyAnnotatable annotatable = GetAnnotatableElement();
(_, var property) = GetStreamElements();
string description;

if (annotatable is IEdmNavigationProperty)
if (property is IEdmNavigationProperty)
{
UpdateRestrictionsType updateRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(annotatable, CapabilitiesConstants.NavigationRestrictions)?
UpdateRestrictionsType updateRestriction = Context.Model.GetRecord<NavigationRestrictionsType>(property, CapabilitiesConstants.NavigationRestrictions)?
.RestrictedProperties?.FirstOrDefault()?.UpdateRestrictions;

description = updateRestriction?.Description ?? Context.Model.GetDescriptionAnnotation(annotatable);
description = updateRestriction?.Description ?? Context.Model.GetDescriptionAnnotation(property);
}
else
{
// Structural property
description = Context.Model.GetDescriptionAnnotation(annotatable);
description = Context.Model.GetDescriptionAnnotation(property);
}

operation.Description = description;
Expand Down Expand Up @@ -77,7 +78,28 @@ protected override void SetRequestBody(OpenApiOperation operation)
/// <inheritdoc/>
protected override void SetResponses(OpenApiOperation operation)
{
operation.AddErrorResponses(Context.Settings, true);
if (LastSegmentIsStreamPropertySegment && Path.LastSegment.Identifier.Equals(Constants.Content, StringComparison.OrdinalIgnoreCase))
{
// Get the entity type declaring this stream property.
(var entityType, _) = GetStreamElements();

OpenApiSchema schema = new()
{
UnresolvedReference = true,
Reference = new OpenApiReference
{
Type = ReferenceType.Schema,
Id = entityType.FullName()
}
};

operation.AddErrorResponses(Context.Settings, addNoContent: true, schema: schema);
}
else
{
operation.AddErrorResponses(Context.Settings, true);
}

base.SetResponses(operation);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public static IEdmModel GetEdmModel(string annotation)
</Key>
<Property Name=""Id"" Type=""Edm.Int32"" Nullable=""false"" />
<Property Name=""Logo"" Type=""Edm.Stream""/>
<Property Name=""Content"" Type=""Edm.Stream""/>
<Property Name = ""Description"" Type = ""Edm.String"" />
</EntityType>
<EntityType Name=""user"" OpenType=""true"">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ public class MediaEntityPutOperationHandlerTests
private readonly MediaEntityPutOperationHandler _operationalHandler = new MediaEntityPutOperationHandler();

[Theory]
[InlineData(true)]
[InlineData(false)]
public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOperationId)
[InlineData(true, false)]
[InlineData(true, true)]
[InlineData(false, false)]
[InlineData(false, true)]
public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOperationId, bool useSuccessStatusCodeRange)
{
// Arrange
string qualifiedName = CoreConstants.AcceptableMediaTypes;
Expand All @@ -33,17 +35,18 @@ public void CreateMediaEntityPutOperationReturnsCorrectOperation(bool enableOper
<Annotation Term=""Org.OData.Core.V1.Description"" String=""The logo image."" />";

// Assert
VerifyMediaEntityPutOperation("", enableOperationId);
VerifyMediaEntityPutOperation(annotation, enableOperationId);
VerifyMediaEntityPutOperation("", enableOperationId, useSuccessStatusCodeRange);
VerifyMediaEntityPutOperation(annotation, enableOperationId, useSuccessStatusCodeRange);
}

private void VerifyMediaEntityPutOperation(string annotation, bool enableOperationId)
private void VerifyMediaEntityPutOperation(string annotation, bool enableOperationId, bool useSuccessStatusCodeRange)
{
// Arrange
IEdmModel model = MediaEntityGetOperationHandlerTests.GetEdmModel(annotation);
OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableOperationId = enableOperationId
EnableOperationId = enableOperationId,
UseSuccessStatusCodeRange = useSuccessStatusCodeRange
};

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

IEdmStructuralProperty sp2 = todo.StructuralProperties().First(c => c.Name == "Content");
ODataPath path3 = new(new ODataNavigationSourceSegment(todos),
new ODataKeySegment(todos.EntityType()),
new ODataStreamPropertySegment(sp2.Name));

// Act
var putOperation = _operationalHandler.CreateOperation(context, path);
var putOperation2 = _operationalHandler.CreateOperation(context, path2);
var putOperation3 = _operationalHandler.CreateOperation(context, path3);

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

Assert.NotNull(putOperation.Responses);
Assert.NotNull(putOperation2.Responses);
Assert.NotNull(putOperation3.Responses);

Assert.Equal(2, putOperation.Responses.Count);
Assert.Equal(2, putOperation2.Responses.Count);
Assert.Equal(new[] { "204", "default" }, putOperation.Responses.Select(r => r.Key));
Assert.Equal(new[] { "204", "default" }, putOperation2.Responses.Select(r => r.Key));
Assert.Equal(2, putOperation3.Responses.Count);

var statusCode = (useSuccessStatusCodeRange) ? Constants.StatusCodeClass2XX : Constants.StatusCode204;
Assert.Equal(new[] { statusCode, "default" }, putOperation.Responses.Select(r => r.Key));
Assert.Equal(new[] { statusCode, "default" }, putOperation2.Responses.Select(r => r.Key));
Assert.Equal(new[] { statusCode, "default" }, putOperation3.Responses.Select(r => r.Key));

// Test only for stream properties of identifier 'content'
if (useSuccessStatusCodeRange)
{
var referenceId = putOperation3.Responses[statusCode]?.Content[Constants.ApplicationJsonMediaType]?.Schema?.Reference.Id;
Assert.NotNull(referenceId);
Assert.Equal("microsoft.graph.Todo", referenceId);
}
else
{
Assert.Empty(putOperation3.Responses[statusCode].Content);
}

if (!string.IsNullOrEmpty(annotation))
{
Expand Down