From 047ee7b94f9a5950557f56ebfccc5f86a4fbe685 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 15:07:21 +0700 Subject: [PATCH] Fix .OrderBy/.NavigateTo truncating nested (dotted) property paths GetMemberName (feeding both .OrderBy(Expression>, bool) and .NavigateTo(Expression>)) only read the selector body's immediate leaf member name, silently dropping any intermediate navigation properties - .OrderBy(p => p.BestFriend.FirstName) produced $orderby=FirstName instead of $orderby=BestFriend/FirstName. Adds MemberPathResolver.TryGetPathLoose, mirroring the existing TryGetLeafMemberNameLoose's loose Unary-gate but resolving the full path via the already-correct GetFlatPath instead of just the leaf name. OData v4's $orderby/$filter/resource-path grammar formally supports "/"-paths through navigation properties, so this fix is fully spec-compliant. The three related $select-family truncation bugs (.Select, .ExpandWithSelect, NestedExpandBuilder.Select) are NOT touched here - fixing those the same way would produce $select paths through navigation properties, which OData v4's $select grammar only defines for complex-typed properties. That's a separate design decision, deliberately deferred. Full suite green (787/787). --- CHANGELOG.md | 5 ++++ .../UnitTests/ODataClientQueryTests.cs | 13 +++++---- .../QueryBuilderQueryOptionsTests.cs | 26 ++++++++++++------ .../MemberPathResolver.cs | 27 +++++++++++++++++++ .../ODataQueryBuilder.ExpressionParsing.cs | 2 +- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a29dd3..b1e097d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [10.0.88] - 2026-07-08 + +### Fixed +- Fix `.OrderBy(p => p.Nav.Prop)` and `.NavigateTo(p => p.Nav.Prop)` resolving only the leaf property name (e.g. `$orderby=FirstName`) instead of the full navigation path (`$orderby=BestFriend/FirstName`) for nested (dotted) property selectors + ## [10.0.87] - 2026-06-12 ### Added diff --git a/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs b/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs index 031ba36..7bc3d4a 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs @@ -242,19 +242,18 @@ public void NavigateTo_NonGenericExpr_ProducesCorrectPath() } /// - /// Tests that non-generic NavigateTo(expr) with a nested (dotted) member path truncates - /// to the leaf segment. Characterization test: NavigateTo shares GetMemberName with - /// OrderBy, which reads only the selector body's immediate Member.Name (no chain walk). - /// Pins this bug as a safety net before the MemberPathResolver consolidation. + /// Tests that non-generic NavigateTo(expr) with a nested (dotted) member path resolves the + /// full navigation path. NavigateTo shares GetMemberName with OrderBy; both now walk the + /// full chain instead of returning just the leaf segment. /// [Fact] - public void NavigateTo_NonGenericExprNested_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + public void NavigateTo_NonGenericExprNested_ResolvesFullPath() { // Act var url = _client.For("People").Key("russellwhyte").NavigateTo(x => x.BestFriend!.Friends).BuildUrl(); - // Assert - "BestFriend/" is silently dropped, leaving just the leaf segment - url.Should().Be("People('russellwhyte')/Friends"); + // Assert + url.Should().Be("People('russellwhyte')/BestFriend/Friends"); } /// diff --git a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs index 231fa0a..d28813a 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs @@ -226,21 +226,31 @@ public void OrderBy_WithExpressionDescending_GeneratesCorrectUrl() } /// - /// Tests orderby with a nested (dotted) member path. - /// Characterization test: GetMemberName reads only the selector body's immediate - /// Member.Name (no chain walk), so a nested selector silently truncates to the leaf - /// segment, dropping the navigation property. Pins this bug as a safety net before - /// the MemberPathResolver consolidation. + /// Tests orderby with a nested (dotted) member path resolves the full navigation path. + /// OData v4's $orderby grammar supports "/"-paths through navigation properties, so this + /// is spec-compliant (unlike the still-deferred $select-family nested-path fixes). /// [Fact] - public void OrderBy_WithNestedExpression_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + public void OrderBy_WithNestedExpression_ResolvesFullPath() { var url = new ODataQueryBuilder("People", NullLogger.Instance) .OrderBy(p => p.BestFriend!.FirstName) .BuildUrl(); - url.Should().Contain("$orderby=FirstName"); - url.Should().NotContain("BestFriend"); + url.Should().Contain("$orderby=BestFriend/FirstName"); + } + + /// + /// Tests orderby with a 3-level nested member path resolves the full navigation path. + /// + [Fact] + public void OrderBy_WithThreeLevelNestedExpression_ResolvesFullPath() + { + var url = new ODataQueryBuilder("People", NullLogger.Instance) + .OrderBy(p => p.BestFriend!.BestFriend!.FirstName) + .BuildUrl(); + + url.Should().Contain("$orderby=BestFriend/BestFriend/FirstName"); } /// diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs index 4ca7f0e..5f904c3 100644 --- a/PanoramicData.OData.Client/MemberPathResolver.cs +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -119,6 +119,33 @@ internal static bool TryGetLeafMemberNameLoose(Expression expression, out string return false; } + /// + /// Resolves the full, slash-separated OData path of a selector body, using the same loose + /// Unary-gated unwrap as . Unlike that method, this + /// walks the full chain via instead of returning just the leaf + /// segment (e.g. p.BestFriend.FirstName resolves to "BestFriend/FirstName"). + /// Kept deliberately distinct from for the same + /// reason that method is kept distinct from - the + /// gates coincide today but must not be silently unified. + /// + internal static bool TryGetPathLoose(Expression expression, out string path) + { + if (expression is MemberExpression member) + { + path = GetFlatPath(member); + return true; + } + + if (expression is UnaryExpression unary && unary.Operand is MemberExpression unaryMember) + { + path = GetFlatPath(unaryMember); + return true; + } + + path = string.Empty; + return false; + } + /// /// Determines if a property is a navigation property (vs a scalar property). /// Navigation properties are entity references or collections of entities. diff --git a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs index 573c6cb..60b4675 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs @@ -509,7 +509,7 @@ private static string GetMemberPathFromExpression(Expression expression) } private static string GetMemberName(Expression> selector) => - MemberPathResolver.TryGetLeafMemberNameLoose(selector.Body, out var name) + MemberPathResolver.TryGetPathLoose(selector.Body, out var name) ? name : throw new ArgumentException("Invalid selector expression");