fix: add defensive reflection checks with ReflectionMethodCache

[Erlend Ellefsen (erlendellefsen)]

• Centralize reflection method lookups in ReflectionMethodCache for DRY
• Cache method lookups for performance
• Add descriptive error messages when lookups fail (includes issue URL)
• Add 12 regression tests

[Copilot]

Pull request overview

Adds a centralized ReflectionMethodCache to DRY up and harden reflection-based method lookups used for LINQ/EF Core expression building, and enables tests to validate these lookups.

Changes:

• Introduces ReflectionMethodCache for common LINQ/EF Core method resolution with clearer failure messages.
• Refactors querying components (includes/filtering/sorting) to use the centralized cache.
• Exposes internals to the test project and adds regression tests for the cache.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
JsonApiToolkit/JsonApiToolkit.csproj	Adds InternalsVisibleTo so tests can call internal cache APIs.
JsonApiToolkit/Helpers/ReflectionMethodCache.cs	New helper that centralizes reflection method discovery and (partially) caches results.
JsonApiToolkit/Extensions/Querying/Includes/FilteredIncludeBuilder.cs	Replaces direct reflection calls with ReflectionMethodCache lookups.
JsonApiToolkit/Extensions/Querying/Includes/EfCoreIncludeExpressions.cs	Routes Include/ThenInclude method selection through the cache.
JsonApiToolkit/Extensions/Querying/Handlers/SortingHandler.cs	Uses cache for Queryable ordering method discovery.
JsonApiToolkit/Extensions/Querying/Filtering/NestedPropertyNavigator.cs	Uses cache for Any/Contains method discovery in expression building.
JsonApiToolkit.Tests/Helpers/ReflectionMethodCacheTests.cs	Adds unit tests for the cache lookup helpers and error behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

GetEnumerableWhere only filters by method name and parameter count. Enumerable.Where has two 2-parameter overloads (Func<TSource,bool> and Func<TSource,int,bool>), so this lookup can non-deterministically pick the index-aware overload, which will later break Expression.Call when you pass a Func<T,bool> lambda. Tighten the predicate to match the expected signature (e.g., ensure the 2nd parameter generic type definition is Func<,>).

[Copilot]

The IIncludableQueryable check relies on comparing GetGenericTypeDefinition().Name to a string. This is brittle and can accidentally match unrelated types or break if EF changes type names. Prefer comparing to the actual open generic type (e.g., typeof(IIncludableQueryable<,>) from Microsoft.EntityFrameworkCore.Query) or at least compare FullName.

[Copilot]

If no ThenInclude overload matches isPreviousCollection, falling back to the first ThenInclude method can select the wrong overload and cause a later, harder-to-diagnose runtime failure. It would be safer to throw an InvalidOperationException when no matching overload is found (and include isPreviousCollection + involved types in the message).

Suggested change

	// Fallback with defensive check
	var fallbackMethod =
	thenIncludeMethods.FirstOrDefault()
	?? throw new InvalidOperationException(
	"Could not find EntityFrameworkQueryableExtensions.ThenInclude method. "
	+ "Ensure Microsoft.EntityFrameworkCore is properly referenced. "
	+ "Please report this issue at https://github.com/Intility/Intility.JsonApiToolkit/issues"
	);
	return fallbackMethod.MakeGenericMethod(entityType, previousPropertyType, newPropertyType);
	// No matching overload found; fail fast with a clear error message instead of
	// falling back to an arbitrary ThenInclude overload, which could cause subtle
	// runtime failures later.
	if (!thenIncludeMethods.Any())
	{
	throw new InvalidOperationException(
	"Could not find EntityFrameworkQueryableExtensions.ThenInclude methods. "
	+ "Ensure Microsoft.EntityFrameworkCore is properly referenced. "
	+ "Please report this issue at https://github.com/Intility/Intility.JsonApiToolkit/issues"
	);
	}
	throw new InvalidOperationException(
	"Could not find a matching EntityFrameworkQueryableExtensions.ThenInclude overload. "
	+ $"isPreviousCollection={isPreviousCollection}, "
	+ $"entityType={entityType}, "
	+ $"previousPropertyType={previousPropertyType}, "
	+ $"newPropertyType={newPropertyType}. "
	+ "This likely indicates an unexpected change in EF Core's ThenInclude overloads. "
	+ "Please report this issue at https://github.com/Intility/Intility.JsonApiToolkit/issues"
	);

[Copilot]

GetQueryableOrderingMethod does a full reflection scan on every call. Since SortingHandler calls this once per sort field, consider caching the generic method definition per methodName (e.g., static fields/dictionary) to match the stated intent of ReflectionMethodCache and avoid repeated allocations.

[Copilot]

This test/comment is misleading: it asserts two calls return equal MethodInfo, but that doesn’t actually prove the underlying reflection lookup was cached (it can still be equal even if looked up twice), and the comment mentions “different generic instantiation” even though both calls use typeof(int). Consider revising the test to validate caching behavior more directly or rename it to reflect what it truly verifies.

Suggested change

	public void GetEnumerableAnyWithPredicate_IsCached()
	{
	// Act
	var method1 = ReflectionMethodCache.GetEnumerableAnyWithPredicate(typeof(int));
	var method2 = ReflectionMethodCache.GetEnumerableAnyWithPredicate(typeof(int));
	// Assert - both should be the same instance (cached base method, different generic instantiation)
	Assert.Equal(method1, method2);
	public void GetEnumerableAnyWithPredicate_ReturnsSameMethodInfoForSameType()
	{
	// Act
	var method1 = ReflectionMethodCache.GetEnumerableAnyWithPredicate(typeof(int));
	var method2 = ReflectionMethodCache.GetEnumerableAnyWithPredicate(typeof(int));
	// Assert - repeated calls with the same type return the same MethodInfo instance
	Assert.Same(method1, method2);