Skip to content

bugfix/missing quotes templates #143

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

Merged
merged 6 commits into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/Microsoft.OpenApi.OData.Reader/Edm/EdmTypeExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// ------------------------------------------------------------

using Microsoft.OData.Edm;

namespace Microsoft.OpenApi.OData.Edm
{
/// <summary>
/// Extension methods for <see cref="IEdmType"/>
/// </summary>
public static class EdmTypeExtensions
{
/// <summary>
/// Determines wether a path parameter should be wrapped in quotes based on the type of the parameter.
/// </summary>
/// <param name="edmType">The type of the parameter.</param>
/// <param name="settings">The conversion settings.</param>
/// <returns>True if the parameter should be wrapped in quotes, false otherwise.</returns>
public static bool ShouldPathParameterBeQuoted(this IEdmType edmType, OpenApiConvertSettings settings)
{
if (edmType == null || settings == null || !settings.AddSingleQuotesForStringParameters)
{
return false;
}

return edmType.TypeKind switch
{
EdmTypeKind.Enum => true,
EdmTypeKind.Primitive when edmType.IsString() || edmType.IsTemporal() => true,
_ => false,
};
}
}

}
11 changes: 3 additions & 8 deletions src/Microsoft.OpenApi.OData.Reader/Edm/ODataKeySegment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,7 @@ public override string GetPathItemName(OpenApiConvertSettings settings, HashSet<
}
else
{
IList<string> keyStrings = new List<string>();
foreach (var key in KeyMappings)
{
keyStrings.Add(key.Key + "={" + key.Value + "}");
}

return String.Join(",", keyStrings);
return string.Join(",", KeyMappings.Select(x => x.Key + "='{" + x.Value + "}'"));
}
}

Expand All @@ -109,7 +103,8 @@ public override string GetPathItemName(OpenApiConvertSettings settings, HashSet<
foreach (var keyProperty in keys)
{
string name = Utils.GetUniqueName(keyProperty.Name, parameters);
keyStrings.Add(keyProperty.Name + "={" + name + "}");
var quote = keyProperty.Type.Definition.ShouldPathParameterBeQuoted(settings) ? "'" : string.Empty;
keyStrings.Add($"{keyProperty.Name}={quote}{{{name}}}{quote}");
}

return String.Join(",", keyStrings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ private string FunctionImportName(IEdmFunctionImport functionImport, OpenApiConv
}
else
{
return p.Name + "={" + uniqueName + "}";
var quote = p.Type.Definition.ShouldPathParameterBeQuoted(settings) ? "'" : string.Empty;
return $"{p.Name}={quote}{{{uniqueName}}}{quote}";
}
})));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ private string FunctionName(IEdmFunction function, OpenApiConvertSettings settin
}
else
{
return p.Name + "={" + uniqueName + "}";
var quote = p.Type.Definition.ShouldPathParameterBeQuoted(settings) ? "'" : string.Empty;
return p.Name + $"={quote}{{{uniqueName}}}{quote}";
}
})));

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathKind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public enum ODataPathKind
Singleton,

/// <summary>
/// Represents an operation (function or action) path, for example: ~/users/NS.findRooms(roomId={roomId})
/// Represents an operation (function or action) path, for example: ~/users/NS.findRooms(roomId='{roomId}')
/// </summary>
Operation,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationPr
{
// Non-Contained
// Single-Valued: ~/entityset/{key}/single-valued-Nav/$ref
// Collection-valued: ~/entityset/{key}/collection-valued-Nav/$ref?$id ={navKey}
// Collection-valued: ~/entityset/{key}/collection-valued-Nav/$ref?$id='{navKey}'
CreateRefPath(currentPath);

if (targetsMany)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ public static IList<OpenApiParameter> CreateParameters(this ODataContext context

if (parameterNameMapping != null)
{
parameter.Description = $"Usage: {edmParameter.Name}={{{parameterNameMapping[edmParameter.Name]}}}";
var quote = edmParameter.Type.Definition.ShouldPathParameterBeQuoted(context.Settings) ? "'" : string.Empty;
parameter.Description = $"Usage: {edmParameter.Name}={quote}{{{parameterNameMapping[edmParameter.Name]}}}{quote}";
}

parameters.Add(parameter);
Expand Down Expand Up @@ -203,7 +204,8 @@ public static IList<OpenApiParameter> CreateKeyParameters(this ODataContext cont

if (keySegment.KeyMappings != null)
{
parameter.Description = parameter.Description + $", {keyProperty.Name}={{{parameter.Name}}}";
var quote = keyProperty.Type.Definition.ShouldPathParameterBeQuoted(context.Settings) ? "'" : string.Empty;
parameter.Description += $", {keyProperty.Name}={quote}{{{parameter.Name}}}{quote}";
}

parameter.Extensions.Add(Constants.xMsKeyType, new OpenApiString(entityType.Name));
Expand Down
6 changes: 6 additions & 0 deletions src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ public string PathPrefix
/// </summary>
public bool EnableDollarCountPath { get; set; } = true;

/// <summary>
/// Gets/sets a value indicating whether or not single quotes surrounding string parameters in url templates should be added.
/// </summary>
public bool AddSingleQuotesForStringParameters { get; set; } = false;

internal OpenApiConvertSettings Clone()
{
var newSettings = new OpenApiConvertSettings
Expand Down Expand Up @@ -219,6 +224,7 @@ internal OpenApiConvertSettings Clone()
ShowRootPath = this.ShowRootPath,
PathProvider = this.PathProvider,
EnableDollarCountPath = this.EnableDollarCountPath,
AddSingleQuotesForStringParameters = this.AddSingleQuotesForStringParameters,
};

return newSettings;
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.OpenApi.OData.Reader/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ abstract Microsoft.OpenApi.OData.Edm.ODataSegment.Identifier.get -> string
abstract Microsoft.OpenApi.OData.Edm.ODataSegment.Kind.get -> Microsoft.OpenApi.OData.Edm.ODataSegmentKind
Microsoft.OpenApi.OData.Common.Utils
Microsoft.OpenApi.OData.Edm.EdmModelExtensions
Microsoft.OpenApi.OData.Edm.EdmTypeExtensions
static Microsoft.OpenApi.OData.Edm.EdmTypeExtensions.ShouldPathParameterBeQuoted(this Microsoft.OData.Edm.IEdmType edmType, Microsoft.OpenApi.OData.OpenApiConvertSettings settings) -> bool
Microsoft.OpenApi.OData.Edm.IODataPathProvider
Microsoft.OpenApi.OData.Edm.IODataPathProvider.CanFilter(Microsoft.OData.Edm.IEdmElement element) -> bool
Microsoft.OpenApi.OData.Edm.IODataPathProvider.GetPaths(Microsoft.OData.Edm.IEdmModel model, Microsoft.OpenApi.OData.OpenApiConvertSettings settings) -> System.Collections.Generic.IEnumerable<Microsoft.OpenApi.OData.Edm.ODataPath>
Expand Down Expand Up @@ -130,6 +132,8 @@ Microsoft.OpenApi.OData.OpenApiConvertSettings.PathProvider.get -> Microsoft.Ope
Microsoft.OpenApi.OData.OpenApiConvertSettings.PathProvider.set -> void
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableDollarCountPath.get -> bool
Microsoft.OpenApi.OData.OpenApiConvertSettings.EnableDollarCountPath.set -> void
Microsoft.OpenApi.OData.OpenApiConvertSettings.AddSingleQuotesForStringParameters.get -> bool
Microsoft.OpenApi.OData.OpenApiConvertSettings.AddSingleQuotesForStringParameters.set -> void
Microsoft.OpenApi.OData.OpenApiConvertSettings.PrefixEntityTypeNameBeforeKey.get -> bool
Microsoft.OpenApi.OData.OpenApiConvertSettings.PrefixEntityTypeNameBeforeKey.set -> void
Microsoft.OpenApi.OData.OpenApiConvertSettings.RequireDerivedTypesConstraintForBoundOperations.get -> bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,22 @@ public void GetPathItemNameReturnsCorrectKeyLiteralForSimpleKey(bool prefix, str
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void GetPathItemNameReturnsCorrectKeyLiteralForCompositeKey(bool prefix)
[InlineData(true, true, "firstName='{firstName}',lastName='{lastName}'")]
[InlineData(false, true, "firstName='{firstName}',lastName='{lastName}'")]
[InlineData(true, false, "firstName={firstName},lastName={lastName}")]
[InlineData(false, false, "firstName={firstName},lastName={lastName}")]
public void GetPathItemNameReturnsCorrectKeyLiteralForCompositeKey(bool prefix, bool addQuotes, string expected)
{
// Arrange & Act
ODataKeySegment segment = new ODataKeySegment(_customer);
OpenApiConvertSettings settings = new OpenApiConvertSettings
{
PrefixEntityTypeNameBeforeKey = prefix
PrefixEntityTypeNameBeforeKey = prefix,
AddSingleQuotesForStringParameters = addQuotes,
};

// Assert
Assert.Equal("firstName={firstName},lastName={lastName}", segment.GetPathItemName(settings));
Assert.Equal(expected, segment.GetPathItemName(settings));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ public void GetPathItemNameReturnsCorrectFunctionImportLiteral()
var segment = new ODataOperationImportSegment(_functionImport);

// Assert
Assert.Equal("MyFunction(firstName={firstName},lastName={lastName})",
segment.GetPathItemName(new OpenApiConvertSettings()));
Assert.Equal("MyFunction(firstName='{firstName}',lastName='{lastName}')",
segment.GetPathItemName(new OpenApiConvertSettings() {
AddSingleQuotesForStringParameters = true
}));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ public void GetPathItemNameReturnsCorrectFunctionLiteral(bool unqualifiedCall, b

[Theory]
[InlineData(true, true, "{param}")]
[InlineData(true, false, "NS.MyFunction(param={param})")]
[InlineData(false, true, "NS.MyFunction(param={param})")]
[InlineData(false, false, "NS.MyFunction(param={param})")]
[InlineData(true, false, "NS.MyFunction(param='{param}')")]
[InlineData(false, true, "NS.MyFunction(param='{param}')")]
[InlineData(false, false, "NS.MyFunction(param='{param}')")]
public void GetPathItemNameReturnsCorrectFunctionLiteralForEscapedFunction(bool isEscapedFunction, bool enableEscapeFunctionCall, string expected)
{
// Arrange & Act
Expand All @@ -112,7 +112,8 @@ public void GetPathItemNameReturnsCorrectFunctionLiteralForEscapedFunction(bool
var segment = new ODataOperationSegment(boundFunction, isEscapedFunction);
OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableUriEscapeFunctionCall = enableEscapeFunctionCall
EnableUriEscapeFunctionCall = enableEscapeFunctionCall,
AddSingleQuotesForStringParameters = true,
};

// Assert
Expand All @@ -121,9 +122,9 @@ public void GetPathItemNameReturnsCorrectFunctionLiteralForEscapedFunction(bool

[Theory]
[InlineData(true, true, "{param}:")]
[InlineData(true, false, "NS.MyFunction(param={param})")]
[InlineData(false, true, "NS.MyFunction(param={param})")]
[InlineData(false, false, "NS.MyFunction(param={param})")]
[InlineData(true, false, "NS.MyFunction(param='{param}')")]
[InlineData(false, true, "NS.MyFunction(param='{param}')")]
[InlineData(false, false, "NS.MyFunction(param='{param}')")]
public void GetPathItemNameReturnsCorrectFunctionLiteralForEscapedComposableFunction(bool isEscapedFunction, bool enableEscapeFunctionCall, string expected)
{
// Arrange & Act
Expand All @@ -135,7 +136,8 @@ public void GetPathItemNameReturnsCorrectFunctionLiteralForEscapedComposableFunc
var segment = new ODataOperationSegment(boundFunction, isEscapedFunction);
OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableUriEscapeFunctionCall = enableEscapeFunctionCall
EnableUriEscapeFunctionCall = enableEscapeFunctionCall,
AddSingleQuotesForStringParameters = true
};

// Assert
Expand Down
11 changes: 6 additions & 5 deletions test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ public void GetPathItemNameReturnsCorrectWithSingleKeySegment(bool keyAsSegment,
}

[Theory]
[InlineData(true, true, "/Customers/FirstName={FirstName},LastName={LastName}")]
[InlineData(true, false, "/Customers/FirstName={FirstName},LastName={LastName}")]
[InlineData(false, true, "/Customers(FirstName={FirstName},LastName={LastName})")]
[InlineData(false, false, "/Customers(FirstName={FirstName},LastName={LastName})")]
[InlineData(true, true, "/Customers/FirstName='{FirstName}',LastName='{LastName}'")]
[InlineData(true, false, "/Customers/FirstName='{FirstName}',LastName='{LastName}'")]
[InlineData(false, true, "/Customers(FirstName='{FirstName}',LastName='{LastName}')")]
[InlineData(false, false, "/Customers(FirstName='{FirstName}',LastName='{LastName}')")]
public void GetPathItemNameReturnsCorrectStringWithMultipleKeySegment(bool keyAsSegment, bool prefix, string expected)
{
// Arrange
Expand All @@ -262,7 +262,8 @@ public void GetPathItemNameReturnsCorrectStringWithMultipleKeySegment(bool keyAs
OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableKeyAsSegment = keyAsSegment,
PrefixEntityTypeNameBeforeKey = prefix
PrefixEntityTypeNameBeforeKey = prefix,
AddSingleQuotesForStringParameters = true,
};

// Act
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ public void TripServiceMetadataToOpenApiJsonWorks(OpenApiSpecVersion specVersion
Version = new Version(1, 0, 1),
ServiceRoot = new Uri("http://services.odata.org/TrippinRESTierService"),
IEEE754Compatible = true,
OpenApiSpecVersion = specVersion
OpenApiSpecVersion = specVersion,
AddSingleQuotesForStringParameters = true,
};
// Act
string json = WriteEdmModelAsOpenApi(model, OpenApiFormat.Json, settings);
Expand Down Expand Up @@ -239,7 +240,8 @@ public void TripServiceMetadataToOpenApiYamlWorks(OpenApiSpecVersion specVersion
Version = new Version(1, 0, 1),
ServiceRoot = new Uri("http://services.odata.org/TrippinRESTierService"),
IEEE754Compatible = true,
OpenApiSpecVersion = specVersion
OpenApiSpecVersion = specVersion,
AddSingleQuotesForStringParameters = true,
};

// Act
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ public void CreatePathItemsReturnsForBasicModel()
[InlineData(true, true, true, "/Customers({ID}):/{param}:")]
[InlineData(true, true, false, "/Customers({ID}):/{param}")]

[InlineData(true, false, true, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(true, false, false, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(false, true, true, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(false, true, false, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(false, false, true, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(false, false, false, "/Customers({ID})/NS.MyFunction(param={param})")]
[InlineData(true, false, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]
[InlineData(true, false, false, "/Customers({ID})/NS.MyFunction(param='{param}')")]
[InlineData(false, true, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]
[InlineData(false, true, false, "/Customers({ID})/NS.MyFunction(param='{param}')")]
[InlineData(false, false, true, "/Customers({ID})/NS.MyFunction(param='{param}')")]
[InlineData(false, false, false, "/Customers({ID})/NS.MyFunction(param='{param}')")]
public void CreatePathItemsReturnsForEscapeFunctionModel(bool enableEscaped, bool hasEscapedAnnotation, bool isComposable, string expected)
{
// Arrange
Expand Down Expand Up @@ -108,7 +108,8 @@ public void CreatePathItemsReturnsForEscapeFunctionModel(bool enableEscaped, boo

OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableUriEscapeFunctionCall = enableEscaped
EnableUriEscapeFunctionCall = enableEscaped,
AddSingleQuotesForStringParameters = true,
};
ODataContext context = new ODataContext(model, settings);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ public void CreateOperationIdWithSHA5ForOverloadEdmFunctionImport(bool enableOpe

OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableOperationId = enableOperationId
EnableOperationId = enableOperationId,
AddSingleQuotesForStringParameters = true,
};
ODataContext context = new ODataContext(model, settings);

Expand All @@ -134,7 +135,7 @@ public void CreateOperationIdWithSHA5ForOverloadEdmFunctionImport(bool enableOpe

if (enableOperationId)
{
Assert.Equal("FunctionImport.MyFunction-3e3f", operation.OperationId);
Assert.Equal("FunctionImport.MyFunction-cc1c", operation.OperationId);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ public void CreateOperationForOverloadEdmFunctionReturnsCorrectOperationId(bool

OpenApiConvertSettings settings = new OpenApiConvertSettings
{
EnableOperationId = enableOperationId
EnableOperationId = enableOperationId,
AddSingleQuotesForStringParameters = true,
};
ODataContext context = new ODataContext(model, settings);

Expand All @@ -229,7 +230,7 @@ public void CreateOperationForOverloadEdmFunctionReturnsCorrectOperationId(bool

if (enableOperationId)
{
Assert.Equal("Customers.Customer.MyFunction-28ae", operation.OperationId);
Assert.Equal("Customers.Customer.MyFunction-df74", operation.OperationId);
}
else
{
Expand Down
Loading