Skip to content

Commit 611067f

Browse files
irvinesundayirvinesunday
authored
[Enhancement] Creates DELETE operation for collection-valued nav. props $ref paths (#112)
* Generate collection nav. prop. entity paths for non-contained nav. props * Revert previous changes * Paths with DELETE operations for collection-valued nav. prop. $ref paths * Don't create extra path for OData Key segment when creating DELETE for $ref * Update tests * Update test files * Minor update to test commit signing * Another minor update to trigger commit signing * Update tests appropriately
1 parent e6a5e52 commit 611067f

12 files changed

+1851
-52
lines changed

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

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -271,37 +271,38 @@ private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationPr
271271

272272
// Check whether a collection-valued navigation property should be indexed by key value(s).
273273
NavigationPropertyRestriction restriction = navigation?.RestrictedProperties?.FirstOrDefault();
274+
274275
if (restriction == null || restriction.IndexableByKey == true)
275276
{
277+
IEdmEntityType navEntityType = navigationProperty.ToEntityType();
278+
276279
if (!navigationProperty.ContainsTarget)
277280
{
278281
// Non-Contained
279-
// Single-Valued: DELETE ~/entityset/{key}/single-valued-Nav/$ref
280-
// collection-valued: DELETE ~/entityset/{key}/collection-valued-Nav/$ref?$id ={ navKey}
281-
ODataPath newPath = currentPath.Clone();
282-
newPath.Push(ODataRefSegment.Instance); // $ref
283-
AppendPath(newPath);
282+
// Single-Valued: ~/entityset/{key}/single-valued-Nav/$ref
283+
// Collection-valued: ~/entityset/{key}/collection-valued-Nav/$ref?$id ={navKey}
284+
CreateRefPath(currentPath);
285+
286+
if (navigationProperty.TargetMultiplicity() == EdmMultiplicity.Many)
287+
{
288+
// Collection-valued: DELETE ~/entityset/{key}/collection-valued-Nav/{key}/$ref
289+
currentPath.Push(new ODataKeySegment(navEntityType));
290+
CreateRefPath(currentPath);
291+
}
292+
293+
// Get possible stream paths for the navigation entity type
294+
RetrieveMediaEntityStreamPaths(navEntityType, currentPath);
284295
}
285296
else
286297
{
287-
IEdmEntityType navEntityType = navigationProperty.ToEntityType();
288-
289298
// append a navigation property key.
290299
if (navigationProperty.TargetMultiplicity() == EdmMultiplicity.Many)
291300
{
292301
currentPath.Push(new ODataKeySegment(navEntityType));
293302
AppendPath(currentPath.Clone());
294-
295-
if (!navigationProperty.ContainsTarget)
296-
{
297-
// TODO: Shall we add "$ref" after {key}, and only support delete?
298-
// ODataPath newPath = currentPath.Clone();
299-
// newPath.Push(ODataRefSegment.Instance); // $ref
300-
// AppendPath(newPath);
301-
}
302303
}
303304

304-
// Get possible navigation property stream paths
305+
// Get possible stream paths for the navigation entity type
305306
RetrieveMediaEntityStreamPaths(navEntityType, currentPath);
306307

307308
if (shouldExpand)
@@ -315,11 +316,11 @@ private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationPr
315316
}
316317
}
317318
}
319+
}
318320

319-
if (navigationProperty.TargetMultiplicity() == EdmMultiplicity.Many)
320-
{
321-
currentPath.Pop();
322-
}
321+
if (navigationProperty.TargetMultiplicity() == EdmMultiplicity.Many)
322+
{
323+
currentPath.Pop();
323324
}
324325
}
325326
currentPath.Pop();
@@ -349,6 +350,17 @@ private bool ShouldExpandNavigationProperty(IEdmNavigationProperty navigationPro
349350
return true;
350351
}
351352

353+
/// <summary>
354+
/// Create $ref paths.
355+
/// </summary>
356+
/// <param name="currentPath">The current OData path.</param>
357+
private void CreateRefPath(ODataPath currentPath)
358+
{
359+
ODataPath newPath = currentPath.Clone();
360+
newPath.Push(ODataRefSegment.Instance); // $ref
361+
AppendPath(newPath);
362+
}
363+
352364
/// <summary>
353365
/// Retrieve all bounding <see cref="IEdmOperation"/>.
354366
/// </summary>

‎src/Microsoft.OpenApi.OData.Reader/PathItem/RefPathItemHandler.cs‎

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -68,37 +68,62 @@ protected override void SetOperations(OpenApiPathItem item)
6868
// So far, we only consider the non-containment
6969
Debug.Assert(!NavigationProperty.ContainsTarget);
7070

71-
// It seems OData supports to "GetRef, DeleteRef",
72-
// Here at this time,let's only consider the "delete"
73-
ReadRestrictionsType read = restriction?.ReadRestrictions;
74-
if (read == null || read.IsReadable)
75-
{
76-
AddOperation(item, OperationType.Get);
77-
}
78-
7971
// Create the ref
8072
if (NavigationProperty.TargetMultiplicity() == EdmMultiplicity.Many)
8173
{
82-
InsertRestrictionsType insert = restriction?.InsertRestrictions;
83-
if (insert == null || insert.IsInsertable)
74+
ODataSegment penultimateSegment = Path.Segments.Reverse().Skip(1).First();
75+
if (penultimateSegment is ODataKeySegment)
76+
{
77+
// Collection-valued: DELETE ~/entityset/{key}/collection-valued-Nav/{key}/$ref
78+
AddDeleteOperation(item, restriction);
79+
}
80+
else
8481
{
85-
AddOperation(item, OperationType.Post);
82+
AddReadOperation(item, restriction);
83+
AddInsertOperation(item, restriction);
8684
}
8785
}
8886
else
8987
{
90-
UpdateRestrictionsType update = restriction?.UpdateRestrictions;
91-
if (update == null || update.IsUpdatable)
92-
{
93-
AddOperation(item, OperationType.Put);
94-
}
88+
AddReadOperation(item, restriction);
89+
AddUpdateOperation(item, restriction);
90+
AddDeleteOperation(item, restriction);
91+
}
92+
}
9593

96-
// delete the link
97-
DeleteRestrictionsType delete = restriction?.DeleteRestrictions;
98-
if (delete == null || delete.IsDeletable)
99-
{
100-
AddOperation(item, OperationType.Delete);
101-
}
94+
private void AddDeleteOperation(OpenApiPathItem item, NavigationPropertyRestriction restriction)
95+
{
96+
DeleteRestrictionsType delete = restriction?.DeleteRestrictions;
97+
if (delete == null || delete.IsDeletable)
98+
{
99+
AddOperation(item, OperationType.Delete);
100+
}
101+
}
102+
103+
private void AddReadOperation(OpenApiPathItem item, NavigationPropertyRestriction restriction)
104+
{
105+
ReadRestrictionsType read = restriction?.ReadRestrictions;
106+
if (read == null || read.IsReadable)
107+
{
108+
AddOperation(item, OperationType.Get);
109+
}
110+
}
111+
112+
private void AddInsertOperation(OpenApiPathItem item, NavigationPropertyRestriction restriction)
113+
{
114+
InsertRestrictionsType insert = restriction?.InsertRestrictions;
115+
if (insert == null || insert.IsInsertable)
116+
{
117+
AddOperation(item, OperationType.Post);
118+
}
119+
}
120+
121+
private void AddUpdateOperation(OpenApiPathItem item, NavigationPropertyRestriction restriction)
122+
{
123+
UpdateRestrictionsType update = restriction?.UpdateRestrictions;
124+
if (update == null || update.IsUpdatable)
125+
{
126+
AddOperation(item, OperationType.Put);
102127
}
103128
}
104129

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// ------------------------------------------------------------
1+
// --------------------------------------------------------------
22
// Copyright (c) Microsoft Corporation. All rights reserved.
33
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
4-
// ------------------------------------------------------------
4+
// --------------------------------------------------------------
55

66
using System;
77
using System.Collections.Generic;
@@ -48,7 +48,7 @@ public void GetPathsForGraphBetaModelReturnsAllPaths()
4848

4949
// Assert
5050
Assert.NotNull(paths);
51-
Assert.Equal(17313, paths.Count());
51+
Assert.Equal(17401, paths.Count());
5252
}
5353

5454
[Fact]
@@ -67,7 +67,7 @@ public void GetPathsForGraphBetaModelWithDerivedTypesConstraintReturnsAllPaths()
6767

6868
// Assert
6969
Assert.NotNull(paths);
70-
Assert.Equal(13642, paths.Count());
70+
Assert.Equal(13730, paths.Count());
7171
}
7272

7373
[Fact]
@@ -336,7 +336,7 @@ public void GetPathsWithFalseNavigabilityInNavigationRestrictionsAnnotationWorks
336336

337337
// Assert
338338
Assert.NotNull(paths);
339-
Assert.Equal(6, paths.Count());
339+
Assert.Equal(7, paths.Count());
340340

341341
var pathItems = paths.Select(p => p.GetPathItemName()).ToList();
342342
Assert.DoesNotContain("/Orders({id})/SingleCustomer", pathItems);
@@ -409,13 +409,14 @@ public void GetPathsWithNonContainedNavigationPropertyWorks()
409409

410410
// Assert
411411
Assert.NotNull(paths);
412-
Assert.Equal(8, paths.Count());
412+
Assert.Equal(9, paths.Count());
413413

414414
var pathItems = paths.Select(p => p.GetPathItemName()).ToList();
415415
Assert.Contains("/Orders({id})/MultipleCustomers", pathItems);
416416
Assert.Contains("/Orders({id})/SingleCustomer", pathItems);
417417
Assert.Contains("/Orders({id})/SingleCustomer/$ref", pathItems);
418418
Assert.Contains("/Orders({id})/MultipleCustomers/$ref", pathItems);
419+
Assert.Contains("/Orders({id})/MultipleCustomers({ID})/$ref", pathItems);
419420
}
420421

421422
[Fact]
@@ -477,22 +478,22 @@ public void GetPathsWithStreamPropertyAndWithEntityHasStreamWorks(bool hasStream
477478
{
478479
if (hasStream)
479480
{
480-
Assert.Equal(12, paths.Count());
481+
Assert.Equal(13, paths.Count());
481482
Assert.Contains(TodosValuePath, paths.Select(p => p.GetPathItemName()));
482483
Assert.Contains(TodosLogoPath, paths.Select(p => p.GetPathItemName()));
483484
Assert.DoesNotContain(TodosContentPath, paths.Select(p => p.GetPathItemName()));
484485
}
485486
else
486487
{
487-
Assert.Equal(11, paths.Count());
488+
Assert.Equal(12, paths.Count());
488489
Assert.Contains(TodosLogoPath, paths.Select(p => p.GetPathItemName()));
489490
Assert.DoesNotContain(TodosContentPath, paths.Select(p => p.GetPathItemName()));
490491
Assert.DoesNotContain(TodosValuePath, paths.Select(p => p.GetPathItemName()));
491492
}
492493
}
493494
else if (streamPropName.Equals("content"))
494495
{
495-
Assert.Equal(11, paths.Count());
496+
Assert.Equal(12, paths.Count());
496497
Assert.Contains(TodosContentPath, paths.Select(p => p.GetPathItemName()));
497498
Assert.DoesNotContain(TodosLogoPath, paths.Select(p => p.GetPathItemName()));
498499
Assert.DoesNotContain(TodosValuePath, paths.Select(p => p.GetPathItemName()));
@@ -609,6 +610,12 @@ private static IEdmModel GetEdmModel(bool hasStream, string streamPropName)
609610
<EntityType Name=""catalog"" BaseType=""microsoft.graph.document"">
610611
<NavigationProperty Name=""reports"" Type = ""Collection(microsoft.graph.report)"" />
611612
</EntityType>
613+
<EntityType Name=""report"">
614+
<Key>
615+
<PropertyRef Name=""id"" />
616+
</Key>
617+
<Property Name=""id"" Type=""Edm.Int32"" Nullable=""false"" />
618+
</EntityType>
612619
<EntityContainer Name =""GraphService"">
613620
<EntitySet Name=""todos"" EntityType=""microsoft.graph.todo"" />
614621
<Singleton Name=""me"" Type=""microsoft.graph.user"" />

‎test/Microsoft.OpenAPI.OData.Reader.Tests/PathItem/RefPathItemHandlerTests.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public void CreatePathItemThrowsForNonNavigationPropertyPath()
5757
}
5858

5959
[Theory]
60+
[InlineData(true, new OperationType[] { OperationType.Delete})]
6061
[InlineData(true, new OperationType[] { OperationType.Get, OperationType.Post})]
6162
[InlineData(false, new OperationType[] { OperationType.Get, OperationType.Put, OperationType.Delete })]
6263
public void CreateNavigationPropertyRefPathItemReturnsCorrectPathItem(bool collectionNav, OperationType[] expected)
@@ -73,10 +74,23 @@ public void CreateNavigationPropertyRefPathItemReturnsCorrectPathItem(bool colle
7374
(collectionNav ? c.TargetMultiplicity() == EdmMultiplicity.Many : c.TargetMultiplicity() != EdmMultiplicity.Many));
7475
Assert.NotNull(property);
7576

76-
ODataPath path = new ODataPath(new ODataNavigationSourceSegment(entitySet),
77+
ODataPath path;
78+
if (collectionNav && expected.Contains(OperationType.Delete))
79+
{
80+
// DELETE ~/entityset/{key}/collection-valued-Nav/{key}/$ref
81+
path = new ODataPath(new ODataNavigationSourceSegment(entitySet),
7782
new ODataKeySegment(entityType),
7883
new ODataNavigationPropertySegment(property),
84+
new ODataKeySegment(property.ToEntityType()),
7985
ODataRefSegment.Instance);
86+
}
87+
else
88+
{
89+
path = new ODataPath(new ODataNavigationSourceSegment(entitySet),
90+
new ODataKeySegment(entityType),
91+
new ODataNavigationPropertySegment(property),
92+
ODataRefSegment.Instance);
93+
}
8094

8195
// Act
8296
var pathItem = _pathItemHandler.CreatePathItem(context, path);

0 commit comments

Comments
 (0)