Skip to content

Commit 1130c74

Browse files
gathogojrgathogojr
authored
Fix invalid partial json generated when serverside paging is applied in multi-level containment scenario (#2771)
1 parent 78b8383 commit 1130c74

12 files changed

Lines changed: 821 additions & 35 deletions

File tree

‎src/Microsoft.AspNet.OData.Shared/Builder/EdmModelHelperMethods.cs‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ public static IEdmModel BuildEdmModel(ODataModelBuilder builder)
5050
// Build the navigation source map
5151
IDictionary<string, EdmNavigationSource> navigationSourceMap = model.GetNavigationSourceMap(edmMap, navigationSources);
5252

53+
// Add navigation property link builders
54+
AddNavigationPropertyLinkBuilders(builder, edmMap, navigationSources);
55+
5356
// Add the core and validation vocabulary annotations
5457
model.AddCoreAndValidationVocabularyAnnotations(navigationSources, edmMap);
5558

@@ -133,30 +136,31 @@ private static IDictionary<string, EdmNavigationSource> GetNavigationSourceMap(t
133136
model.SetAnnotationValue(navigationSource, navigationSourceAndAnnotation.Url);
134137
model.SetNavigationSourceLinkBuilder(navigationSource, navigationSourceAndAnnotation.LinkBuilder);
135138

136-
AddNavigationBindings(edmMap, navigationSourceAndAnnotation.Configuration, navigationSource, navigationSourceAndAnnotation.LinkBuilder,
137-
edmNavigationSourceMap);
139+
AddNavigationBindings(edmMap, navigationSourceAndAnnotation.Configuration, navigationSource, edmNavigationSourceMap);
138140
}
139141

140142
return edmNavigationSourceMap;
141143
}
142144

143-
private static void AddNavigationBindings(EdmTypeMap edmMap,
145+
private static void AddNavigationBindings(
146+
EdmTypeMap edmMap,
144147
NavigationSourceConfiguration navigationSourceConfiguration,
145148
EdmNavigationSource navigationSource,
146-
NavigationSourceLinkBuilderAnnotation linkBuilder,
147149
Dictionary<string, EdmNavigationSource> edmNavigationSourceMap)
148150
{
149-
foreach (var binding in navigationSourceConfiguration.Bindings)
151+
foreach (NavigationPropertyBindingConfiguration binding in navigationSourceConfiguration.Bindings)
150152
{
151153
NavigationPropertyConfiguration navigationProperty = binding.NavigationProperty;
152154
bool isContained = navigationProperty.ContainsTarget;
153155

154156
IEdmType edmType = edmMap.EdmTypes[navigationProperty.DeclaringType.ClrType];
155-
IEdmStructuredType structuraType = edmType as IEdmStructuredType;
156-
IEdmNavigationProperty edmNavigationProperty = structuraType.NavigationProperties()
157+
IEdmStructuredType structuralType = edmType as IEdmStructuredType;
158+
IEdmNavigationProperty edmNavigationProperty = structuralType.NavigationProperties()
157159
.Single(np => np.Name == navigationProperty.Name);
158160

159161
string bindingPath = ConvertBindingPath(edmMap, binding);
162+
// This check for whether navigation property is contained is possibly redundant
163+
// Unless contained navigation properties can have navigation property bindings...
160164
if (!isContained)
161165
{
162166
// calculate the binding path
@@ -165,11 +169,42 @@ private static void AddNavigationBindings(EdmTypeMap edmMap,
165169
edmNavigationSourceMap[binding.TargetNavigationSource.Name],
166170
new EdmPathExpression(bindingPath));
167171
}
172+
}
173+
}
174+
175+
/// <summary>
176+
/// Adds navigation property link builders
177+
/// </summary>
178+
/// <param name="modelBuilder">The model builder.</param>
179+
/// <param name="edmMap">Edm type mappings.</param>
180+
/// <param name="navigationSourceAndAnnotations">The navigation source annotations.</param>
181+
private static void AddNavigationPropertyLinkBuilders(
182+
ODataModelBuilder modelBuilder,
183+
EdmTypeMap edmMap,
184+
IEnumerable<NavigationSourceAndAnnotations> navigationSourceAndAnnotations)
185+
{
186+
foreach (NavigationSourceAndAnnotations navigationSourceAndAnnotation in navigationSourceAndAnnotations)
187+
{
188+
NavigationSourceConfiguration navigationSourceConfiguration = navigationSourceAndAnnotation.Configuration;
189+
NavigationSourceLinkBuilderAnnotation linkBuilder = navigationSourceAndAnnotation.LinkBuilder;
190+
191+
IEnumerable<EntityTypeConfiguration> derivedEntityTypeConfigurations = modelBuilder.DerivedTypes(navigationSourceConfiguration.EntityType);
168192

169-
NavigationLinkBuilder linkBuilderFunc = navigationSourceConfiguration.GetNavigationPropertyLink(navigationProperty);
170-
if (linkBuilderFunc != null)
193+
foreach (EntityTypeConfiguration entityTypeConfiguration in new[] { navigationSourceConfiguration.EntityType }.Concat(derivedEntityTypeConfigurations))
171194
{
172-
linkBuilder.AddNavigationPropertyLinkBuilder(edmNavigationProperty, linkBuilderFunc);
195+
foreach (NavigationPropertyConfiguration navigationProperty in entityTypeConfiguration.NavigationProperties)
196+
{
197+
IEdmType edmType = edmMap.EdmTypes[navigationProperty.DeclaringType.ClrType];
198+
IEdmStructuredType structuralType = edmType as IEdmStructuredType;
199+
IEdmNavigationProperty edmNavigationProperty = structuralType.NavigationProperties()
200+
.First(np => np.Name == navigationProperty.Name);
201+
202+
NavigationLinkBuilder linkBuilderFunc = navigationSourceConfiguration.GetNavigationPropertyLink(navigationProperty);
203+
if (linkBuilderFunc != null)
204+
{
205+
linkBuilder.AddNavigationPropertyLinkBuilder(edmNavigationProperty, linkBuilderFunc);
206+
}
207+
}
173208
}
174209
}
175210
}

‎src/Microsoft.AspNet.OData.Shared/Builder/LinkGenerationHelpers.cs‎

Lines changed: 114 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System;
99
using System.Collections.Generic;
1010
using System.ComponentModel;
11+
using System.Diagnostics;
1112
using System.Diagnostics.Contracts;
1213
using System.Linq;
1314
using Microsoft.AspNet.OData.Builder.Conventions;
@@ -81,7 +82,16 @@ public static Uri GenerateNavigationPropertyLink(this ResourceContext resourceCo
8182
throw Error.Argument("resourceContext", SRResources.UrlHelperNull, typeof(ResourceContext).Name);
8283
}
8384

84-
IList<ODataPathSegment> navigationPathSegments = resourceContext.GenerateBaseODataPathSegments();
85+
IList<ODataPathSegment> navigationPathSegments;
86+
if (resourceContext.NavigationSource is IEdmContainedEntitySet &&
87+
resourceContext.NavigationSource != resourceContext.SerializerContext.Path.NavigationSource)
88+
{
89+
navigationPathSegments = resourceContext.GenerateContainmentODataPathSegments();
90+
}
91+
else
92+
{
93+
navigationPathSegments = resourceContext.GenerateBaseODataPathSegments();
94+
}
8595

8696
if (includeCast)
8797
{
@@ -514,8 +524,7 @@ private static void GenerateBaseODataPathSegments(
514524
// the case.
515525
odataPath.Clear();
516526

517-
IEdmContainedEntitySet containmnent = navigationSource as IEdmContainedEntitySet;
518-
if (containmnent != null)
527+
if (navigationSource is IEdmContainedEntitySet)
519528
{
520529
EdmEntityContainer container = new EdmEntityContainer("NS", "Default");
521530
IEdmEntitySet entitySet = new EdmEntitySet(container, navigationSource.Name,
@@ -550,5 +559,107 @@ private static void GenerateBaseODataPathSegmentsForFeed(
550559
feedContext.EntitySetBase,
551560
odataPath);
552561
}
562+
563+
private static IList<ODataPathSegment> GenerateContainmentODataPathSegments(this ResourceContext resourceContext)
564+
{
565+
List<ODataPathSegment> navigationPathSegments = new List<ODataPathSegment>();
566+
ResourceContext currentResourceContext = resourceContext;
567+
568+
// We loop till the base of the $expand expression then use GenerateBaseODataPathSegments to generate the base path segments
569+
// For instance, given $expand=Tabs($expand=Items($expand=Notes($expand=Tips))), we loop until we get to Tabs at the base
570+
while (currentResourceContext != null && currentResourceContext.NavigationSource != resourceContext.InternalRequest.Context.Path.NavigationSource)
571+
{
572+
if (currentResourceContext.NavigationSource is IEdmContainedEntitySet containedEntitySet)
573+
{
574+
// Type-cast segment for the expanded resource that is passed into the method is added by the caller
575+
if (currentResourceContext != resourceContext && currentResourceContext.StructuredType != containedEntitySet.EntityType())
576+
{
577+
navigationPathSegments.Add(new TypeSegment(currentResourceContext.StructuredType, currentResourceContext.NavigationSource));
578+
}
579+
580+
KeySegment keySegment = new KeySegment(
581+
ConventionsHelpers.GetEntityKey(currentResourceContext),
582+
currentResourceContext.StructuredType as IEdmEntityType,
583+
navigationSource: currentResourceContext.NavigationSource);
584+
navigationPathSegments.Add(keySegment);
585+
586+
NavigationPropertySegment navPropertySegment = new NavigationPropertySegment(
587+
containedEntitySet.NavigationProperty,
588+
containedEntitySet.ParentNavigationSource);
589+
navigationPathSegments.Add(navPropertySegment);
590+
}
591+
else if (currentResourceContext.NavigationSource is IEdmEntitySet entitySet)
592+
{
593+
// We will get here if there's a non-contained entity set on the $expand expression
594+
if (currentResourceContext.StructuredType != entitySet.EntityType())
595+
{
596+
navigationPathSegments.Add(new TypeSegment(currentResourceContext.StructuredType, currentResourceContext.NavigationSource));
597+
}
598+
599+
KeySegment keySegment = new KeySegment(
600+
ConventionsHelpers.GetEntityKey(currentResourceContext),
601+
currentResourceContext.StructuredType as IEdmEntityType,
602+
currentResourceContext.NavigationSource);
603+
navigationPathSegments.Add(keySegment);
604+
605+
EntitySetSegment entitySetSegment = new EntitySetSegment(entitySet);
606+
navigationPathSegments.Add(entitySetSegment);
607+
608+
// Reverse the list such that the segments are in the right order
609+
navigationPathSegments.Reverse();
610+
return navigationPathSegments;
611+
}
612+
else if (currentResourceContext.NavigationSource is IEdmSingleton singleton)
613+
{
614+
// We will get here if there's a singleton on the $expand expression
615+
if (currentResourceContext.StructuredType != singleton.EntityType())
616+
{
617+
navigationPathSegments.Add(new TypeSegment(currentResourceContext.StructuredType, currentResourceContext.NavigationSource));
618+
}
619+
620+
SingletonSegment singletonSegment = new SingletonSegment(singleton);
621+
navigationPathSegments.Add(singletonSegment);
622+
623+
// Reverse the list such that the segments are in the right order
624+
navigationPathSegments.Reverse();
625+
return navigationPathSegments;
626+
}
627+
628+
currentResourceContext = currentResourceContext.SerializerContext.ExpandedResource;
629+
}
630+
631+
Debug.Assert(currentResourceContext != null, "currentResourceContext != null");
632+
// Once we are at the base of the $expand expression, we call GenerateBaseODataPathSegments to generate the base path segments
633+
IList<ODataPathSegment> pathSegments = currentResourceContext.GenerateBaseODataPathSegments();
634+
635+
Debug.Assert(pathSegments.Count > 0, "pathSegments.Count > 0");
636+
637+
ODataPathSegment lastNonKeySegment;
638+
639+
if (pathSegments.Count == 1)
640+
{
641+
lastNonKeySegment = pathSegments[0];
642+
Debug.Assert(lastNonKeySegment is SingletonSegment, "lastNonKeySegment is SingletonSegment");
643+
}
644+
else
645+
{
646+
Debug.Assert(pathSegments[pathSegments.Count - 1] is KeySegment, "pathSegments[pathSegments.Count - 1] is KeySegment");
647+
// 2nd last segment would be NavigationPathSegment or EntitySetSegment
648+
lastNonKeySegment = pathSegments[pathSegments.Count - 2];
649+
}
650+
651+
if (currentResourceContext.StructuredType != lastNonKeySegment.EdmType.AsElementType())
652+
{
653+
pathSegments.Add(new TypeSegment(currentResourceContext.StructuredType, currentResourceContext.NavigationSource));
654+
}
655+
656+
// Add the segments from the $expand expression in reverse order
657+
for (int i = navigationPathSegments.Count - 1; i >= 0; i--)
658+
{
659+
pathSegments.Add(navigationPathSegments[i]);
660+
}
661+
662+
return pathSegments;
663+
}
553664
}
554665
}

‎src/Microsoft.AspNet.OData.Shared/Builder/NavigationSourceAndAnnotations.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
namespace Microsoft.AspNet.OData.Builder
1111
{
1212
/// <summary>
13-
/// This class is used in internal as a helper class to build the Edm model.
14-
/// This class wrappers a relationship between Edm type and the CLR type configuration.
13+
/// This class is used internally as a helper class to build the Edm model.
14+
/// This class wraps a relationship between Edm type and the CLR type configuration.
1515
/// This relationship is used to builder the navigation property and the corresponding links.
1616
/// </summary>
1717
internal class NavigationSourceAndAnnotations

‎src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataResourceSetSerializer.cs‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -481,26 +481,12 @@ private IEnumerable<ODataOperation> CreateODataOperations(IEnumerable<IEdmOperat
481481
private static Uri GetNestedNextPageLink(ODataSerializerContext writeContext, int pageSize, object obj)
482482
{
483483
Contract.Assert(writeContext.ExpandedResource != null);
484-
Uri navigationLink = null;
485484
IEdmNavigationSource sourceNavigationSource = writeContext.ExpandedResource.NavigationSource;
486485
NavigationSourceLinkBuilderAnnotation linkBuilder = writeContext.Model.GetNavigationSourceLinkBuilder(sourceNavigationSource);
487486

488-
// In Contained Navigation, we don't have navigation property binding,
489-
// Hence we cannot get the NavigationLink from the NavigationLinkBuilder
490-
if (writeContext.NavigationSource.NavigationSourceKind() == EdmNavigationSourceKind.ContainedEntitySet)
491-
{
492-
// Contained navigation.
493-
Uri idlink = linkBuilder.BuildIdLink(writeContext.ExpandedResource);
494-
495-
var link = idlink.ToString() + "/" + writeContext.NavigationProperty.Name;
496-
navigationLink = new Uri(link);
497-
}
498-
else
499-
{
500-
// Non-Contained navigation.
501-
navigationLink =
502-
linkBuilder.BuildNavigationLink(writeContext.ExpandedResource, writeContext.NavigationProperty);
503-
}
487+
Uri navigationLink = linkBuilder.BuildNavigationLink(
488+
writeContext.ExpandedResource,
489+
writeContext.NavigationProperty);
504490

505491
Uri nestedNextLink = GenerateQueryFromExpandedItem(writeContext, navigationLink);
506492

‎test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNet/Microsoft.Test.E2E.AspNet.OData.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1705,6 +1705,9 @@
17051705
<Compile Include="..\ServerSidePaging\ServerSidePagingDataModel.cs">
17061706
<Link>ServerSidePaging\ServerSidePagingDataModel.cs</Link>
17071707
</Compile>
1708+
<Compile Include="..\ServerSidePaging\ServerSidePagingDataSource.cs">
1709+
<Link>ServerSidePaging\ServerSidePagingDataSource.cs</Link>
1710+
</Compile>
17081711
<Compile Include="..\ServerSidePaging\ServerSidePagingTests.cs">
17091712
<Link>ServerSidePaging\ServerSidePagingTests.cs</Link>
17101713
</Compile>

‎test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore/Microsoft.Test.E2E.AspNetCore.OData.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,6 +1561,9 @@
15611561
<Compile Include="..\ServerSidePaging\ServerSidePagingDataModel.cs">
15621562
<Link>ServerSidePaging\ServerSidePagingDataModel.cs</Link>
15631563
</Compile>
1564+
<Compile Include="..\ServerSidePaging\ServerSidePagingDataSource.cs">
1565+
<Link>ServerSidePaging\ServerSidePagingDataSource.cs</Link>
1566+
</Compile>
15641567
<Compile Include="..\ServerSidePaging\ServerSidePagingTests.cs">
15651568
<Link>ServerSidePaging\ServerSidePagingTests.cs</Link>
15661569
</Compile>

‎test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,9 @@
15591559
<Compile Include="..\ServerSidePaging\ServerSidePagingDataModel.cs">
15601560
<Link>ServerSidePaging\ServerSidePagingDataModel.cs</Link>
15611561
</Compile>
1562+
<Compile Include="..\ServerSidePaging\ServerSidePagingDataSource.cs">
1563+
<Link>ServerSidePaging\ServerSidePagingDataSource.cs</Link>
1564+
</Compile>
15621565
<Compile Include="..\ServerSidePaging\ServerSidePagingTests.cs">
15631566
<Link>ServerSidePaging\ServerSidePagingTests.cs</Link>
15641567
</Compile>

‎test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Containment/ContainmentEdmModels.cs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
// </copyright>
66
//------------------------------------------------------------------------------
77

8+
using System;
9+
using Microsoft.AspNet.OData;
810
using Microsoft.AspNet.OData.Builder;
911
using Microsoft.OData.Edm;
1012
using Microsoft.Test.E2E.AspNet.OData.Common.Execution;
@@ -42,15 +44,29 @@ public static IEdmModel GetExplicitModel()
4244
statementType.Property(s => s.TransactionDescription);
4345
statementType.Property(s => s.Amount);
4446

47+
Func<ResourceContext<Account>, IEdmNavigationProperty, Uri> navigationPropertyLinkFactory = (
48+
resourceContext, navigationProperty) => resourceContext.GenerateNavigationPropertyLink(navigationProperty, false);
49+
Func<ResourceContext<Account>, IEdmNavigationProperty, Uri> navigationPropertyLinkFactoryWithCast = (
50+
resourceContext, navigationProperty) => resourceContext.GenerateNavigationPropertyLink(navigationProperty, true);
51+
4552
var accounts = builder.EntitySet<Account>("Accounts");
4653
accounts.HasIdLink(c => c.GenerateSelfLink(false), true);
4754
accounts.HasEditLink(c => c.GenerateSelfLink(true), true);
55+
accounts.HasNavigationPropertyLink(payoutPI, navigationPropertyLinkFactory, followsConventions: true);
56+
accounts.HasNavigationPropertyLink(payinPIs, navigationPropertyLinkFactory, followsConventions: true);
57+
accounts.HasNavigationPropertyLink(giftCard, navigationPropertyLinkFactoryWithCast, true);
4858

4959
var paginatedAccounts = builder.EntitySet<Account>("PaginatedAccounts");
5060
paginatedAccounts.HasIdLink(c => c.GenerateSelfLink(true), true);
5161
paginatedAccounts.HasEditLink(c => c.GenerateSelfLink(true), true);
52-
53-
builder.Singleton<Account>("AnonymousAccount");
62+
paginatedAccounts.HasNavigationPropertyLink(payoutPI, navigationPropertyLinkFactory, followsConventions: true);
63+
paginatedAccounts.HasNavigationPropertyLink(payinPIs, navigationPropertyLinkFactory, followsConventions: true);
64+
paginatedAccounts.HasNavigationPropertyLink(giftCard, navigationPropertyLinkFactoryWithCast, true);
65+
66+
var accountSingleton = builder.Singleton<Account>("AnonymousAccount");
67+
accountSingleton.HasNavigationPropertyLink(payoutPI, navigationPropertyLinkFactory, followsConventions: true);
68+
accountSingleton.HasNavigationPropertyLink(payinPIs, navigationPropertyLinkFactory, followsConventions: true);
69+
accountSingleton.HasNavigationPropertyLink(giftCard, navigationPropertyLinkFactoryWithCast, true);
5470

5571
AddBoundActionsAndFunctions(builder);
5672

0 commit comments

Comments
 (0)