From 55a8e3e916e5edc817d121b9acd1e35afb08a0be Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Mon, 17 Aug 2026 11:28:38 -0700 Subject: [PATCH 1/3] Fix contained navigation paths when keys are missing Recover contained writer scope paths from explicit resource IDs when key properties are unavailable. Add scenario coverage for relative, absolute, nested collection, and invalid IDs. --- src/Microsoft.OData.Core/ODataWriterCore.cs | 36 ++- .../ContainedNavigationPathReproTests.cs | 213 ++++++++++++++++++ 2 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs diff --git a/src/Microsoft.OData.Core/ODataWriterCore.cs b/src/Microsoft.OData.Core/ODataWriterCore.cs index b84c31339a..9df9c4255e 100644 --- a/src/Microsoft.OData.Core/ODataWriterCore.cs +++ b/src/Microsoft.OData.Core/ODataWriterCore.cs @@ -2787,7 +2787,11 @@ private void EnterScope(WriterState newState, ODataItem item) // Containment cannot be written alone without odata uri. if (!odataPath.Any()) { - throw new ODataException(SRResources.ODataWriterCore_PathInODataUriMustBeSetWhenWritingContainedElement); + this.TryBuildPathFromResourceId(out odataPath); + if (odataPath == null || !odataPath.Any()) + { + throw new ODataException(SRResources.ODataWriterCore_PathInODataUriMustBeSetWhenWritingContainedElement); + } } ODataPath newPath = null; @@ -2808,7 +2812,8 @@ private void EnterScope(WriterState newState, ODataItem item) } else { - newPath = null; + ODataPath resourcePath; + newPath = this.TryBuildPathFromResourceId(out resourcePath) ? resourcePath : null; } } @@ -2903,6 +2908,33 @@ private bool TryBuildKeySegment(out KeySegment keySegment) return true; } + private bool TryBuildPathFromResourceId(out ODataPath resourcePath) + { + ODataResourceBase resource = this.CurrentScope.Item as ODataResourceBase; + Uri resourceId = resource != null && resource.HasNonComputedId ? resource.NonComputedId : null; + Uri serviceRoot = this.CurrentScope.ODataUri.ServiceRoot; + if (resourceId == null || serviceRoot == null) + { + resourcePath = null; + return false; + } + + try + { + resourcePath = new ODataUriParser( + this.outputContext.Model, + serviceRoot, + resourceId, + this.outputContext.Container).ParsePath(); + return resourcePath.Any(); + } + catch (ODataException) + { + resourcePath = null; + return false; + } + } + private IList> GetKeyProperties(IEdmEntityType currentEntityType, bool throwIfFail) { ODataResourceBase resource = this.CurrentScope.Item as ODataResourceBase; diff --git a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs new file mode 100644 index 0000000000..477b977487 --- /dev/null +++ b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs @@ -0,0 +1,213 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +using System; +using System.IO; +using System.Text; +using Microsoft.OData.Edm; +using Microsoft.OData.UriParser; +using Xunit; + +namespace Microsoft.OData.Tests.ScenarioTests.Writer +{ + public class ContainedNavigationPathReproTests + { + [Theory] + [InlineData(true, false)] + [InlineData(true, true)] + [InlineData(false, false)] + [InlineData(false, true)] + public void WriteNestedContainedResourcesUsesExplicitIdsWhenKeysAreMissing(bool parentHasId, bool useAbsoluteIds) + { + Uri serviceRoot = new Uri("https://service.test/"); + Uri driveItemId = parentHasId ? CreateResourceId(serviceRoot, "driveItems('parent')", useAbsoluteIds) : null; + Uri listItemId = CreateResourceId(serviceRoot, "driveItems('parent')/listItem", useAbsoluteIds); + Uri fieldsId = CreateResourceId(serviceRoot, "driveItems('parent')/listItem/fields", useAbsoluteIds); + + string payload = WriteSingleContainedResources( + serviceRoot, + driveItemId, + listItemId, + fieldsId, + setCustomLinks: !parentHasId); + + Assert.Contains($"\"@odata.id\":\"{fieldsId.OriginalString}\"", payload); + } + + [Fact] + public void WriteNestedContainedCollectionUsesEachResourcesExplicitIdWhenKeysAreMissing() + { + Uri serviceRoot = new Uri("https://service.test/"); + EdmModel model = CreateModel( + listItemNavigationName: "items", + listItemMultiplicity: EdmMultiplicity.Many, + out EdmEntityType driveItemType, + out EdmEntitySet driveItems); + Uri requestUri = new Uri(serviceRoot, "driveItems('parent')"); + ODataMessageWriterSettings settings = CreateSettings(model, serviceRoot, requestUri); + + ODataResource driveItem = new ODataResource + { + Id = new Uri("driveItems('parent')", UriKind.Relative) + }; + ODataResource listItem = new ODataResource + { + Id = new Uri("driveItems('parent')/items('child')", UriKind.Relative) + }; + ODataResource fields = new ODataResource + { + Id = new Uri("driveItems('parent')/items('child')/fields", UriKind.Relative) + }; + + using (MemoryStream stream = new MemoryStream()) + { + InMemoryMessage message = new InMemoryMessage { Stream = stream }; + using (ODataMessageWriter messageWriter = new ODataMessageWriter((IODataResponseMessage)message, settings, model)) + { + ODataWriter resourceWriter = messageWriter.CreateODataResourceWriter(driveItems, driveItemType); + resourceWriter.WriteStart(driveItem); + resourceWriter.WriteStart(new ODataNestedResourceInfo { Name = "items", IsCollection = true }); + resourceWriter.WriteStart(new ODataResourceSet()); + resourceWriter.WriteStart(listItem); + resourceWriter.WriteStart(new ODataNestedResourceInfo { Name = "fields", IsCollection = false }); + resourceWriter.WriteStart(fields); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.Flush(); + } + + string payload = Encoding.UTF8.GetString(stream.ToArray()); + Assert.Contains("\"@odata.id\":\"driveItems('parent')/items('child')/fields\"", payload); + } + } + + [Fact] + public void InvalidExplicitIdsPreserveMissingContainedPathException() + { + Uri serviceRoot = new Uri("https://service.test/"); + ODataException exception = Assert.Throws(() => WriteSingleContainedResources( + serviceRoot, + new Uri("unknown('parent')", UriKind.Relative), + new Uri("unknown('parent')/listItem", UriKind.Relative), + new Uri("unknown('parent')/listItem/fields", UriKind.Relative), + setCustomLinks: true)); + + Assert.Equal( + "The Path property in ODataMessageWriterSettings.ODataUri must be set when writing contained elements.", + exception.Message); + } + + private static string WriteSingleContainedResources( + Uri serviceRoot, + Uri driveItemId, + Uri listItemId, + Uri fieldsId, + bool setCustomLinks) + { + EdmModel model = CreateModel( + listItemNavigationName: "listItem", + listItemMultiplicity: EdmMultiplicity.ZeroOrOne, + out EdmEntityType driveItemType, + out EdmEntitySet driveItems); + Uri requestUri = new Uri(serviceRoot, "driveItems('parent')"); + ODataMessageWriterSettings settings = CreateSettings(model, serviceRoot, requestUri); + + ODataNestedResourceInfo listItemInfo = new ODataNestedResourceInfo + { + Name = "listItem", + IsCollection = false + }; + if (setCustomLinks) + { + listItemInfo.Url = listItemId; + listItemInfo.AssociationLinkUrl = new Uri($"{listItemId.OriginalString}/$ref", listItemId.IsAbsoluteUri ? UriKind.Absolute : UriKind.Relative); + } + + using (MemoryStream stream = new MemoryStream()) + { + InMemoryMessage message = new InMemoryMessage { Stream = stream }; + using (ODataMessageWriter messageWriter = new ODataMessageWriter((IODataResponseMessage)message, settings, model)) + { + ODataWriter resourceWriter = messageWriter.CreateODataResourceWriter(driveItems, driveItemType); + resourceWriter.WriteStart(new ODataResource { Id = driveItemId }); + resourceWriter.WriteStart(listItemInfo); + resourceWriter.WriteStart(new ODataResource { Id = listItemId }); + resourceWriter.WriteStart(new ODataNestedResourceInfo { Name = "fields", IsCollection = false }); + resourceWriter.WriteStart(new ODataResource { Id = fieldsId }); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.Flush(); + } + + return Encoding.UTF8.GetString(stream.ToArray()); + } + } + + private static EdmModel CreateModel( + string listItemNavigationName, + EdmMultiplicity listItemMultiplicity, + out EdmEntityType driveItemType, + out EdmEntitySet driveItems) + { + EdmModel model = new EdmModel(); + driveItemType = model.AddEntityType("Microsoft.Graph", "DriveItem"); + driveItemType.AddKeys(driveItemType.AddStructuralProperty("id", EdmPrimitiveTypeKind.String)); + EdmEntityType listItemType = model.AddEntityType("Microsoft.Graph", "ListItem"); + listItemType.AddKeys(listItemType.AddStructuralProperty("id", EdmPrimitiveTypeKind.String)); + EdmEntityType fieldsType = model.AddEntityType("Microsoft.Graph", "FieldValueSet"); + fieldsType.AddKeys(fieldsType.AddStructuralProperty("id", EdmPrimitiveTypeKind.String)); + + driveItemType.AddUnidirectionalNavigation( + new EdmNavigationPropertyInfo + { + Name = listItemNavigationName, + Target = listItemType, + TargetMultiplicity = listItemMultiplicity, + ContainsTarget = true + }); + listItemType.AddUnidirectionalNavigation( + new EdmNavigationPropertyInfo + { + Name = "fields", + Target = fieldsType, + TargetMultiplicity = EdmMultiplicity.ZeroOrOne, + ContainsTarget = true + }); + + EdmEntityContainer container = model.AddEntityContainer("Microsoft.Graph", "Container"); + driveItems = container.AddEntitySet("driveItems", driveItemType); + return model; + } + + private static ODataMessageWriterSettings CreateSettings(EdmModel model, Uri serviceRoot, Uri requestUri) + { + ODataMessageWriterSettings settings = new ODataMessageWriterSettings + { + ODataUri = new ODataUri + { + RequestUri = requestUri, + ServiceRoot = serviceRoot, + Path = new ODataUriParser(model, serviceRoot, requestUri).ParsePath() + }, + EnableMessageStreamDisposal = false + }; + settings.SetContentType("application/json;odata.metadata=full", null); + return settings; + } + + private static Uri CreateResourceId(Uri serviceRoot, string path, bool absolute) + { + return absolute ? new Uri(serviceRoot, path) : new Uri(path, UriKind.Relative); + } + } +} From f711c493b8d574bd0a60f157922b3cb782392731 Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Wed, 19 Aug 2026 16:10:08 -0700 Subject: [PATCH 2/3] Address contained path review feedback --- src/Microsoft.OData.Core/ODataWriterCore.cs | 8 +- .../ContainedNavigationPathReproTests.cs | 73 ++++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OData.Core/ODataWriterCore.cs b/src/Microsoft.OData.Core/ODataWriterCore.cs index 9df9c4255e..33b6489cb7 100644 --- a/src/Microsoft.OData.Core/ODataWriterCore.cs +++ b/src/Microsoft.OData.Core/ODataWriterCore.cs @@ -2785,9 +2785,10 @@ private void EnterScope(WriterState newState, ODataItem item) { case EdmNavigationSourceKind.ContainedEntitySet: // Containment cannot be written alone without odata uri. + bool pathBuiltFromResourceId = false; if (!odataPath.Any()) { - this.TryBuildPathFromResourceId(out odataPath); + pathBuiltFromResourceId = this.TryBuildPathFromResourceId(out odataPath); if (odataPath == null || !odataPath.Any()) { throw new ODataException(SRResources.ODataWriterCore_PathInODataUriMustBeSetWhenWritingContainedElement); @@ -2800,6 +2801,11 @@ private void EnterScope(WriterState newState, ODataItem item) // if there's no key, for example in a complex property, just use the existing odata path. newPath = odataPath; } + else if (pathBuiltFromResourceId) + { + // The resource ID represents the complete resource path and already includes its key. + newPath = odataPath; + } else { // If there's only non-key properties selected in the request URI, then We don't have the key values to calcuate the key segment. diff --git a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs index 477b977487..bfe645dc73 100644 --- a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs +++ b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs @@ -6,7 +6,10 @@ using System; using System.IO; +using System.Linq; +using System.Reflection; using System.Text; +using Microsoft.OData.Core; using Microsoft.OData.Edm; using Microsoft.OData.UriParser; using Xunit; @@ -88,6 +91,65 @@ public void WriteNestedContainedCollectionUsesEachResourcesExplicitIdWhenKeysAre } } + [Fact] + public void WriteNestedContainedCollectionDoesNotDuplicateKeyInRecoveredResourcePath() + { + Uri serviceRoot = new Uri("https://service.test/"); + EdmModel model = CreateModel( + listItemNavigationName: "items", + listItemMultiplicity: EdmMultiplicity.Many, + out EdmEntityType driveItemType, + out EdmEntitySet driveItems); + Uri requestUri = new Uri(serviceRoot, "driveItems('parent')"); + ODataMessageWriterSettings settings = CreateSettings(model, serviceRoot, requestUri); + settings.SetContentType("application/json;odata.metadata=minimal", null); + Uri listItemId = new Uri("driveItems('parent')/items('child')", UriKind.Relative); + int keySegmentCount; + + using (MemoryStream stream = new MemoryStream()) + { + InMemoryMessage message = new InMemoryMessage { Stream = stream }; + using (ODataMessageWriter messageWriter = new ODataMessageWriter((IODataResponseMessage)message, settings, model)) + { + ODataWriter resourceWriter = messageWriter.CreateODataResourceWriter(driveItems, driveItemType); + resourceWriter.WriteStart(new ODataResource + { + EditLink = new Uri("driveItems('parent')", UriKind.Relative) + }); + resourceWriter.WriteStart(new ODataNestedResourceInfo + { + Name = "items", + IsCollection = true, + Url = new Uri("driveItems('parent')/items", UriKind.Relative), + AssociationLinkUrl = new Uri("driveItems('parent')/items/$ref", UriKind.Relative) + }); + resourceWriter.WriteStart(new ODataResourceSet()); + resourceWriter.WriteStart(new ODataResource + { + Id = listItemId, + Properties = new[] { new ODataProperty { Name = "id", Value = "child" } } + }); + resourceWriter.WriteStart(new ODataNestedResourceInfo { Name = "fields", IsCollection = false }); + keySegmentCount = GetCurrentPath(resourceWriter).OfType().Count(); + resourceWriter.WriteStart(new ODataResource + { + Id = new Uri($"{listItemId.OriginalString}/fields", UriKind.Relative) + }); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.Flush(); + } + + string payload = Encoding.UTF8.GetString(stream.ToArray()); + Assert.Equal(2, keySegmentCount); + Assert.Contains("\"@odata.id\":\"driveItems('parent')/items('child')/fields\"", payload); + } + } + [Fact] public void InvalidExplicitIdsPreserveMissingContainedPathException() { @@ -100,7 +162,7 @@ public void InvalidExplicitIdsPreserveMissingContainedPathException() setCustomLinks: true)); Assert.Equal( - "The Path property in ODataMessageWriterSettings.ODataUri must be set when writing contained elements.", + SRResources.ODataWriterCore_PathInODataUriMustBeSetWhenWritingContainedElement, exception.Message); } @@ -209,5 +271,14 @@ private static Uri CreateResourceId(Uri serviceRoot, string path, bool absolute) { return absolute ? new Uri(serviceRoot, path) : new Uri(path, UriKind.Relative); } + + private static ODataPath GetCurrentPath(ODataWriter writer) + { + PropertyInfo currentScopeProperty = typeof(ODataWriterCore).GetProperty( + "CurrentScope", + BindingFlags.Instance | BindingFlags.NonPublic); + ODataWriterCore.Scope currentScope = (ODataWriterCore.Scope)currentScopeProperty.GetValue(writer); + return currentScope.ODataUri.Path; + } } } From f562994fba96edccb1214f5d608012e25be00e52 Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Mon, 24 Aug 2026 10:17:06 -0700 Subject: [PATCH 3/3] Handle collection-only resource IDs Treat recovered resource IDs as complete only when they target a single resource. When an ID resolves to an entity set or collection, append available key properties before building the contained navigation path. Add regression coverage for a recovered collection path with key properties, while preserving the existing no-duplicate-key coverage for complete resource IDs. --- src/Microsoft.OData.Core/ODataWriterCore.cs | 4 +- .../ContainedNavigationPathReproTests.cs | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OData.Core/ODataWriterCore.cs b/src/Microsoft.OData.Core/ODataWriterCore.cs index 33b6489cb7..66ac9298c7 100644 --- a/src/Microsoft.OData.Core/ODataWriterCore.cs +++ b/src/Microsoft.OData.Core/ODataWriterCore.cs @@ -2801,9 +2801,9 @@ private void EnterScope(WriterState newState, ODataItem item) // if there's no key, for example in a complex property, just use the existing odata path. newPath = odataPath; } - else if (pathBuiltFromResourceId) + else if (pathBuiltFromResourceId && !odataPath.IsCollection()) { - // The resource ID represents the complete resource path and already includes its key. + // A resource ID that targets a single resource already represents its complete path. newPath = odataPath; } else diff --git a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs index bfe645dc73..e11f5e7345 100644 --- a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs +++ b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/ContainedNavigationPathReproTests.cs @@ -150,6 +150,65 @@ public void WriteNestedContainedCollectionDoesNotDuplicateKeyInRecoveredResource } } + [Fact] + public void WriteNestedContainedCollectionAppendsKeyToRecoveredCollectionPath() + { + Uri serviceRoot = new Uri("https://service.test/"); + EdmModel model = CreateModel( + listItemNavigationName: "items", + listItemMultiplicity: EdmMultiplicity.Many, + out EdmEntityType driveItemType, + out EdmEntitySet driveItems); + Uri requestUri = new Uri(serviceRoot, "driveItems('parent')"); + ODataMessageWriterSettings settings = CreateSettings(model, serviceRoot, requestUri); + settings.SetContentType("application/json;odata.metadata=minimal", null); + ODataPath recoveredPath; + + using (MemoryStream stream = new MemoryStream()) + { + InMemoryMessage message = new InMemoryMessage { Stream = stream }; + using (ODataMessageWriter messageWriter = new ODataMessageWriter((IODataResponseMessage)message, settings, model)) + { + ODataWriter resourceWriter = messageWriter.CreateODataResourceWriter(driveItems, driveItemType); + resourceWriter.WriteStart(new ODataResource + { + EditLink = new Uri("driveItems('parent')", UriKind.Relative) + }); + resourceWriter.WriteStart(new ODataNestedResourceInfo + { + Name = "items", + IsCollection = true, + Url = new Uri("driveItems('parent')/items", UriKind.Relative), + AssociationLinkUrl = new Uri("driveItems('parent')/items/$ref", UriKind.Relative) + }); + resourceWriter.WriteStart(new ODataResourceSet()); + resourceWriter.WriteStart(new ODataResource + { + Id = new Uri("driveItems('parent')/items", UriKind.Relative), + Properties = new[] { new ODataProperty { Name = "id", Value = "child" } } + }); + resourceWriter.WriteStart(new ODataNestedResourceInfo { Name = "fields", IsCollection = false }); + recoveredPath = GetCurrentPath(resourceWriter); + resourceWriter.WriteStart(new ODataResource + { + Id = new Uri("driveItems('parent')/items('child')/fields", UriKind.Relative) + }); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.WriteEnd(); + resourceWriter.Flush(); + } + } + + Assert.Equal(2, recoveredPath.OfType().Count()); + Assert.Equal( + "driveItems('parent')/items('child')/fields", + recoveredPath.ToResourcePathString(ODataUrlKeyDelimiter.Parentheses)); + } + [Fact] public void InvalidExplicitIdsPreserveMissingContainedPathException() {