Generate valid id/read/edit links for expanded non-contained resource with a contained navigation source

src/Microsoft.AspNetCore.OData/Formatter/Serialization/ODataResourceSerializer.cs

@@ -581,7 +581,18 @@ public virtual ODataResource CreateResource(SelectExpandNode selectExpandNode, R
 
             if (resourceContext.StructuredType.TypeKind == EdmTypeKind.Entity && resourceContext.NavigationSource != null)
             {
-                if (!(resourceContext.NavigationSource is IEdmContainedEntitySet))
+                // Condition 1. If resourceContext.NavigationSource is a contained entity set
+                //    and a contained resource is being written, the id/read/edit links can be derived
+                //    from the entity set or parent resource, i.e., no need to use link builder to build the links.
+                // Condition 2. If resourceContext.NavigationSource is a contained entity set
+                //    but an expanded non-contained resource is being written,
+                //    deriving the id/read/edit links from the entity set or parent resource will
+                //    most likely result into invalid links.
+                //    A navigation property binding should exist and we should try
+                //    to use the navigation link builder to build the links.
+                // NOTE: resourceContext.SerializerContext.NavigationProperty will not be null when writing an expanded resource
+                if (!(resourceContext.NavigationSource is IEdmContainedEntitySet)
+                    || resourceContext.SerializerContext.NavigationProperty?.ContainsTarget == false)
                 {
                     IEdmModel model = resourceContext.SerializerContext.Model;
                     NavigationSourceLinkBuilderAnnotation linkBuilder = EdmModelLinkBuilderExtensions.GetNavigationSourceLinkBuilder(model, resourceContext.NavigationSource);

test/Microsoft.AspNetCore.OData.E2E.Tests/MetadataProperties/MetadataPropertiesController.cs

@@ -0,0 +1,98 @@
+//-----------------------------------------------------------------------------
+// <copyright file="MetadataPropertiesController.cs" company=".NET Foundation">
+//      Copyright (c) .NET Foundation and Contributors. All rights reserved.
+//      See License.txt in the project root for license information.
+// </copyright>
+//------------------------------------------------------------------------------
+
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Core;
+using Microsoft.AspNetCore.OData.Query;
+using Microsoft.AspNetCore.OData.Routing.Controllers;
+
+namespace Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties
+{
+    [Route("NonContainedNavPropInContainedNavSource")]
+    [Route("ContainedNavPropInContainedNavSource")]
+    public class SitesController : ODataController
+    {
+        [EnableQuery]
+        [HttpGet("Sites")]
+        public ActionResult<IEnumerable<Site>> Get()
+        {
+            return MetadataPropertiesDataSource.Sites;
+        }
+
+        [EnableQuery]
+        [HttpGet("Sites({key})")]
+        public ActionResult<Site> Get(int key)
+        {
+            var site = MetadataPropertiesDataSource.Sites.SingleOrDefault(d => d.Id == key);
+
+            if (site == null)
+            {
+                return NotFound();
+            }
+
+            return site;
+        }
+
+        [EnableQuery]
+        [HttpGet("Sites({key})/Plants")]
+        public ActionResult<IEnumerable<Plant>> GetPlants(int key)
+        {
+            var site = MetadataPropertiesDataSource.Sites.SingleOrDefault(d => d.Id == key);
+
+            if (site == null || site.Plants == null)
+            {
+                return Enumerable.Empty<Plant>().ToList();
+            }
+
+            return site.Plants.ToList();
+        }
+
+        [EnableQuery]
+        [HttpGet("Sites({siteKey})/Plants({plantKey})")]
+        public ActionResult<Plant> GetPlant(int siteKey, int plantKey)
+        {
+            var plant = MetadataPropertiesDataSource.Sites.SingleOrDefault(d => d.Id == siteKey)?.Plants?.SingleOrDefault(d => d.Id == plantKey);
+
+            if (plant == null)
+            {
+                return NotFound();
+            }
+
+            return plant;
+        }
+
+        [EnableQuery]
+        [HttpGet("Sites({siteKey})/Plants({plantKey})/Pipelines")]
+        public ActionResult<IEnumerable<PipelineBase>> GetPipelines(int siteKey, int plantKey)
+        {
+            var plant = MetadataPropertiesDataSource.Sites.SingleOrDefault(d => d.Id == siteKey)?.Plants?.SingleOrDefault(d => d.Id == plantKey);
+
+            if (plant == null || plant.Pipelines == null)
+            {
+                return Enumerable.Empty<PipelineBase>().ToList();
+            }
+
+            return plant.Pipelines.ToList();
+        }
+
+        [EnableQuery]
+        [HttpGet("Sites({siteKey})/Plants({plantKey})/Pipelines({pipelineKey})")]
+        public ActionResult<PipelineBase> GetPlantPipeline(int siteKey, int plantKey, int pipelineKey)
+        {
+            var pipeline = MetadataPropertiesDataSource.Sites.SingleOrDefault(d => d.Id == siteKey)?.Plants?.SingleOrDefault(d => d.Id == plantKey)?.Pipelines?.SingleOrDefault(d => d.Id == pipelineKey);
+
+            if (pipeline == null)
+            {
+                return NotFound();
+            }
+
+            return pipeline;
+        }
+    }
+}

test/Microsoft.AspNetCore.OData.E2E.Tests/MetadataProperties/MetadataPropertiesDataModel.cs

@@ -0,0 +1,50 @@
+//-----------------------------------------------------------------------------
+// <copyright file="MetadataPropertiesDataModel.cs" company=".NET Foundation">
+//      Copyright (c) .NET Foundation and Contributors. All rights reserved.
+//      See License.txt in the project root for license information.
+// </copyright>
+//------------------------------------------------------------------------------
+
+namespace Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Core
+{
+    using System.Collections.Generic;
+    using System.ComponentModel.DataAnnotations;
+    using Microsoft.OData.ModelBuilder;
+
+    public abstract class EntityBase
+    {
+        [Key]
+        public int Id { get; set; }
+        public string Name { get; set; }
+        public Dictionary<string, object> Attributes { get; set; }
+    }
+
+    public class Site : EntityBase
+    {
+        [Contained]
+        public IEnumerable<Plant> Plants { get; set; }
+    }
+
+    public class Plant : EntityBase
+    {
+        public Site Site { get; set; }
+        [Contained]
+        public IEnumerable<PipelineBase> Pipelines { get; set; }
+    }
+
+    public abstract class PipelineBase : EntityBase
+    {
+        public Plant Plant { get; set; }
+    }
+}
+
+// NOTE: Pipeline class defined in a different namespace to repro a reported scenario
+namespace Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Plant1
+{
+    using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Core;
+
+    public class Pipeline : PipelineBase
+    {
+        public int? Length { get; set; }
+    }
+}

test/Microsoft.AspNetCore.OData.E2E.Tests/MetadataProperties/MetadataPropertiesDataSource.cs

@@ -0,0 +1,57 @@
+//-----------------------------------------------------------------------------
+// <copyright file="MetadataPropertiesDataSource.cs" company=".NET Foundation">
+//      Copyright (c) .NET Foundation and Contributors. All rights reserved.
+//      See License.txt in the project root for license information.
+// </copyright>
+//------------------------------------------------------------------------------
+
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Core;
+using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Plant1;
+
+namespace Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties
+{
+    internal static class MetadataPropertiesDataSource
+    {
+        private readonly static List<Site> sites;
+        private readonly static List<Plant> plants;
+        private readonly static List<PipelineBase> pipelines;
+
+        static MetadataPropertiesDataSource()
+        {
+            pipelines = new List<PipelineBase>(Enumerable.Range(1, 8).Select(idx => new Pipeline
+            {
+                Id = idx,
+                Name = $"Pipeline {idx}",
+                Length = idx * 100
+            }));
+
+            plants = new List<Plant>(Enumerable.Range(1, 4).Select(idx => new Plant
+            {
+                Id = idx,
+                Name = $"Plant {idx}",
+                Pipelines = pipelines.Skip((idx - 1) * 2).Take(2)
+            }));
+
+            sites = new List<Site>(Enumerable.Range(1, 2).Select(idx => new Site
+            {
+                Id = idx,
+                Name = $"Site {idx}",
+                Plants = plants.Skip((idx - 1) * 2).Take(2)
+            }));
+
+            for (var i = 0; i < plants.Count; i++)
+            {
+                plants[i].Site = sites[i / 2];
+            }
+
+            for (var i = 0; i < pipelines.Count; i++)
+            {
+                pipelines[i].Plant = plants[i / 2];
+            }
+        }
+
+        public static List<Site> Sites => sites;
+    }
+}

test/Microsoft.AspNetCore.OData.E2E.Tests/MetadataProperties/MetadataPropertiesEdmModel.cs

@@ -0,0 +1,68 @@
+//-----------------------------------------------------------------------------
+// <copyright file="MetadataPropertiesEdmModel.cs" company=".NET Foundation">
+//      Copyright (c) .NET Foundation and Contributors. All rights reserved.
+//      See License.txt in the project root for license information.
+// </copyright>
+//------------------------------------------------------------------------------
+
+using System.Linq;
+using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Core;
+using Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties.Plant1;
+using Microsoft.OData.Edm;
+using Microsoft.OData.ModelBuilder;
+
+namespace Microsoft.AspNetCore.OData.E2E.Tests.MetadataProperties
+{
+    public class MetadataPropertiesEdmModel
+    {
+        /// <summary>
+        /// Returns model where Site and Plant navigation properties are non-contained and navigation source is contained.
+        /// </summary>
+        /// <returns>Returns Edm model.</returns>
+        public static IEdmModel GetEdmModelWithNonContainedNavPropInContainedNavSource()
+        {
+            var modelBuilder = new ODataConventionModelBuilder();
+            modelBuilder.EntityType<EntityBase>();
+            modelBuilder.EntityType<Site>();
+            modelBuilder.EntityType<Plant>();
+            modelBuilder.EntityType<PipelineBase>();
+            modelBuilder.EntityType<Pipeline>();
+            modelBuilder.EntitySet<Site>("Sites");
+
+            var model = modelBuilder.GetEdmModel();
+
+            var sitesEntitySet = (EdmEntitySet)model.FindDeclaredEntitySet("Default.Container.Sites");
+            var plantsNavigationProperty = sitesEntitySet.EntityType().DeclaredNavigationProperties().Single(d => d.Name.Equals("Plants"));
+            var plantsContainedEntitySet = sitesEntitySet.FindNavigationTarget(plantsNavigationProperty);
+            var siteNavigationProperty = plantsContainedEntitySet.EntityType().DeclaredNavigationProperties().Single(d => d.Name.Equals("Site"));
+            var pipelinesNavigationProperty = plantsContainedEntitySet.EntityType().DeclaredNavigationProperties().Single(d => d.Name.Equals("Pipelines"));
+            var pipelineContainedEntitySet = plantsContainedEntitySet.FindNavigationTarget(pipelinesNavigationProperty, new EdmPathExpression("Plants", "Pipelines"));
+            var plantNavigationProperty = pipelineContainedEntitySet.EntityType().DeclaredNavigationProperties().Single(d => d.Name.Equals("Plant"));
+
+            sitesEntitySet.AddNavigationTarget(siteNavigationProperty, sitesEntitySet, new EdmPathExpression("Plants", "Site"));
+            sitesEntitySet.AddNavigationTarget(plantNavigationProperty, plantsContainedEntitySet, new EdmPathExpression("Plants", "Pipelines", "Plant"));
+
+            return model;
+        }
+
+        /// <summary>
+        /// Returns model where Site and Plant navigation properties are contained and navigation source is contained.
+        /// </summary>
+        /// <returns>Returns Edm model.</returns>
+        public static IEdmModel GetEdmModelWithContainedNavPropInContainedNavSource()
+        {
+            var modelBuilder = new ODataConventionModelBuilder();
+            modelBuilder.EntityType<EntityBase>();
+            modelBuilder.EntityType<Site>();
+            // Make Site and Plant navigation properties contained
+            modelBuilder.EntityType<Plant>().ContainsRequired(d => d.Site).Contained();
+            modelBuilder.EntityType<PipelineBase>().ContainsRequired(d => d.Plant).Contained();
+            modelBuilder.EntityType<Pipeline>();
+            modelBuilder.EntitySet<Site>("Sites");
+
+            var model = modelBuilder.GetEdmModel();
+
+            return model;
+        }
+    }
+}