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.

Sign up for GitHub

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/port 590 #630

Open
wants to merge 3 commits into
base: support/v1
Choose a base branch
from
Open

fix/port 590 #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4136c6a
Creates unique operation ids for paths with composable overloaded fun…
irvinesunday Oct 2, 2024
c53d3a5
Ensures unique generation of operation ids for paths with composable …
irvinesunday Oct 11, 2024
4c8eafa
chore: fix merge issue
baywet Dec 24, 2024
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
77 changes: 41 additions & 36 deletions src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

/// <inheritdoc/>
protected override void Initialize(ODataContext context, ODataPath path)
{
{
base.Initialize(context, path);

// It's bound operation, the first segment must be the navigaiton source.
Expand All @@ -57,14 +57,14 @@

HasTypeCast = path.Segments.Any(s => s is ODataTypeCastSegment);

_operationRestriction = Context.Model.GetRecord<OperationRestrictionsType>(TargetPath, CapabilitiesConstants.OperationRestrictions);
var operationRestrictions = Context.Model.GetRecord<OperationRestrictionsType>(EdmOperation, CapabilitiesConstants.OperationRestrictions);
_operationRestriction?.MergePropertiesIfNull(operationRestrictions);
_operationRestriction = Context.Model.GetRecord<OperationRestrictionsType>(TargetPath, CapabilitiesConstants.OperationRestrictions);
var operationRestrictions = Context.Model.GetRecord<OperationRestrictionsType>(EdmOperation, CapabilitiesConstants.OperationRestrictions);
_operationRestriction?.MergePropertiesIfNull(operationRestrictions);
_operationRestriction ??= operationRestrictions;
}

/// <inheritdoc/>
protected override void SetBasicInfo(OpenApiOperation operation)

Check warning on line 67 in src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 31 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
// Summary
operation.Summary = "Invoke " + (EdmOperation.IsAction() ? "action " : "function ") + EdmOperation.Name;
Expand All @@ -81,6 +81,7 @@
// duplicates in entity vs entityset functions/actions

List<string> identifiers = new();
string pathHash = string.Empty;
foreach (ODataSegment segment in Path.Segments)
{
if (segment is ODataKeySegment keySegment)
Expand All @@ -89,41 +90,45 @@
{
identifiers.Add(segment.EntityType.Name);
continue;
}

// We'll consider alternate keys in the operation id to eliminate potential duplicates with operation id of primary path
if (segment == Path.Segments.Last())
{
identifiers.Add("By" + string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x))));
}
else
{
identifiers.Add(keySegment.Identifier);
}
}

// We'll consider alternate keys in the operation id to eliminate potential duplicates with operation id of primary path
if (segment == Path.Segments.Last())

Check warning on line 96 in src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs

View workflow job for this annotation

GitHub Actions / Build 

Indexing at Count-1 should be used instead of the "Enumerable" extension method "Last" (https://rules.sonarsource.com/csharp/RSPEC-6608)
{
identifiers.Add("By" + string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x))));
}
else
{
identifiers.Add(keySegment.Identifier);
}
}
else if (segment is ODataOperationSegment opSegment)
{
if (opSegment.Operation is IEdmFunction function && Context.Model.IsOperationOverload(function))
{
// Hash the segment to avoid duplicate operationIds
pathHash = string.IsNullOrEmpty(pathHash)
? opSegment.GetPathHash(Context.Settings)
: (pathHash + opSegment.GetPathHash(Context.Settings)).GetHashSHA256().Substring(0, 4);
}

identifiers.Add(segment.Identifier);
}
else
{
identifiers.Add(segment.Identifier);
identifiers.Add(segment.Identifier);
}
}

string operationId = string.Join(".", identifiers);

if (EdmOperation.IsAction())
if (!string.IsNullOrEmpty(pathHash))
{
operation.OperationId = operationId;
operation.OperationId = operationId + "-" + pathHash;
}
else
{
if (Path.LastSegment is ODataOperationSegment operationSegment &&
Context.Model.IsOperationOverload(operationSegment.Operation))
{
operation.OperationId = operationId + "-" + Path.LastSegment.GetPathHash(Context.Settings);
}
else
{
operation.OperationId = operationId;
}
operation.OperationId = operationId;
}
}

Expand Down Expand Up @@ -195,7 +200,7 @@
}
}

private void AppendSystemQueryOptions(IEdmFunction function, OpenApiOperation operation)

Check warning on line 203 in src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (function.ReturnType.IsCollection())
{
Expand Down Expand Up @@ -269,15 +274,15 @@
if (Context.Settings.ShowExternalDocs)
{
var externalDocs = Context.Model.GetLinkRecord(TargetPath, CustomLinkRel) ??
Context.Model.GetLinkRecord(EdmOperation, CustomLinkRel);
if (externalDocs != null)
{
operation.ExternalDocs = new OpenApiExternalDocs()
{
Description = CoreConstants.ExternalDocsDescription,
Url = externalDocs.Href
};
Context.Model.GetLinkRecord(EdmOperation, CustomLinkRel);

if (externalDocs != null)
{
operation.ExternalDocs = new OpenApiExternalDocs()
{
Description = CoreConstants.ExternalDocsDescription,
Url = externalDocs.Href
};
}
}
}
Expand Down
95 changes: 94 additions & 1 deletion test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/EdmFunctionOperationHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// ------------------------------------------------------------
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// ------------------------------------------------------------
Expand Down Expand Up @@ -299,6 +299,99 @@ public void CreateOperationForOverloadEdmFunctionReturnsCorrectOperationId(bool
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void CreateOperationForComposableOverloadEdmFunctionReturnsCorrectOperationId(bool enableOperationId)
{
// Arrange
EdmModel model = new();
EdmEntityType customer = new("NS", "Customer");
customer.AddKeys(customer.AddStructuralProperty("ID", EdmPrimitiveTypeKind.Int32));
model.AddElement(customer);

// Overloaded function 1
EdmFunction function1 = new("NS", "MyFunction1", EdmCoreModel.Instance.GetString(false), true, null, false);
function1.AddParameter("entity", new EdmEntityTypeReference(customer, false));
model.AddElement(function1);

// Overloaded function 1
EdmFunction function2 = new("NS", "MyFunction1", EdmCoreModel.Instance.GetString(false), true, null, false);
function2.AddParameter("entity", new EdmEntityTypeReference(customer, false));
function2.AddParameter("param", EdmCoreModel.Instance.GetString(false));

model.AddElement(function2);

// Overloaded function 2
EdmFunction function3 = new("NS", "MyFunction2", EdmCoreModel.Instance.GetString(false), true, null, false);
function3.AddParameter("entity2", new EdmEntityTypeReference(customer, false));
model.AddElement(function3);

// Overloaded function 2
EdmFunction function4 = new("NS", "MyFunction2", EdmCoreModel.Instance.GetString(false), true, null, false);
function4.AddParameter("entity2", new EdmEntityTypeReference(customer, false));
function4.AddParameter("param", EdmCoreModel.Instance.GetString(false));
model.AddElement(function4);

EdmEntityContainer container = new("NS", "Default");
EdmEntitySet customers = new(container, "Customers", customer);
model.AddElement(container);

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

ODataPath path1 = new(new ODataNavigationSourceSegment(customers),
new ODataKeySegment(customer),
new ODataOperationSegment(function1),
new ODataOperationSegment(function3));

ODataPath path2 = new(new ODataNavigationSourceSegment(customers),
new ODataKeySegment(customer),
new ODataOperationSegment(function1),
new ODataOperationSegment(function4));

ODataPath path3 = new(new ODataNavigationSourceSegment(customers),
new ODataKeySegment(customer),
new ODataOperationSegment(function2),
new ODataOperationSegment(function3));

ODataPath path4 = new(new ODataNavigationSourceSegment(customers),
new ODataKeySegment(customer),
new ODataOperationSegment(function2),
new ODataOperationSegment(function4));

// Act
var operation1 = _operationHandler.CreateOperation(context, path1);
var operation2 = _operationHandler.CreateOperation(context, path2);
var operation3 = _operationHandler.CreateOperation(context, path3);
var operation4 = _operationHandler.CreateOperation(context, path4);

// Assert
Assert.NotNull(operation1);
Assert.NotNull(operation2);
Assert.NotNull(operation3);
Assert.NotNull(operation4);

if (enableOperationId)
{
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-c53d", operation1.OperationId);
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-4d93", operation2.OperationId);
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-a2b2", operation3.OperationId);
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-7bea", operation4.OperationId);
}
else
{
Assert.Null(operation1.OperationId);
Assert.Null(operation2.OperationId);
Assert.Null(operation3.OperationId);
Assert.Null(operation4.OperationId);
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down
Loading