From ccea3b544a9bc3086645a57559b564cc87ed83d6 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:25:03 +0700 Subject: [PATCH 1/8] Add characterization tests for nested member-path resolution Pins current behavior (including four confirmed truncation bugs) across the 8 call sites that resolve a LINQ MemberExpression chain into an OData path segment, as a regression safety net ahead of consolidating them into a single shared resolver. No production code changes. --- .../UnitTests/ExpandWithSelectTests.cs | 66 +++++++++++++++++++ .../UnitTests/ODataClientQueryTests.cs | 16 +++++ .../UnitTests/QueryBuilderAnyAllTests.cs | 44 +++++++++++++ .../UnitTests/QueryBuilderFilterTests.cs | 21 ++++++ .../QueryBuilderQueryOptionsTests.cs | 36 ++++++++++ 5 files changed, 183 insertions(+) diff --git a/PanoramicData.OData.Client.Test/UnitTests/ExpandWithSelectTests.cs b/PanoramicData.OData.Client.Test/UnitTests/ExpandWithSelectTests.cs index 51c7ea1..99b0e11 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/ExpandWithSelectTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/ExpandWithSelectTests.cs @@ -126,6 +126,49 @@ public void ExpandWithSelect_CombinedWithOtherOptions_GeneratesCorrectUrl() url.Should().Contain("$top=10"); } + /// + /// Tests ExpandWithSelect with a nested (dotted) property in the select selector. + /// Characterization test: GetSelectFieldNames/GetDirectMemberName read only the + /// immediate Member.Name (no chain walk), so a nested selector silently truncates + /// to the leaf segment, dropping the intermediate navigation property. Pins this + /// bug as a safety net before the MemberPathResolver consolidation. + /// + [Fact] + public void ExpandWithSelect_NestedPropertyInSelector_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + { + // Arrange & Act + var url = _client.For("ReportBatchJobs") + .ExpandWithSelect( + rbj => rbj.ReportSchedule, + rs => rs.Owner!.Name); + + var builtUrl = url.BuildUrl(); + + // Assert - "Owner/" is silently dropped, leaving just the leaf segment + builtUrl.Should().Contain("$expand=ReportSchedule($select=Name)"); + } + + /// + /// Tests ExpandWithSelect with a NewExpression selector mixing a direct property and a + /// nested (dotted) property. Characterization test: same truncation as above, but via + /// the NewExpression/GetDirectMemberName branch - note this silently produces the same + /// output as selecting ReportSchedule's own Id/Name directly. + /// + [Fact] + public void ExpandWithSelect_NewExpressionWithNestedProperty_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + { + // Arrange & Act + var url = _client.For("ReportBatchJobs") + .ExpandWithSelect( + rbj => rbj.ReportSchedule, + rs => new { rs.Id, rs.Owner!.Name }); + + var builtUrl = url.BuildUrl(); + + // Assert - "Owner/" is silently dropped from the second field + builtUrl.Should().Contain("$expand=ReportSchedule($select=Id,Name)"); + } + #endregion #region Expand with NestedExpandBuilder Tests @@ -148,6 +191,29 @@ public void Expand_WithNestedSelect_GeneratesCorrectSyntax() url.Should().Contain("$expand=ReportSchedule($select=Id,Name)"); } + /// + /// Tests Expand with a nested Select using a nested (dotted) property. + /// Characterization test: NestedExpandBuilder.Select's private GetDirectMemberName + /// reads only the immediate Member.Name (no chain walk), so a nested selector + /// silently truncates to the leaf segment, dropping the intermediate navigation + /// property. Pins this bug as a safety net before the MemberPathResolver + /// consolidation. + /// + [Fact] + public void Expand_NestedSelectWithNestedProperty_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + { + // Arrange & Act + var query = _client.For("ReportBatchJobs") + .Expand( + rbj => rbj.ReportSchedule, + nested => nested.Select(rs => rs.Owner!.Name)); + + var url = query.BuildUrl(); + + // Assert - "Owner/" is silently dropped, leaving just the leaf segment + url.Should().Contain("$expand=ReportSchedule($select=Name)"); + } + /// /// Tests Expand with both nested select and nested expand. /// diff --git a/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs b/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs index 89c5b20..031ba36 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/ODataClientQueryTests.cs @@ -241,6 +241,22 @@ public void NavigateTo_NonGenericExpr_ProducesCorrectPath() url.Should().Be("People('russellwhyte')/Friends"); } + /// + /// 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. + /// + [Fact] + public void NavigateTo_NonGenericExprNested_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + { + // 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"); + } + /// /// Tests that As<T>() after non-generic NavigateTo preserves the navigation path. /// diff --git a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderAnyAllTests.cs b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderAnyAllTests.cs index ca719dc..ba74864 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderAnyAllTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderAnyAllTests.cs @@ -72,6 +72,26 @@ public void Filter_WithAnyOnNavigationProperty_GeneratesCorrectUrl() url.Should().Contain("Friends%2Fany%28f%3A%20f%2FFirstName%20eq%20%27John%27%29"); } + /// + /// Tests that Any() with a nested (two-hop) member path inside the predicate generates + /// correct URL. Characterization test: GetLambdaMemberPath currently walks the full + /// chain correctly here, but this exact scenario was previously untested. Pins it as a + /// safety net before the MemberPathResolver consolidation (which changes the internal + /// List.Insert(0,...) walk to a Stack-based one with byte-identical output). + /// + [Fact] + public void Filter_WithAnyNestedMemberPath_GeneratesCorrectUrl() + { + // Arrange & Act + var url = _queryBuilder + .Filter(p => p.Friends!.Any(f => f.BestFriend!.FirstName == "John")) + .BuildUrl(); + + // Assert + // Friends/any(f: f/BestFriend/FirstName eq 'John') + url.Should().Contain("Friends%2Fany%28f%3A%20f%2FBestFriend%2FFirstName%20eq%20%27John%27%29"); + } + /// /// Tests that Any() with greater than comparison generates correct URL. /// @@ -255,6 +275,30 @@ public void Filter_AnyWithCapturedVariable_GeneratesCorrectUrl() url.Should().Contain("f%2FAge%20ge%2021"); } + /// + /// Tests that a two-hop closure member access (a captured anonymous object's property, + /// rather than a captured local directly) works in an Any lambda. Characterization test: + /// GetLambdaMemberPath walks the chain until it hits the ConstantExpression root, then + /// discards the partially-built path and evaluates the original member expression + /// directly - this exercises that discard-and-evaluate branch with more than one hop + /// before the root, which the existing single-hop closure tests don't reach. Pins this + /// as a safety net before the MemberPathResolver consolidation. + /// + [Fact] + public void Filter_WithAnyNestedClosureMemberAccess_GeneratesCorrectUrl() + { + // Arrange + var ageFilter = new { MinAge = 21 }; + + // Act + var url = _queryBuilder + .Filter(p => p.Friends!.Any(f => f.Age >= ageFilter.MinAge)) + .BuildUrl(); + + // Assert + url.Should().Contain("f%2FAge%20ge%2021"); + } + /// /// Tests that captured string variables work in Any lambda. /// diff --git a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderFilterTests.cs b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderFilterTests.cs index b27244b..91ef308 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderFilterTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderFilterTests.cs @@ -380,4 +380,25 @@ public void Filter_WithDateTimeUtcNow_EvaluatesCurrentTime() } #endregion + + #region Nested Member Paths + + /// + /// Tests filter with a nested (dotted) member path resolves the full path. + /// Characterization test: pins the currently-correct full-chain walk in GetMemberPath + /// as a safety net before the MemberPathResolver consolidation. + /// + [Fact] + public void Filter_WithNestedMemberPath_GeneratesCorrectUrl() + { + // Arrange & Act + var url = new ODataQueryBuilder("People", NullLogger.Instance) + .Filter(p => p.BestFriend!.FirstName == "John") + .BuildUrl(); + + // Assert - BestFriend/FirstName eq 'John' + url.Should().Contain("BestFriend%2FFirstName%20eq%20%27John%27"); + } + + #endregion } diff --git a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs index 040bd25..231fa0a 100644 --- a/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs +++ b/PanoramicData.OData.Client.Test/UnitTests/QueryBuilderQueryOptionsTests.cs @@ -50,6 +50,24 @@ public void Select_MultipleCalls_CombinesFields() url.Should().Contain("$select=ID,Name,Price"); } + /// + /// Tests select with a nested (dotted) member path. + /// Characterization test: GetMemberNames currently takes only the FIRST path segment + /// (Split('/')[0]) of the full path GetMemberPath resolves, so a nested selector + /// silently collapses to just the navigation property name. Pins this quirk as a + /// safety net before the MemberPathResolver consolidation. + /// + [Fact] + public void Select_WithNestedExpression_UsesOnlyFirstSegment_CharacterizationOfExistingBehavior() + { + var url = new ODataQueryBuilder("People", NullLogger.Instance) + .Select(p => p.BestFriend!.FirstName) + .BuildUrl(); + + url.Should().Contain("$select=BestFriend"); + url.Should().NotContain("FirstName"); + } + #endregion #region $expand @@ -207,6 +225,24 @@ public void OrderBy_WithExpressionDescending_GeneratesCorrectUrl() url.Should().Contain("$orderby=Price desc"); } + /// + /// 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. + /// + [Fact] + public void OrderBy_WithNestedExpression_TruncatesToLeafSegment_CharacterizationOfExistingBehavior() + { + var url = new ODataQueryBuilder("People", NullLogger.Instance) + .OrderBy(p => p.BestFriend!.FirstName) + .BuildUrl(); + + url.Should().Contain("$orderby=FirstName"); + url.Should().NotContain("BestFriend"); + } + /// /// Tests multiple orderby clauses. /// From f1ec4734fb2ac5bc98c2668afb8f34bf5798f76b Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:27:01 +0700 Subject: [PATCH 2/8] Add MemberPathResolver.WalkChain/GetFlatPath and migrate GetMemberPath First step of unifying the seven independent member-path-resolution implementations. GetMemberPath (used by $filter, Contains/in clauses, string methods, and top-level Select) now delegates to the new shared resolver with byte-identical output. No behavior change - full suite green (786/786), including the new characterization tests. --- .../MemberPathResolver.cs | 48 +++++++++++++++++++ .../ODataQueryBuilder.ExpressionParsing.cs | 21 +------- 2 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 PanoramicData.OData.Client/MemberPathResolver.cs diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs new file mode 100644 index 0000000..d11f5bc --- /dev/null +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -0,0 +1,48 @@ +using System.Linq; +using System.Reflection; + +namespace PanoramicData.OData.Client; + +/// +/// Shared logic for resolving a LINQ chain into OData path segments. +/// Consolidates what were previously several independent, hand-rolled implementations across +/// and . +/// +internal static class MemberPathResolver +{ + /// + /// Walks a member access chain (e.g. p.BestFriend.FirstName) from leaf to root, + /// returning each segment's name and , plus the terminal + /// (non-) root expression - typically the lambda parameter, + /// a closure , or . + /// + internal static (Stack<(string Name, MemberInfo Member)> Segments, Expression? Root) WalkChain(MemberExpression member) + { + var segments = new Stack<(string Name, MemberInfo Member)>(); + Expression? current = member; + + while (current is MemberExpression memberExpr) + { + segments.Push((memberExpr.Member.Name, memberExpr.Member)); + current = memberExpr.Expression; + } + + return (segments, current); + } + + /// + /// Resolves a member access chain to a flat, slash-separated OData path (e.g. BestFriend/FirstName). + /// + internal static string GetFlatPath(MemberExpression member) + { + var (segments, _) = WalkChain(member); + + // For small paths (common case), avoid string.Join allocation + return segments.Count switch + { + 0 => string.Empty, + 1 => segments.Pop().Name, + _ => string.Join("/", segments.Select(s => s.Name)) + }; + } +} diff --git a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs index 017878a..b223dab 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs @@ -278,26 +278,7 @@ private static string FormatInClause(string propertyPath, System.Collections.IEn return $"{propertyPath} in ({string.Join(",", formattedValues)})"; } - private static string GetMemberPath(MemberExpression member) - { - // Use a stack to avoid List.Insert(0) which is O(n) - var pathStack = new Stack(); - Expression? current = member; - - while (current is MemberExpression memberExpr) - { - pathStack.Push(memberExpr.Member.Name); - current = memberExpr.Expression; - } - - // For small paths (common case), avoid string.Join allocation - return pathStack.Count switch - { - 0 => string.Empty, - 1 => pathStack.Pop(), - _ => string.Join("/", pathStack) - }; - } + private static string GetMemberPath(MemberExpression member) => MemberPathResolver.GetFlatPath(member); private static object? GetValue(Expression expression) => expression switch { From e65f606e3c820dd29db9002b7387d2702805fcf9 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:30:52 +0700 Subject: [PATCH 3/8] Migrate GetExpandPathInfoFromExpression to MemberPathResolver.GetExpandSegments Relocates ExpandSegment and IsNavigationProperty (both T-independent) to MemberPathResolver alongside the new GetExpandSegments, built on WalkChain. Best-covered path in the codebase (NestedExpandExpressionTests, 3-level nesting) - strong evidence the shared primitive is correct before riskier migrations rely on it. No behavior change - full suite green (786/786). --- .../MemberPathResolver.cs | 92 +++++++++++++++++++ .../ODataQueryBuilder.ExpressionParsing.cs | 88 +----------------- 2 files changed, 94 insertions(+), 86 deletions(-) diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs index d11f5bc..4c4b764 100644 --- a/PanoramicData.OData.Client/MemberPathResolver.cs +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -45,4 +45,96 @@ internal static string GetFlatPath(MemberExpression member) _ => string.Join("/", segments.Select(s => s.Name)) }; } + + /// + /// Resolves a member access chain to a root-to-leaf list of , + /// each flagged as navigation (entity reference/collection) or scalar. Returns + /// only when 's chain is empty, which + /// cannot occur given a non-null input - preserved to + /// mirror the original implementation's guard exactly. + /// + internal static List? GetExpandSegments(MemberExpression member) + { + var (chain, _) = WalkChain(member); + + if (chain.Count == 0) + { + return null; + } + + var segments = new List(chain.Count); + + foreach (var (name, memberInfo) in chain) + { + segments.Add(memberInfo is PropertyInfo propInfo + ? new ExpandSegment(propInfo.Name, IsNavigationProperty(propInfo)) + : new ExpandSegment(name, false)); + } + + return segments; + } + + /// + /// Determines if a property is a navigation property (vs a scalar property). + /// Navigation properties are entity references or collections of entities. + /// Scalar properties are primitives, strings, dates, guids, etc. + /// + internal static bool IsNavigationProperty(PropertyInfo property) + { + var propertyType = property.PropertyType; + + // Check for nullable types - get underlying type + var underlyingType = Nullable.GetUnderlyingType(propertyType); + if (underlyingType is not null) + { + propertyType = underlyingType; + } + + // Primitives are scalar + if (propertyType.IsPrimitive) + { + return false; + } + + // Common scalar types + if (propertyType == typeof(string) || + propertyType == typeof(DateTime) || + propertyType == typeof(DateTimeOffset) || + propertyType == typeof(DateOnly) || + propertyType == typeof(TimeOnly) || + propertyType == typeof(TimeSpan) || + propertyType == typeof(Guid) || + propertyType == typeof(decimal)) + { + return false; + } + + // Enums are scalar + if (propertyType.IsEnum) + { + return false; + } + + // byte[] is scalar (used for binary data) + if (propertyType == typeof(byte[])) + { + return false; + } + + // Collections of entities are navigation properties (but not string which is IEnumerable) + if (typeof(System.Collections.IEnumerable).IsAssignableFrom(propertyType) && + propertyType != typeof(string)) + { + return true; + } + + // Reference types that are classes are typically navigation properties + // (entity references like Tenant, Role, etc.) + return propertyType.IsClass; + } } + +/// +/// Represents a segment in an expand path with property type information. +/// +internal sealed record ExpandSegment(string Name, bool IsNavigation); diff --git a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs index b223dab..1088e7d 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs @@ -579,88 +579,9 @@ private static List GetExpandMemberPathsWithInfo(Expression(); - Expression? current = member; - - while (current is MemberExpression memberExpr) - { - if (memberExpr.Member is PropertyInfo propInfo) - { - segments.Insert(0, new ExpandSegment(propInfo.Name, IsNavigationProperty(propInfo))); - } - else - { - segments.Insert(0, new ExpandSegment(memberExpr.Member.Name, false)); - } - - current = memberExpr.Expression; - } - - if (segments.Count == 0) - { - return null; - } + var segments = MemberPathResolver.GetExpandSegments(member); - return new ExpandPathInfo(segments); - } - - /// - /// Determines if a property is a navigation property (vs a scalar property). - /// Navigation properties are entity references or collections of entities. - /// Scalar properties are primitives, strings, dates, guids, etc. - /// - private static bool IsNavigationProperty(PropertyInfo property) - { - var propertyType = property.PropertyType; - - // Check for nullable types - get underlying type - var underlyingType = Nullable.GetUnderlyingType(propertyType); - if (underlyingType is not null) - { - propertyType = underlyingType; - } - - // Primitives are scalar - if (propertyType.IsPrimitive) - { - return false; - } - - // Common scalar types - if (propertyType == typeof(string) || - propertyType == typeof(DateTime) || - propertyType == typeof(DateTimeOffset) || - propertyType == typeof(DateOnly) || - propertyType == typeof(TimeOnly) || - propertyType == typeof(TimeSpan) || - propertyType == typeof(Guid) || - propertyType == typeof(decimal)) - { - return false; - } - - // Enums are scalar - if (propertyType.IsEnum) - { - return false; - } - - // byte[] is scalar (used for binary data) - if (propertyType == typeof(byte[])) - { - return false; - } - - // Collections of entities are navigation properties (but not string which is IEnumerable) - if (typeof(System.Collections.IEnumerable).IsAssignableFrom(propertyType) && - propertyType != typeof(string)) - { - return true; - } - - // Reference types that are classes are typically navigation properties - // (entity references like Tenant, Role, etc.) - return propertyType.IsClass; + return segments is null ? null : new ExpandPathInfo(segments); } /// @@ -750,11 +671,6 @@ private static void AddPathToTreeWithInfo(Dictionary nodes, AddPathToTreeWithInfo(node.Children, segments, index + 1); } - /// - /// Represents a segment in an expand path with property type information. - /// - private sealed record ExpandSegment(string Name, bool IsNavigation); - /// /// Represents an expand path with its segments and property type information. /// From 5fef4ca8c8a92fe8ca7e97364157714277d3b0a7 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:31:51 +0700 Subject: [PATCH 4/8] Migrate GetNavigationPropertyName/GetCollectionNavigationPropertyName to MemberPathResolver.GetLeafMemberNameOrEmpty Simplest, bug-free, single-hop-by-design consumer - first migration onto the new leaf-read primitive. No behavior change - full suite green (786/786). --- .../MemberPathResolver.cs | 18 +++++++++ .../ODataQueryBuilder.cs | 38 ++----------------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs index 4c4b764..13572e5 100644 --- a/PanoramicData.OData.Client/MemberPathResolver.cs +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -74,6 +74,24 @@ internal static string GetFlatPath(MemberExpression member) return segments; } + /// + /// Resolves the leaf (immediate) member name of a single-hop selector body (e.g. p.Orders), + /// unwrapping a single (such as the null-forgiving operator) + /// if present. Does NOT walk nested chains - returns just the last segment's name for a + /// multi-hop selector (e.g. p.Nav.Prop resolves to "Prop"). Returns + /// if isn't (optionally Convert-wrapped) + /// a . + /// + internal static string GetLeafMemberNameOrEmpty(Expression expression) + { + if (expression is UnaryExpression unary && unary.NodeType == ExpressionType.Convert) + { + expression = unary.Operand; + } + + return expression is MemberExpression member ? member.Member.Name : string.Empty; + } + /// /// 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.cs b/PanoramicData.OData.Client/ODataQueryBuilder.cs index fe9208a..1bfbe6b 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.cs @@ -463,41 +463,11 @@ public ODataQueryBuilder Expand( return this; } - private static string GetNavigationPropertyName(Expression> selector) - { - var body = selector.Body; - - // Handle null-forgiving operator (!.) - if (body is UnaryExpression unary && unary.NodeType == ExpressionType.Convert) - { - body = unary.Operand; - } - - if (body is MemberExpression member) - { - return member.Member.Name; - } - - return string.Empty; - } + private static string GetNavigationPropertyName(Expression> selector) => + MemberPathResolver.GetLeafMemberNameOrEmpty(selector.Body); - private static string GetCollectionNavigationPropertyName(Expression?>> selector) - { - var body = selector.Body; - - // Handle null-forgiving operator (!.) - if (body is UnaryExpression unary && unary.NodeType == ExpressionType.Convert) - { - body = unary.Operand; - } - - if (body is MemberExpression member) - { - return member.Member.Name; - } - - return string.Empty; - } + private static string GetCollectionNavigationPropertyName(Expression?>> selector) => + MemberPathResolver.GetLeafMemberNameOrEmpty(selector.Body); private static List GetSelectFieldNames(Expression> selector) { From fd668b6d5a1bd30a5041534ecc14477280376f6f Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:33:31 +0700 Subject: [PATCH 5/8] Migrate GetSelectFieldNames to MemberPathResolver.GetLeafMemberNameOrEmpty ExpandWithSelect's nested $select field resolution now uses the shared leaf-read primitive; the local GetDirectMemberName is removed as redundant. First call site with a confirmed untested truncation bug (nested property in the select selector), gated on its new characterization test. No behavior change - full suite green (786/786). --- .../MemberPathResolver.cs | 1 - PanoramicData.OData.Client/ODataQueryBuilder.cs | 17 +---------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs index 13572e5..91dd831 100644 --- a/PanoramicData.OData.Client/MemberPathResolver.cs +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -1,4 +1,3 @@ -using System.Linq; using System.Reflection; namespace PanoramicData.OData.Client; diff --git a/PanoramicData.OData.Client/ODataQueryBuilder.cs b/PanoramicData.OData.Client/ODataQueryBuilder.cs index 1bfbe6b..15fe684 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.cs @@ -485,7 +485,7 @@ private static List GetSelectFieldNames(Expression GetSelectFieldNames(Expression /// Adds an order by clause. /// From 1bf04eea0cc0ed5eda93c9636016d9dec4bdc77b Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:34:39 +0700 Subject: [PATCH 6/8] Migrate NestedExpandBuilder.Select/.Expand to MemberPathResolver.GetLeafMemberNameOrEmpty First cross-class consumer of MemberPathResolver (NestedExpandBuilder is a separate generic class from ODataQueryBuilder), proving the non-generic static design works across both. Removes the now-unused local GetDirectMemberName. No behavior change - full suite green (786/786), including the new nested-Select truncation characterization test. --- .../NestedExpandBuilder.cs | 28 +++---------------- 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/PanoramicData.OData.Client/NestedExpandBuilder.cs b/PanoramicData.OData.Client/NestedExpandBuilder.cs index 74ad930..d0546b9 100644 --- a/PanoramicData.OData.Client/NestedExpandBuilder.cs +++ b/PanoramicData.OData.Client/NestedExpandBuilder.cs @@ -29,7 +29,7 @@ public NestedExpandBuilder Select(Expression> selector) { foreach (var arg in newExpr.Arguments) { - var memberName = GetDirectMemberName(arg); + var memberName = MemberPathResolver.GetLeafMemberNameOrEmpty(arg); if (!string.IsNullOrEmpty(memberName) && !_selectFields.Contains(memberName)) { _selectFields.Add(memberName); @@ -62,16 +62,11 @@ public NestedExpandBuilder Select(string fields) /// public NestedExpandBuilder Expand(Expression> selector) { - var body = selector.Body; - - if (body is UnaryExpression unary && unary.NodeType == ExpressionType.Convert) - { - body = unary.Operand; - } + var memberName = MemberPathResolver.GetLeafMemberNameOrEmpty(selector.Body); - if (body is MemberExpression member) + if (!string.IsNullOrEmpty(memberName)) { - _expandFields.Add(member.Member.Name); + _expandFields.Add(memberName); } return this; @@ -174,19 +169,4 @@ internal string Build() return string.Join(";", options); } - - private static string GetDirectMemberName(Expression expression) - { - if (expression is UnaryExpression unary && unary.NodeType == ExpressionType.Convert) - { - expression = unary.Operand; - } - - if (expression is MemberExpression member) - { - return member.Member.Name; - } - - return string.Empty; - } } From d41f9c5edc24ded147b689eec58b725267c81410 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:35:43 +0700 Subject: [PATCH 7/8] Migrate GetMemberName to MemberPathResolver.TryGetLeafMemberNameLoose Two call sites (OrderBy, NavigateTo) re-verified against the shared primitive. This is where the Convert-vs-any-Unary discrepancy from the other leaf-read call sites lives, so it's kept as its own distinct method rather than folded into GetLeafMemberNameOrEmpty. No behavior change - full suite green (786/786), including the OrderBy/NavigateTo truncation characterization tests. --- .../MemberPathResolver.cs | 28 +++++++++++++++++++ .../ODataQueryBuilder.ExpressionParsing.cs | 18 +++--------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/PanoramicData.OData.Client/MemberPathResolver.cs b/PanoramicData.OData.Client/MemberPathResolver.cs index 91dd831..4ca7f0e 100644 --- a/PanoramicData.OData.Client/MemberPathResolver.cs +++ b/PanoramicData.OData.Client/MemberPathResolver.cs @@ -91,6 +91,34 @@ internal static string GetLeafMemberNameOrEmpty(Expression expression) return expression is MemberExpression member ? member.Member.Name : string.Empty; } + /// + /// Resolves the leaf (immediate) member name of a single-hop selector body, using a looser + /// unwrap than : matches ANY + /// wrapping a , not just , and + /// does not itself unwrap before the primary check (only the fallback branch inspects a + /// Unary's ). Kept deliberately distinct from + /// - the two gates coincide for every expression shape + /// the C# compiler currently emits here, but must not be silently unified. Does NOT walk nested + /// chains - returns just the last segment's name for a multi-hop selector. + /// + internal static bool TryGetLeafMemberNameLoose(Expression expression, out string name) + { + if (expression is MemberExpression member) + { + name = member.Member.Name; + return true; + } + + if (expression is UnaryExpression unary && unary.Operand is MemberExpression unaryMember) + { + name = unaryMember.Member.Name; + return true; + } + + name = 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 1088e7d..573c6cb 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.ExpressionParsing.cs @@ -508,20 +508,10 @@ private static string GetMemberPathFromExpression(Expression expression) return string.Empty; } - private static string GetMemberName(Expression> selector) - { - if (selector.Body is MemberExpression member) - { - return member.Member.Name; - } - - if (selector.Body is UnaryExpression unary && unary.Operand is MemberExpression unaryMember) - { - return unaryMember.Member.Name; - } - - throw new ArgumentException("Invalid selector expression"); - } + private static string GetMemberName(Expression> selector) => + MemberPathResolver.TryGetLeafMemberNameLoose(selector.Body, out var name) + ? name + : throw new ArgumentException("Invalid selector expression"); /// /// Gets full member paths from an expand expression. From 4d9a1a98fcea22ae0e6e63526fcdca61fbfcd408 Mon Sep 17 00:00:00 2001 From: Roland Banks Date: Wed, 8 Jul 2026 14:37:13 +0700 Subject: [PATCH 8/8] Migrate GetLambdaMemberPath to MemberPathResolver.WalkChain Final migration - consumes WalkChain directly rather than a flat-path helper, since Any/All predicates need the raw root expression to decide between lambda-parameter prefixing and closure-constant evaluation; that dual-root branching stays local to LambdaParsing.cs. As a side effect this replaces the old List.Insert(0,...) O(n^2) walk with the Stack-based O(n) one, with byte-identical output. Highest-risk step (genuinely different logic, weakest prior coverage) - saved for last, gated on the new two-hop-predicate and two-level-closure characterization tests. No behavior change - full suite green (786/786). --- .../ODataQueryBuilder.LambdaParsing.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/PanoramicData.OData.Client/ODataQueryBuilder.LambdaParsing.cs b/PanoramicData.OData.Client/ODataQueryBuilder.LambdaParsing.cs index adceeb1..f55d7c2 100644 --- a/PanoramicData.OData.Client/ODataQueryBuilder.LambdaParsing.cs +++ b/PanoramicData.OData.Client/ODataQueryBuilder.LambdaParsing.cs @@ -196,24 +196,19 @@ private static string ParseLambdaStringMethod(MethodCallExpression methodCall, P private static string GetLambdaMemberPath(MemberExpression member, ParameterExpression lambdaParam, string odataParamName) { - var path = new List(); - Expression? current = member; + var (segments, root) = MemberPathResolver.WalkChain(member); + var names = segments.Select(s => s.Name); - while (current is MemberExpression memberExpr) + if (root == lambdaParam) { - path.Insert(0, memberExpr.Member.Name); - current = memberExpr.Expression; + return string.Join("/", new[] { odataParamName }.Concat(names)); } - if (current == lambdaParam) - { - path.Insert(0, odataParamName); - } - else if (current is ConstantExpression) + if (root is ConstantExpression) { return FormatValue(EvaluateExpression(member)); } - return string.Join("/", path); + return string.Join("/", names); } }