Skip to content

Commit

Permalink
- Fix bug with in-memory cursor paging logic incorrectly indexing the…
Browse files Browse the repository at this point in the history
… results resulting in invalid page results.

- Removed unnecessary class constraint for both cursor and offset in-memory paging extensions making them more flexible.
- Improved Unit test naming for async methods.
- Fixed method comments for in-memory extensions.
  • Loading branch information
cajuncoding committed May 24, 2024
1 parent 2aeff1b commit 5acdcc4
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
<PropertyGroup>
<TargetFrameworks>netstandard2.0;netstandard2.1;net6.0;</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Version>1.1.5.5</Version>
<AssemblyVersion>1.1.5.5</AssemblyVersion>
<FileVersion>1.1.5.5</FileVersion>
<Version>1.1.5.6</Version>
<AssemblyVersion>1.1.5.6</AssemblyVersion>
<FileVersion>1.1.5.6</FileVersion>
<Authors>BBernard / CajunCoding</Authors>
<Company>CajunCoding</Company>
<Description>The primitives and helpers needed for RepoDbExtensions.SqlServer.PagingOperations pacakge; used for working with modern pagination approaches such as Cursor based paging, as well as Offset based pagination, using the RepoDb ORM with Sql Server.</Description>
Expand All @@ -16,9 +16,10 @@
<PackageTags>repodb, paging, pagination, cursor, offset, skip, take, sorting, graphql, graph-ql, hotchocolate, dapper, sqlkata</PackageTags>
<PackageReleaseNotes>
Release Notes:
- Improve flexibility of base interface support for cursor navigation with non-generic ICursorPageNavigationInfo.
- Fix bug with in-memory cursor paging logic incorrectly indexing the results resulting in invalid page results.

Prior Release Notes:
- Improve flexibility of base interface support for cursor navigation with non-generic ICursorPageNavigationInfo.
- Simplify RetrieveTotalCount param name on ICursorParams.
- Fix SQL building bug not using Raw SQL as specified; improved integration tests to validate this better.
- Improved raw sql parameter name to be more consistent with RepDb naming conventions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace RepoDb.SqlServer.PagingOperations.Tests
public class PagingTestsUsingExecuteQueryApiForRawSql : BaseTest
{
[TestMethod]
public async Task TestCursorPagingWithRawSql()
public async Task TestCursorPagingWithRawSqlAsync()
{
using (var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false))
{
Expand Down Expand Up @@ -75,7 +75,7 @@ public async Task TestCursorPagingWithRawSql()
}

[TestMethod]
public async Task TestOffsetPagingWithRawSql()
public async Task TestOffsetPagingWithRawSqlAsync()
{
using (var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.Data.SqlClient;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using RepoDb.PagingPrimitives.CursorPaging;
using RepoDb.PagingPrimitives.OffsetPaging;
using RepoDb.SqlServer.PagingOperations.InMemoryPaging;
using StarWars.Characters.DbModels;


namespace RepoDb.SqlServer.PagingOperations.Tests
{
[TestClass]
public class PagingTestsUsingInMemoryPaging : BaseTest
{
[TestMethod]
public void TestCursorPagingWithInMemorySlicing()
{
var expectedTotalCount = 100;
var items = Enumerable.Range(1, expectedTotalCount).ToArray();

const int pageSize = 10;
int? totalCount = null;
int runningTotal = 0;
ICursorPageResults<int> page = null;
var pageCounter = 0;

do
{
page = items.SliceAsCursorPage(
first: pageSize,
after: page?.EndCursor,
before: null,
last: null,
includeTotalCount: totalCount is null
);

pageCounter++;
page.Should().NotBeNull();

if (pageCounter == 1)
{
page.HasNextPage.Should().BeTrue();
page.HasPreviousPage.Should().BeFalse();
}
else if (pageCounter == (int)Math.Ceiling((decimal)expectedTotalCount / pageSize))
{
page.HasNextPage.Should().BeFalse();
page.HasPreviousPage.Should().BeTrue();
}
else
{
page.HasNextPage.Should().BeTrue();
page.HasPreviousPage.Should().BeTrue();
}

var resultsList = page.CursorResults.ToList();
resultsList.Should().HaveCount(pageSize);

//Validate that we get Total Count only once, and on all following pages it is skipped and Null is returned as expected!
if (totalCount is null)
{
page.TotalCount.Should().Be(expectedTotalCount);
totalCount = page.TotalCount;
TestContext.WriteLine("*********************************************************");
TestContext.WriteLine($"[{totalCount}] Total Results to be processed...");
TestContext.WriteLine("*********************************************************");
}
else
{
page.TotalCount.Should().BeNull();
}

runningTotal += resultsList.Count;

TestContext.WriteLine("");
TestContext.WriteLine($"[{resultsList.Count}] Page Results:");
TestContext.WriteLine("----------------------------------------");
foreach (var result in resultsList)
{
var value = result.Entity;
value.Should().BeInRange(1, expectedTotalCount);
TestContext.WriteLine($"[{result.Cursor}] ==> ({value})");
}

} while (page.HasNextPage);

Assert.AreEqual(totalCount, runningTotal, "Total Count doesn't Match the final running total tally!");
}

[TestMethod]
public void TestOffsetPagingWithInMemorySlicing()
{
var expectedTotalCount = 100;
var items = Enumerable.Range(1, expectedTotalCount).ToArray();

const int pageSize = 10;
int? totalCount = null;
int runningTotal = 0;
IOffsetPageResults<int> page = null;
var pageCounter = 0;

do
{
page = items.SliceAsOffsetPage(
skip: page?.EndIndex,
take: pageSize,
includeTotalCount: totalCount is null
);

pageCounter++;
page.Should().NotBeNull();

if (pageCounter == 1)
{
page.HasNextPage.Should().BeTrue();
page.HasPreviousPage.Should().BeFalse();
}
else if (pageCounter == (int)Math.Ceiling((decimal)expectedTotalCount / pageSize))
{
page.HasNextPage.Should().BeFalse();
page.HasPreviousPage.Should().BeTrue();
}
else
{
page.HasNextPage.Should().BeTrue();
page.HasPreviousPage.Should().BeTrue();
}

var resultsList = page.Results.ToList();
resultsList.Should().HaveCount(pageSize);

//Validate that we get Total Count only once, and on all following pages it is skipped and Null is returned as expected!
if (totalCount is null)
{
page.TotalCount.Should().Be(expectedTotalCount);
totalCount = page.TotalCount;
TestContext.WriteLine("*********************************************************");
TestContext.WriteLine($"[{totalCount}] Total Results to be processed...");
TestContext.WriteLine("*********************************************************");
}
else
{
page.TotalCount.Should().BeNull();
}

runningTotal += resultsList.Count;

TestContext.WriteLine("");
TestContext.WriteLine($"[{resultsList.Count}] Page Results:");
TestContext.WriteLine("----------------------------------------");
foreach (var result in resultsList)
{
result.Should().BeInRange(1, expectedTotalCount);
TestContext.WriteLine($"[{result}]");
}

} while (page.HasNextPage);

Assert.AreEqual(totalCount, runningTotal, "Total Count doesn't Match the final running total tally!");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace RepoDb.SqlServer.PagingOperations.Tests
public class PagingTestsUsingQueryApiForObjectExpressions : BaseTest
{
[TestMethod]
public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApi()
public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApiAsync()
{
using var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false);

Expand Down Expand Up @@ -47,7 +47,7 @@ public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApi()
}

[TestMethod]
public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApiWithWhereExpression()
public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApiWithWhereExpressionAsync()
{
using var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false);

Expand Down Expand Up @@ -81,7 +81,7 @@ public async Task SimpleExampleOfCursorPagingThroughDatasetUsingQueryApiWithWher
}

[TestMethod]
public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApi()
public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApiAsync()
{
using var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false);

Expand Down Expand Up @@ -113,7 +113,7 @@ public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApi()
}

[TestMethod]
public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApiWithWhereExpression()
public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApiWithWhereExpressionAsync()
{
using var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false);

Expand Down Expand Up @@ -148,7 +148,7 @@ public async Task SimpleExampleOfOffsetPagingThroughDatasetUsingQueryApiWithWher
}

[TestMethod]
public async Task TestCursorPagingQueryApiSyntax()
public async Task TestCursorPagingQueryApiSyntaxAsync()
{
using (var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false))
{
Expand Down Expand Up @@ -206,7 +206,7 @@ public async Task TestCursorPagingQueryApiSyntax()
}

[TestMethod]
public async Task TestOffsetPagingQueryApiSyntax()
public async Task TestOffsetPagingQueryApiSyntaxAsync()
{
using (var sqlConnection = await CreateSqlConnectionAsync().ConfigureAwait(false))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\RepoDb.SqlServer.PagingOperations\RepoDbExtensions.SqlServer.PagingOperations.csproj" />
<ProjectReference Include="..\RepoDb.PagingPrimitives\RepoDbExtensions.PagingPrimitives.csproj" />
<ProjectReference Include="..\RepoDb.SqlServer.PagingOperations\RepoDbExtensions.SqlServer.PagingOperations.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ public static class IEnumerableInMemoryCursorPagingExtensions
/// Implement Linq in-memory slicing as described by Relay spec here:
/// https://relay.dev/graphql/connections.htm#sec-Pagination-algorithm
/// NOTE: This is primarily used for Unit Testing of in-memory data sets and is generally not recommended for production
/// use unless you always have 100% of all your data in-memory; this is because sorting must be done on a pre-filtered and/or
/// use unless you always have 100% of all your data in-memory. But there are valid use-cases such as if data is archived
/// in compressed format and is always retrieved into memory, etc.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="items"></param>
/// <param name="pagingArgs"></param>
/// <returns></returns>
public static ICursorPageResults<T> SliceAsCursorPage<T>(this IEnumerable<T> items, string after, int? first, string before, int? last)
where T : class
public static ICursorPageResults<T> SliceAsCursorPage<T>(this IEnumerable<T> items, string after, int? first, string before, int? last, bool includeTotalCount = true)
{
//Do nothing if there are no results...
if (!items.Any())
Expand All @@ -39,7 +39,12 @@ public static ICursorPageResults<T> SliceAsCursorPage<T>(this IEnumerable<T> ite

//NOTE: We MUST materialize this after applying index values to prevent ongoing increments...
IEnumerable<CursorResult<T>> slice = items
.Select((item, index) => CursorResult<T>.CreateIndexedCursor(item, CursorFactory.CreateCursor(index), index))
.Select((item, index) =>
{
//NOTE: We must increment index+1 to ensure our indexes are 1 based to simplify the rest of the algorithm...
var cursorIndex = index + 1;
return CursorResult<T>.CreateIndexedCursor(item, CursorFactory.CreateCursor(cursorIndex), cursorIndex);
})
.ToList();

int totalCount = slice.Count();
Expand Down Expand Up @@ -76,7 +81,7 @@ public static ICursorPageResults<T> SliceAsCursorPage<T>(this IEnumerable<T> ite

var cursorPageSlice = new CursorPageResults<T>(
results,
totalCount,
includeTotalCount ? totalCount : (int?)null,
hasPreviousPage: firstCursor?.CursorIndex > 1,
hasNextPage: lastCursor?.CursorIndex < totalCount
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public static class IEnumerableInMemoryOffsetPagingExtensions
/// Implement Linq in-memory slicing as described by Relay spec here:
/// https://relay.dev/graphql/connections.htm#sec-Pagination-algorithm
/// NOTE: This is primarily used for Unit Testing of in-memory data sets and is generally not recommended for production
/// use unless you always have 100% of all your data in-memory; this is because sorting must be done on a pre-filtered and/or
/// use unless you always have 100% of all your data in-memory. But there are valid use-cases such as if data is archived
/// in compressed format and is always retrieved into memory, etc.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="items"></param>
Expand All @@ -20,7 +21,6 @@ public static class IEnumerableInMemoryOffsetPagingExtensions
/// <param name="includeTotalCount"></param>
/// <returns></returns>
public static IOffsetPageResults<T> SliceAsOffsetPage<T>(this IEnumerable<T> items, int? skip, int? take, bool includeTotalCount = true)
where T : class
{
//Do nothing if there are no results...
if (items?.Any() != true)
Expand All @@ -40,7 +40,7 @@ public static IOffsetPageResults<T> SliceAsOffsetPage<T>(this IEnumerable<T> ite
if (hasNextPage) pagedResults.Remove(pagedResults.Last());

//NOTE: We only materialize the FULL Count if actually requested to do so...
var totalCount = includeTotalCount ? (int?)items.Count() : null;
var totalCount = includeTotalCount ? items.Count() : (int?)null;

//Wrap all results into a Offset Page Slice result with Total Count...
return new OffsetPageResults<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

<PropertyGroup>
<TargetFrameworks>netstandard2.0;netstandard2.1;net6.0;</TargetFrameworks>
<Version>1.1.5.5</Version>
<AssemblyVersion>1.1.5.5</AssemblyVersion>
<FileVersion>1.1.5.5</FileVersion>
<Version>1.1.5.6</Version>
<AssemblyVersion>1.1.5.6</AssemblyVersion>
<FileVersion>1.1.5.6</FileVersion>
<Authors>BBernard / CajunCoding</Authors>
<Company>CajunCoding</Company>
<Description>A set of extensions for working with modern pagination approaches such as Cursor based paging, as well as Offset based pagination, using the RepoDb ORM with Sql Server.</Description>
Expand All @@ -15,9 +15,11 @@
<PackageTags>repodb, paging, pagination, cursor, offset, skip, take, sorting, graphql, graph-ql, hotchocolate, dapper, sqlkata</PackageTags>
<PackageReleaseNotes>
Release Notes:
- Improve flexibility of base interface support for cursor navigation with non-generic ICursorPageNavigationInfo.
- Fix bug with in-memory cursor paging logic incorrectly indexing the results resulting in invalid page results.
- Removed unnecessary class constraint for both cursor and offset in-memory paging extensions making them more flexible.

Prior Release Notes:
- Improve flexibility of base interface support for cursor navigation with non-generic ICursorPageNavigationInfo.
- Simplify RetrieveTotalCount param name on ICursorParams.
- Fix SQL building bug not using Raw SQL as specified; improved integration tests to validate this better.
- Improved raw sql parameter name to be more consistent with RepDb naming conventions.
Expand Down

0 comments on commit 5acdcc4

Please sign in to comment.