Skip to content

Commit 32e673e

Browse files
irvinesundayirvinesunday
authored
Fixes potential duplicate operationIds in action/function paths (#98)
* Include EntityType name of ODataKeySegment in Action/Function paths This helps in preventing potential duplicate operationIds in entity vs entityset functions/actions * Update tests to validate action/function duplicate operationId fix * Update test files' action/function operationIds * Redesign how we retrieve the segment identifiers * Update test files in line with new segment retrieval redesign Co-authored-by: Irvine Sunday <[email protected]>
1 parent b4d3ae3 commit 32e673e

11 files changed

+97
-68
lines changed

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

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

6-
using System;
76
using System.Collections.Generic;
87
using System.Linq;
9-
using System.Text;
108
using Microsoft.OData.Edm;
119
using Microsoft.OpenApi.Any;
1210
using Microsoft.OpenApi.Models;
@@ -66,7 +64,26 @@ protected override void SetBasicInfo(OpenApiOperation operation)
6664
// OperationId
6765
if (Context.Settings.EnableOperationId)
6866
{
69-
string operationId = String.Join(".", Path.Segments.Where(s => !(s is ODataKeySegment)).Select(s => s.Identifier));
67+
// When the key segment is available,
68+
// its EntityType name will be used
69+
// in the operationId to avoid potential
70+
// duplicates in entity vs entityset functions/actions
71+
72+
List<string> identifiers = new();
73+
foreach (ODataSegment segment in Path.Segments)
74+
{
75+
if (segment is not ODataKeySegment)
76+
{
77+
identifiers.Add(segment.Identifier);
78+
}
79+
else
80+
{
81+
identifiers.Add(segment.EntityType.Name);
82+
}
83+
}
84+
85+
string operationId = string.Join(".", identifiers);
86+
7087
if (EdmOperation.IsAction())
7188
{
7289
operation.OperationId = operationId;

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,25 @@ public void CreateOperationForEdmActionReturnsCorrectOperationId(bool enableOper
116116
new ODataKeySegment(customer),
117117
new ODataOperationSegment(action));
118118

119+
ODataPath path2 = new ODataPath(new ODataNavigationSourceSegment(customers),
120+
new ODataOperationSegment(action));
121+
119122
// Act
120123
var operation = _operationHandler.CreateOperation(context, path);
124+
var operation2 = _operationHandler.CreateOperation(context, path2);
121125

122126
// Assert
123127
Assert.NotNull(operation);
124128

125129
if (enableOperationId)
126130
{
127-
Assert.Equal("Customers.MyAction", operation.OperationId);
131+
Assert.Equal("Customers.Customer.MyAction", operation.OperationId);
132+
Assert.Equal("Customers.MyAction", operation2.OperationId);
128133
}
129134
else
130135
{
131136
Assert.Null(operation.OperationId);
137+
Assert.Null(operation2.OperationId);
132138
}
133139
}
134140

@@ -171,7 +177,7 @@ public void CreateOperationForEdmActionWithTypeCastReturnsCorrectOperationId(boo
171177

172178
if (enableOperationId)
173179
{
174-
Assert.Equal("Customers.NS.VipCustomer.MyAction", operation.OperationId);
180+
Assert.Equal("Customers.Customer.NS.VipCustomer.MyAction", operation.OperationId);
175181
}
176182
else
177183
{

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,25 @@ public void CreateOperationForEdmFunctionReturnsCorrectOperationId(bool enableOp
116116
new ODataKeySegment(customer),
117117
new ODataOperationSegment(function));
118118

119+
ODataPath path2 = new ODataPath(new ODataNavigationSourceSegment(customers),
120+
new ODataOperationSegment(function));
121+
119122
// Act
120123
var operation = _operationHandler.CreateOperation(context, path);
124+
var operation2 = _operationHandler.CreateOperation(context, path2);
121125

122126
// Assert
123127
Assert.NotNull(operation);
124128

125129
if (enableOperationId)
126130
{
127-
Assert.Equal("Customers.MyFunction", operation.OperationId);
131+
Assert.Equal("Customers.Customer.MyFunction", operation.OperationId);
132+
Assert.Equal("Customers.MyFunction", operation2.OperationId);
128133
}
129134
else
130135
{
131136
Assert.Null(operation.OperationId);
137+
Assert.Null(operation2.OperationId);
132138
}
133139
}
134140

@@ -171,7 +177,7 @@ public void CreateOperationForEdmFunctionWithTypeCastReturnsCorrectOperationId(b
171177

172178
if (enableOperationId)
173179
{
174-
Assert.Equal("Customers.NS.VipCustomer.MyFunction", operation.OperationId);
180+
Assert.Equal("Customers.Customer.NS.VipCustomer.MyFunction", operation.OperationId);
175181
}
176182
else
177183
{
@@ -222,7 +228,7 @@ public void CreateOperationForOverloadEdmFunctionReturnsCorrectOperationId(bool
222228

223229
if (enableOperationId)
224230
{
225-
Assert.Equal("Customers.MyFunction-28ae", operation.OperationId);
231+
Assert.Equal("Customers.Customer.MyFunction-28ae", operation.OperationId);
226232
}
227233
else
228234
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@
594594
"Documents.Actions"
595595
],
596596
"summary": "Invoke action Upload",
597-
"operationId": "Documents.Upload",
597+
"operationId": "Documents.DocumentDto.Upload",
598598
"produces": [
599599
"application/json"
600600
],
@@ -2381,7 +2381,7 @@
23812381
"Tasks.Actions"
23822382
],
23832383
"summary": "Invoke action Upload",
2384-
"operationId": "Tasks.Upload",
2384+
"operationId": "Tasks.DocumentDto.Upload",
23852385
"produces": [
23862386
"application/json"
23872387
],

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ paths:
416416
tags:
417417
- Documents.Actions
418418
summary: Invoke action Upload
419-
operationId: Documents.Upload
419+
operationId: Documents.DocumentDto.Upload
420420
produces:
421421
- application/json
422422
parameters:
@@ -1720,7 +1720,7 @@ paths:
17201720
tags:
17211721
- Tasks.Actions
17221722
summary: Invoke action Upload
1723-
operationId: Tasks.Upload
1723+
operationId: Tasks.DocumentDto.Upload
17241724
produces:
17251725
- application/json
17261726
parameters:

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Multiple.Schema.OpenApi.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@
667667
"Documents.Actions"
668668
],
669669
"summary": "Invoke action Upload",
670-
"operationId": "Documents.Upload",
670+
"operationId": "Documents.DocumentDto.Upload",
671671
"parameters": [
672672
{
673673
"name": "Id",
@@ -2718,7 +2718,7 @@
27182718
"Tasks.Actions"
27192719
],
27202720
"summary": "Invoke action Upload",
2721-
"operationId": "Tasks.Upload",
2721+
"operationId": "Tasks.DocumentDto.Upload",
27222722
"parameters": [
27232723
{
27242724
"name": "Id",

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Multiple.Schema.OpenApi.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ paths:
462462
tags:
463463
- Documents.Actions
464464
summary: Invoke action Upload
465-
operationId: Documents.Upload
465+
operationId: Documents.DocumentDto.Upload
466466
parameters:
467467
- name: Id
468468
in: path
@@ -1935,7 +1935,7 @@ paths:
19351935
tags:
19361936
- Tasks.Actions
19371937
summary: Invoke action Upload
1938-
operationId: Tasks.Upload
1938+
operationId: Tasks.DocumentDto.Upload
19391939
parameters:
19401940
- name: Id
19411941
in: path

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,7 +1564,7 @@
15641564
"Me.Functions"
15651565
],
15661566
"summary": "Invoke function GetInvolvedPeople",
1567-
"operationId": "Me.Trips.GetInvolvedPeople",
1567+
"operationId": "Me.Trips.Trip.GetInvolvedPeople",
15681568
"produces": [
15691569
"application/json"
15701570
],
@@ -2598,7 +2598,7 @@
25982598
"NewComePeople.Functions"
25992599
],
26002600
"summary": "Invoke function GetFavoriteAirline",
2601-
"operationId": "NewComePeople.GetFavoriteAirline",
2601+
"operationId": "NewComePeople.Person.GetFavoriteAirline",
26022602
"produces": [
26032603
"application/json"
26042604
],
@@ -2632,7 +2632,7 @@
26322632
"NewComePeople.Functions"
26332633
],
26342634
"summary": "Invoke function GetFriendsTrips",
2635-
"operationId": "NewComePeople.GetFriendsTrips",
2635+
"operationId": "NewComePeople.Person.GetFriendsTrips",
26362636
"produces": [
26372637
"application/json"
26382638
],
@@ -2676,7 +2676,7 @@
26762676
"NewComePeople.Actions"
26772677
],
26782678
"summary": "Invoke action GetPeersForTrip",
2679-
"operationId": "NewComePeople.GetPeersForTrip",
2679+
"operationId": "NewComePeople.Person.GetPeersForTrip",
26802680
"consumes": [
26812681
"application/json"
26822682
],
@@ -2736,7 +2736,7 @@
27362736
"NewComePeople.Actions"
27372737
],
27382738
"summary": "Invoke action ShareTrip",
2739-
"operationId": "NewComePeople.ShareTrip",
2739+
"operationId": "NewComePeople.Person.ShareTrip",
27402740
"consumes": [
27412741
"application/json"
27422742
],
@@ -2787,7 +2787,7 @@
27872787
"NewComePeople.Functions"
27882788
],
27892789
"summary": "Invoke function UpdatePersonLastName",
2790-
"operationId": "NewComePeople.UpdatePersonLastName",
2790+
"operationId": "NewComePeople.Person.UpdatePersonLastName",
27912791
"produces": [
27922792
"application/json"
27932793
],
@@ -3161,7 +3161,7 @@
31613161
"NewComePeople.Functions"
31623162
],
31633163
"summary": "Invoke function GetInvolvedPeople",
3164-
"operationId": "NewComePeople.Trips.GetInvolvedPeople",
3164+
"operationId": "NewComePeople.Person.Trips.Trip.GetInvolvedPeople",
31653165
"produces": [
31663166
"application/json"
31673167
],
@@ -4227,7 +4227,7 @@
42274227
"People.Functions"
42284228
],
42294229
"summary": "Invoke function GetFavoriteAirline",
4230-
"operationId": "People.GetFavoriteAirline",
4230+
"operationId": "People.Person.GetFavoriteAirline",
42314231
"produces": [
42324232
"application/json"
42334233
],
@@ -4261,7 +4261,7 @@
42614261
"People.Functions"
42624262
],
42634263
"summary": "Invoke function GetFriendsTrips",
4264-
"operationId": "People.GetFriendsTrips",
4264+
"operationId": "People.Person.GetFriendsTrips",
42654265
"produces": [
42664266
"application/json"
42674267
],
@@ -4305,7 +4305,7 @@
43054305
"People.Actions"
43064306
],
43074307
"summary": "Invoke action GetPeersForTrip",
4308-
"operationId": "People.GetPeersForTrip",
4308+
"operationId": "People.Person.GetPeersForTrip",
43094309
"consumes": [
43104310
"application/json"
43114311
],
@@ -4365,7 +4365,7 @@
43654365
"People.Actions"
43664366
],
43674367
"summary": "Invoke action ShareTrip",
4368-
"operationId": "People.ShareTrip",
4368+
"operationId": "People.Person.ShareTrip",
43694369
"consumes": [
43704370
"application/json"
43714371
],
@@ -4416,7 +4416,7 @@
44164416
"People.Functions"
44174417
],
44184418
"summary": "Invoke function UpdatePersonLastName",
4419-
"operationId": "People.UpdatePersonLastName",
4419+
"operationId": "People.Person.UpdatePersonLastName",
44204420
"produces": [
44214421
"application/json"
44224422
],
@@ -4790,7 +4790,7 @@
47904790
"People.Functions"
47914791
],
47924792
"summary": "Invoke function GetInvolvedPeople",
4793-
"operationId": "People.Trips.GetInvolvedPeople",
4793+
"operationId": "People.Person.Trips.Trip.GetInvolvedPeople",
47944794
"produces": [
47954795
"application/json"
47964796
],

0 commit comments

Comments
 (0)