Skip to content

Commit 07f0d2e

Browse files
irvinesundaybaywet
irvinesunday
and
baywet
authored
[Fix] Skips adding a $count path if a similar count() function path exists (#351)
* Don't add $count paths if count() function paths exist * Update release notes * Update test * Update src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs Co-authored-by: Vincent Biret <[email protected]> * Update src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs Co-authored-by: Vincent Biret <[email protected]> * Update src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs * Update integration file * PR review suggestions * PR review suggestion * Apply suggestions from code review * Update src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs * Update tests --------- Co-authored-by: Vincent Biret <[email protected]>
1 parent a8d815a commit 07f0d2e

File tree

8 files changed

+127295
-60717
lines changed

8 files changed

+127295
-60717
lines changed

‎src/Microsoft.OpenApi.OData.Reader/Common/Constants.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,5 +189,10 @@ internal static class Constants
189189
/// entity name
190190
/// </summary>
191191
public static string EntityName = "entity";
192+
193+
/// <summary>
194+
/// count segment identifier
195+
/// </summary>
196+
public const string CountSegmentIdentifier = "count";
192197
}
193198
}

‎src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public class ODataPathProvider : IODataPathProvider
3131

3232
private IEdmModel _model;
3333

34+
private readonly IDictionary<IEdmEntityType, IList<ODataPath>> _dollarCountPaths =
35+
new Dictionary<IEdmEntityType, IList<ODataPath>>();
36+
3437
/// <summary>
3538
/// Can filter the <see cref="IEdmElement"/> or not.
3639
/// </summary>
@@ -99,6 +102,7 @@ protected virtual void Initialize(IEdmModel model)
99102
_allNavigationSourcePaths.Clear();
100103
_allNavigationPropertyPaths.Clear();
101104
_allOperationPaths.Clear();
105+
_dollarCountPaths.Clear();
102106
}
103107

104108
private IEnumerable<ODataPath> MergePaths()
@@ -142,6 +146,25 @@ private void AppendPath(ODataPath path)
142146
nsList = new List<ODataPath>();
143147
_allNavigationSourcePaths[navigationSourceSegment.EntityType] = nsList;
144148
}
149+
150+
if (kind == ODataPathKind.DollarCount)
151+
{
152+
if (_allOperationPaths.FirstOrDefault(p => DollarCountAndOperationPathsSimilar(p, path)) is not null)
153+
{
154+
// Don't add a path for $count if a similar count() function path already exists.
155+
return;
156+
}
157+
else
158+
{
159+
if (!_dollarCountPaths.TryGetValue(navigationSourceSegment.EntityType, out IList<ODataPath> dollarPathList))
160+
{
161+
dollarPathList = new List<ODataPath>();
162+
_dollarCountPaths[navigationSourceSegment.EntityType] = dollarPathList;
163+
}
164+
dollarPathList.Add(path);
165+
}
166+
}
167+
145168
nsList.Add(path);
146169
}
147170
break;
@@ -161,12 +184,47 @@ private void AppendPath(ODataPath path)
161184

162185
case ODataPathKind.Operation:
163186
case ODataPathKind.OperationImport:
187+
if (kind == ODataPathKind.Operation)
188+
{
189+
foreach (var kvp in _dollarCountPaths)
190+
{
191+
if (kvp.Value.FirstOrDefault(p => DollarCountAndOperationPathsSimilar(p, path)) is ODataPath dollarCountPath &&
192+
_allNavigationSourcePaths.TryGetValue(kvp.Key, out IList<ODataPath> dollarPathList))
193+
{
194+
dollarPathList.Remove(dollarCountPath);
195+
break;
196+
}
197+
}
198+
}
199+
164200
_allOperationPaths.Add(path);
165201
break;
166202

167203
default:
168204
return;
169205
}
206+
207+
bool DollarCountAndOperationPathsSimilar(ODataPath path1, ODataPath path2)
208+
{
209+
if ((path1.Kind == ODataPathKind.DollarCount &&
210+
path2.Kind == ODataPathKind.Operation && path2.LastSegment.Identifier.Equals(Constants.CountSegmentIdentifier, StringComparison.OrdinalIgnoreCase)) ||
211+
(path2.Kind == ODataPathKind.DollarCount &&
212+
path1.Kind == ODataPathKind.Operation && path1.LastSegment.Identifier.Equals(Constants.CountSegmentIdentifier, StringComparison.OrdinalIgnoreCase)))
213+
{
214+
return GetModifiedPathItemName(path1)?.Equals(GetModifiedPathItemName(path2), StringComparison.OrdinalIgnoreCase) ?? false;
215+
}
216+
217+
return false;
218+
}
219+
220+
string GetModifiedPathItemName(ODataPath path)
221+
{
222+
if (!path.Any()) return null;
223+
224+
IEnumerable<ODataSegment> modifiedSegments = path.Take(path.Count - 1);
225+
ODataPath modifiedPath = new(modifiedSegments);
226+
return modifiedPath.GetPathItemName();
227+
}
170228
}
171229

172230
/// <summary>

‎src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@
1515
<TargetFrameworks>netstandard2.0</TargetFrameworks>
1616
<PackageId>Microsoft.OpenApi.OData</PackageId>
1717
<SignAssembly>true</SignAssembly>
18-
<Version>1.3.0-preview1</Version>
18+
<Version>1.3.0-preview2</Version>
1919
<Description>This package contains the codes you need to convert OData CSDL to Open API Document of Model.</Description>
2020
<Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
2121
<PackageTags>Microsoft OpenApi OData EDM</PackageTags>
2222
<RepositoryUrl>https://github.com/Microsoft/OpenAPI.NET.OData</RepositoryUrl>
2323
<PackageReleaseNotes>
2424
- Update key path parameter descriptions #309
25+
- Skips adding a $count path if a similar count() function path exists #347
2526
</PackageReleaseNotes>
2627
<AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
2728
<AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmOperationProviderTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ public void FindOperationsReturnsCorrectCollectionOrOperations()
3434
var operations = provider.FindOperations(entitySet.EntityType(), false);
3535

3636
// Assert
37-
Assert.Equal(29, operations.Count());
37+
Assert.Equal(30, operations.Count());
3838

3939
// Act
4040
entitySet = model.EntityContainer.FindEntitySet("directoryObjects");
4141

4242
operations = provider.FindOperations(entitySet.EntityType(), false);
4343

4444
// Assert
45-
Assert.Equal(57, operations.Count());
45+
Assert.Equal(58, operations.Count());
4646
}
4747
}
4848
}

‎test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,18 @@ public void GetPathsForGraphBetaModelReturnsAllPaths()
4343
ODataPathProvider provider = new();
4444
OpenApiConvertSettings settings = new()
4545
{
46-
AddAlternateKeyPaths = true
46+
AddAlternateKeyPaths = true,
47+
PrefixEntityTypeNameBeforeKey = true
4748
};
4849

4950
// Act
5051
var paths = provider.GetPaths(model, settings);
5152

5253
// Assert
5354
Assert.NotNull(paths);
54-
Assert.Equal(18317, paths.Count());
55+
Assert.Null(paths.FirstOrDefault(p => p.GetPathItemName().Equals("/drives({id})/items({id1})/workbook/tables/$count")));
56+
Assert.NotNull(paths.FirstOrDefault(p => p.GetPathItemName().Equals("/drives({id})/items({id1})/workbook/tables/microsoft.graph.count()")));
57+
Assert.Equal(18024, paths.Count());
5558
}
5659

5760
[Fact]
@@ -72,7 +75,7 @@ public void GetPathsForGraphBetaModelWithDerivedTypesConstraintReturnsAllPaths()
7275

7376
// Assert
7477
Assert.NotNull(paths);
75-
Assert.Equal(19773, paths.Count());
78+
Assert.Equal(18675, paths.Count());
7679
}
7780

7881
[Fact]

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,29 @@ public void CreateLinksForSingletons()
158158

159159
// Assert
160160
Assert.NotNull(links);
161-
Assert.Equal(2, links.Count);
161+
Assert.Equal(5, links.Count);
162162
Assert.Collection(links,
163+
item =>
164+
{
165+
Assert.Equal("edge", item.Key);
166+
Assert.Equal("admin.GetEdge", item.Value.OperationId);
167+
},
168+
item =>
169+
{
170+
Assert.Equal("sharepoint", item.Key);
171+
Assert.Equal("admin.GetSharepoint", item.Value.OperationId);
172+
},
163173
item =>
164174
{
165175
Assert.Equal("serviceAnnouncement", item.Key);
166176
Assert.Equal("admin.GetServiceAnnouncement", item.Value.OperationId);
167177
},
168178
item =>
179+
{
180+
Assert.Equal("reportSettings", item.Key);
181+
Assert.Equal("admin.GetReportSettings", item.Value.OperationId);
182+
},
183+
item =>
169184
{
170185
Assert.Equal("windows", item.Key);
171186
Assert.Equal("admin.GetWindows", item.Value.OperationId);

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

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ public void CreateStructuredTypeSchemaForEntityTypeWithDiscriminatorValueEnabled
206206
""deletedDateTime"": {
207207
""pattern"": ""^[0-9]{4,}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])T([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]([.][0-9]{1,12})?(Z|[+-][0-9][0-9]:[0-9][0-9])$"",
208208
""type"": ""string"",
209+
""description"": ""Date and time when this object was deleted. Always null when the object hasn't been deleted."",
209210
""format"": ""date-time"",
210211
""nullable"": true
211212
},
@@ -218,10 +219,11 @@ public void CreateStructuredTypeSchemaForEntityTypeWithDiscriminatorValueEnabled
218219
""propertyName"": ""@odata.type"",
219220
""mapping"": {
220221
""#microsoft.graph.user"": ""#/components/schemas/microsoft.graph.user"",
222+
""#microsoft.graph.servicePrincipal"": ""#/components/schemas/microsoft.graph.servicePrincipal"",
221223
""#microsoft.graph.group"": ""#/components/schemas/microsoft.graph.group"",
222224
""#microsoft.graph.device"": ""#/components/schemas/microsoft.graph.device"",
225+
""#microsoft.graph.administrativeUnit"": ""#/components/schemas/microsoft.graph.administrativeUnit"",
223226
""#microsoft.graph.application"": ""#/components/schemas/microsoft.graph.application"",
224-
""#microsoft.graph.servicePrincipal"": ""#/components/schemas/microsoft.graph.servicePrincipal"",
225227
""#microsoft.graph.policyBase"": ""#/components/schemas/microsoft.graph.policyBase"",
226228
""#microsoft.graph.appManagementPolicy"": ""#/components/schemas/microsoft.graph.appManagementPolicy"",
227229
""#microsoft.graph.stsPolicy"": ""#/components/schemas/microsoft.graph.stsPolicy"",
@@ -231,14 +233,16 @@ public void CreateStructuredTypeSchemaForEntityTypeWithDiscriminatorValueEnabled
231233
""#microsoft.graph.claimsMappingPolicy"": ""#/components/schemas/microsoft.graph.claimsMappingPolicy"",
232234
""#microsoft.graph.activityBasedTimeoutPolicy"": ""#/components/schemas/microsoft.graph.activityBasedTimeoutPolicy"",
233235
""#microsoft.graph.authorizationPolicy"": ""#/components/schemas/microsoft.graph.authorizationPolicy"",
236+
""#microsoft.graph.tenantRelationshipAccessPolicyBase"": ""#/components/schemas/microsoft.graph.tenantRelationshipAccessPolicyBase"",
237+
""#microsoft.graph.crossTenantAccessPolicy"": ""#/components/schemas/microsoft.graph.crossTenantAccessPolicy"",
234238
""#microsoft.graph.tenantAppManagementPolicy"": ""#/components/schemas/microsoft.graph.tenantAppManagementPolicy"",
239+
""#microsoft.graph.externalIdentitiesPolicy"": ""#/components/schemas/microsoft.graph.externalIdentitiesPolicy"",
235240
""#microsoft.graph.permissionGrantPolicy"": ""#/components/schemas/microsoft.graph.permissionGrantPolicy"",
236241
""#microsoft.graph.servicePrincipalCreationPolicy"": ""#/components/schemas/microsoft.graph.servicePrincipalCreationPolicy"",
237242
""#microsoft.graph.identitySecurityDefaultsEnforcementPolicy"": ""#/components/schemas/microsoft.graph.identitySecurityDefaultsEnforcementPolicy"",
238243
""#microsoft.graph.extensionProperty"": ""#/components/schemas/microsoft.graph.extensionProperty"",
239244
""#microsoft.graph.endpoint"": ""#/components/schemas/microsoft.graph.endpoint"",
240245
""#microsoft.graph.resourceSpecificPermissionGrant"": ""#/components/schemas/microsoft.graph.resourceSpecificPermissionGrant"",
241-
""#microsoft.graph.administrativeUnit"": ""#/components/schemas/microsoft.graph.administrativeUnit"",
242246
""#microsoft.graph.contract"": ""#/components/schemas/microsoft.graph.contract"",
243247
""#microsoft.graph.directoryObjectPartnerReference"": ""#/components/schemas/microsoft.graph.directoryObjectPartnerReference"",
244248
""#microsoft.graph.directoryRole"": ""#/components/schemas/microsoft.graph.directoryRole"",
@@ -285,6 +289,7 @@ public void CreateStructuredTypeSchemaForComplexTypeWithDiscriminatorValueEnable
285289
""properties"": {
286290
""isBackup"": {
287291
""type"": ""boolean"",
292+
""description"": ""For a user in an approval stage, this property indicates whether the user is a backup fallback approver."",
288293
""nullable"": true
289294
},
290295
""@odata.type"": {
@@ -313,6 +318,7 @@ public void CreateStructuredTypeSchemaForComplexTypeWithDiscriminatorValueEnable
313318
""properties"": {
314319
""isBackup"": {
315320
""type"": ""boolean"",
321+
""description"": ""For a user in an approval stage, this property indicates whether the user is a backup fallback approver."",
316322
""nullable"": true
317323
},
318324
""@odata.type"": {
@@ -363,11 +369,11 @@ public void CreateStructuredTypePropertiesSchemaWithCustomAttributeReturnsCorrec
363369
""properties"": {
364370
""contributionToContentDiscoveryAsOrganizationDisabled"": {
365371
""type"": ""boolean"",
366-
""x-ms-isHidden"": ""true""
372+
""description"": ""Reflects the Office Delve organization level setting. When set to true, the organization doesn't have access to Office Delve. This setting is read-only and can only be changed by administrators in the SharePoint admin center.""
367373
},
368374
""contributionToContentDiscoveryDisabled"": {
369375
""type"": ""boolean"",
370-
""x-ms-isHidden"": ""true""
376+
""description"": ""When set to true, documents in the user's Office Delve are disabled. Users can control this setting in Office Delve.""
371377
},
372378
""itemInsights"": {
373379
""anyOf"": [
@@ -379,7 +385,20 @@ public void CreateStructuredTypePropertiesSchemaWithCustomAttributeReturnsCorrec
379385
""nullable"": true
380386
}
381387
],
382-
""x-ms-isHidden"": ""true"",
388+
""description"": ""The user's settings for the visibility of meeting hour insights, and insights derived between a user and other items in Microsoft 365, such as documents or sites. Get userInsightsSettings through this navigation property."",
389+
""x-ms-navigationProperty"": true
390+
},
391+
""contactMergeSuggestions"": {
392+
""anyOf"": [
393+
{
394+
""$ref"": ""#/components/schemas/microsoft.graph.contactMergeSuggestions""
395+
},
396+
{
397+
""type"": ""object"",
398+
""nullable"": true
399+
}
400+
],
401+
""description"": ""The user's settings for the visibility of merge suggestion for the duplicate contacts in the user's contact list."",
383402
""x-ms-navigationProperty"": true
384403
},
385404
""regionalAndLanguageSettings"": {
@@ -392,6 +411,7 @@ public void CreateStructuredTypePropertiesSchemaWithCustomAttributeReturnsCorrec
392411
""nullable"": true
393412
}
394413
],
414+
""description"": ""The user's preferences for languages, regional locale and date/time formatting."",
395415
""x-ms-navigationProperty"": true
396416
},
397417
""shiftPreferences"": {
@@ -404,6 +424,7 @@ public void CreateStructuredTypePropertiesSchemaWithCustomAttributeReturnsCorrec
404424
""nullable"": true
405425
}
406426
],
427+
""description"": ""The shift preferences for the user."",
407428
""x-ms-navigationProperty"": true
408429
}
409430
}
@@ -931,6 +952,7 @@ public void CreatePropertySchemaWithComputedAnnotationReturnsCorrectSchema(OpenA
931952
{
932953
Assert.Equal(@"{
933954
""format"": ""duration"",
955+
""description"": ""The length of the appointment, denoted in ISO8601 format."",
934956
""pattern"": ""^-?P([0-9]+D)?(T([0-9]+H)?([0-9]+M)?([0-9]+([.][0-9]+)?S)?)?$"",
935957
""type"": ""string"",
936958
""readOnly"": true
@@ -941,6 +963,7 @@ public void CreatePropertySchemaWithComputedAnnotationReturnsCorrectSchema(OpenA
941963
Assert.Equal(@"{
942964
""pattern"": ""^-?P([0-9]+D)?(T([0-9]+H)?([0-9]+M)?([0-9]+([.][0-9]+)?S)?)?$"",
943965
""type"": ""string"",
966+
""description"": ""The length of the appointment, denoted in ISO8601 format."",
944967
""format"": ""duration"",
945968
""readOnly"": true
946969
}".ChangeLineBreaks(), json);

0 commit comments

Comments
 (0)