Make KeysetColumn<T> part of public API.

[skwasjer]

I would like to be able to assert in unit tests which columns (eg. the expression(s) and its sort direction(s)) are configured on the KeysetPaginationBuilder<T>. But currently the builder its Columns property is internal everywhere (also on the KeysetPaginationContext, as is the KeysetColumn<T> type.

Can we selectively make some of this public? I'd understand you want to avoid exposing too much to keep the API surface small, but tests require too much setup because I have to run the complete query logic, and am effectively writing integration tests. For small unit tests, I do not care and just want to verify that the columns used is correct. I do not really see any downsides of exposing the column expression and direction?

I could also extract it with an expression visitor but this is way too cumbersome and I'd still have to build the entire query (which in my use case is not relevant). Or do you have another recommendation?

[mrahhal]

I want to avoid exposing KeysetColumn because I want the freedom of changing this internally without having to ship a breaking change. This isn't a good class to expose in the public api as it could easily get refactored and I already have plans for that.

A workaround is to use reflection to access the internal Columns prop on KeysetPaginationBuilder (or KeysetPaginationContext), and then again to access IsDescending/LambdaExpression. Not sure why you would need an expression visitor (unless you mean on LambdaExpression, but that's not solved by exposing this). Of course there's no guarantee this won't break in the future, but at least it works now.

Here is a reflection function I've put together quickly:

private static List<(bool IsDecreasing, LambdaExpression LambdaExpression)> GetColumnInternals<T>(KeysetPaginationContext<T> context)
{
	var list = new List<(bool, LambdaExpression)>();

	var columnsProp = context.GetType()
		.GetProperty("Columns", BindingFlags.Instance | BindingFlags.NonPublic)!;
	var columnsObject = columnsProp.GetValue(context) as System.Collections.ICollection;
	foreach (var columnObject in columnsObject)
	{
		var props = columnObject.GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public);
		var isDecreasing = (bool)props.Single(x => x.Name == "IsDescending").GetValue(columnObject);
		var lambdaExpression = (LambdaExpression)props.First(x => x.Name == "LambdaExpression").GetValue(columnObject);
		list.Add((isDecreasing, lambdaExpression));
	}

	return list;
}

[skwasjer]

Reflection is not really a good solution, although I appreciate the suggestion. It is unfortunate, now I need to abstract it away behind my own intermediate builder + value type which is just unnecessary bloat.

KeysetColumn<T> would only need to expose the 2 properties (lambda expression and direction) and the builder or context the columns property. All the other members/methods currently on it could just be made internal. This still should give enough freedom to refactor in the future and not impact the public API surface too much.

Do you at least think you'd be exposing this in the future in some way?

[mrahhal]

I'm aware that direction and the lambda expression are the properties that are needed, but when I say refactoring I'm talking in particular about these properties.

Could you show me an example code and its unit test that you would like to write here? It feels a bit needless to me to have unit tests for the direction/column, since you hard code that in the request. I'm trying to understand your needs for this. Is it because you have dynamic direction/columns depending on request args, and you want to ensure a request arg is translating to the right direction/column?

Do you at least think you'd be exposing this in the future in some way?

The next major version can expose this since I'll be done with a few breaking changes then. Unfortunately I can't tell you when that will happen.

[skwasjer]

Is it because you have dynamic direction/columns depending on request args, and you want to ensure a request arg is translating to the right direction/column?

Exactly this. I have MediatR requests where ordering could either be configured via a protected member, or in other use cases a caller (eg. an API consumer) could specify ordering. So for example when I want to unit test an API controller, I don't actually reach the handler implementation. In this specific case, all I need is to assert that request sent with expected (keyset) arguments.

It feels a bit needless to me to have unit tests for the direction/column, since you hard code that in the request.

This is based on the assumption that we actually execute the query against say an in-mem DB or real DB and verify the results are in expected ordering? Additionally, I actually have a generic implementation where request and query are decoupled, see below.

Just for sake of example, just whipped up.

1. I did not include a test for BulkGetHandler as it is irrelevant in this context, it was written from top of head, so it may contain issues. Do not focus on this part :D
2. This example has no real 'dynamic' column selection but that would still be a viable use case and trivial to add.
3. Of course I could just add my own abstraction over top of how I use the keyset builder, but then I am just moving the problem, I'd still want to be able to add assertions, just in a different place/implementation. As mentioned in OP, I already considered an integration test for that purpose, but one does not exclude the other.

Hopefully this makes it a bit more clear.

using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using MediatR;
using Microsoft.EntityFrameworkCore;
using MR.EntityFrameworkCore.KeysetPagination;
using NSubstitute;
using Xunit;

namespace KeysetExample;

public class MyService
{
    private readonly ISender _sender;

    public MyService(ISender sender)
    {
        _sender = sender;
    }

    public async Task ProcessAllAsync()
    {
        IAsyncEnumerable<MyEntity> items = _sender.CreateStream(new BulkGet<MyEntity>
        {
            Keyset = b => b
                .Ascending(e => e.Name)
                .Ascending(e => e.Id)
        });

        await foreach (MyEntity item in items)
        {
            // Do something.
        }
    }
}

public record MyEntity
{
    public int Id { get; init; }
    public string Name { get; init; }
}

public record BulkGet<T> : IStreamRequest<T>
{
    public Action<KeysetPaginationBuilder<T>> Keyset { get; init; }
}

public class BulkGetHandler<T> : IStreamRequestHandler<BulkGet<T>, T>
    where T : class
{
    private readonly IDbContextFactory<DbContext> _dbContextFactory;

    public BulkGetHandler(IDbContextFactory<DbContext> dbContextFactory)
    {
        _dbContextFactory = dbContextFactory;
    }

    public async IAsyncEnumerable<T> Handle(BulkGet<T> request, [EnumeratorCancellation] CancellationToken cancellationToken)
    {
        await using DbContext dbContext = await _dbContextFactory.CreateDbContextAsync(cancellationToken);
        object? reference = null;
        do
        {
            IQueryable<T> q = dbContext.Set<T>()
                .AsNoTracking()
                .KeysetPaginateQuery(request.Keyset, reference: reference)
                .Take(100);

            reference = null;
            await foreach (T? item in q.AsAsyncEnumerable().WithCancellation(cancellationToken))
            {
                yield return item;
                reference = item;
            }
        } while (reference is not null);
    }
}

public class MyServiceTests
{
    private readonly ISender _senderMock;
    private readonly MyService _sut;

    public MyServiceTests()
    {
        _senderMock = Substitute.For<ISender>();
        _sut = new MyService(_senderMock);
    }

    [Fact]
    public async Task Test_Should_SendQuery()
    {
        MyEntity[] data =
        [
            new()
            {
                Name = "A",
                Id = 1
            },
            new()
            {
                Name = "A",
                Id = 2
            }
        ];
        _senderMock
            .CreateStream(Arg.Any<BulkGet<MyEntity>>(), Arg.Any<CancellationToken>())
            .Returns(data.ToAsyncEnumerable());

        // Act
        await _sut.ProcessAllAsync();

        // Assert
        Func<BulkGet<MyEntity>, bool> assertRequest = req =>
        {
            var b = new KeysetPaginationBuilder<MyEntity>();
            req.Keyset.Invoke(b);
            // Here I would like to do something like:
            KeysetColumn<MyEntity> col0 = b.Columns[0];
            PropertyInfo p0 = MemberVisitor.GetProperty(col0.LambdaExpression);
            return p0.Name == nameof(MyEntity.Name)
                   && p0.DeclaringType == typeof(MyEntity)
                   && !col0.IsDescending;
        };
        await _senderMock
            .Received(1)
            .CreateStream(Arg.Is<BulkGet<MyEntity>>(req => assertRequest(req)), Arg.Any<CancellationToken>())
            .ToListAsync();
    }
}

internal class MemberVisitor : ExpressionVisitor
{
    private PropertyInfo? _property;

    private MemberVisitor()
    {
    }

    public static PropertyInfo GetProperty(LambdaExpression expr)
    {
        var visitor = new MemberVisitor();
        visitor.Visit(expr);
        return visitor._property ?? throw new InvalidOperationException("Not a property.");
    }

    protected override Expression VisitMember(MemberExpression node)
    {
        if (node.Member is PropertyInfo pi)
        {
            _property = pi;
        }

        return base.VisitMember(node);
    }
}