From 4930fb13ce54e3aea4dd9445359663206e9f8ff8 Mon Sep 17 00:00:00 2001
From: Jcparkyn <51850908+Jcparkyn@users.noreply.github.com>
Date: Sat, 25 Nov 2023 19:57:53 +1100
Subject: [PATCH] BREAKING CHANGE: Change signature for GetNextPageArg

Also make LoadNextPageAsync return result, and throw if unable to fetch next page.
---
 .../Shared/InfinitePostList.razor             |   2 +-
 .../Experimental/UseEndpointInfinite.razor.cs |  69 ++--
 .../UseEndpointInfinite/PageResponse.cs       |   3 +
 .../UseEndpointInfiniteTestContent.razor      |   6 +
 .../UseEndpointInfiniteTests.razor            |  16 +
 .../UseEndpointInfiniteTests.razor.cs         | 325 ++++++++++++++++++
 test/Phetch.Tests/MockQueryFunction.cs        |  10 +
 7 files changed, 407 insertions(+), 24 deletions(-)
 create mode 100644 test/Phetch.Blazor.Tests/UseEndpointInfinite/PageResponse.cs
 create mode 100644 test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTestContent.razor
 create mode 100644 test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor
 create mode 100644 test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor.cs

diff --git a/samples/HackerNewsClient/Shared/InfinitePostList.razor b/samples/HackerNewsClient/Shared/InfinitePostList.razor
index c6c2040..f41b28f 100644
--- a/samples/HackerNewsClient/Shared/InfinitePostList.razor
+++ b/samples/HackerNewsClient/Shared/InfinitePostList.razor
@@ -2,7 +2,7 @@
 
 <UseEndpointInfinite Endpoint="Api.GetTopStories"
                      Arg="Arg"
-                     GetNextPageArg="(q, pageCount) => pageCount < q.Data?.NbPages ? q.Arg?.GetNextPageArgs() : null">
+                     GetNextPageArg="pages => (pages[^1].Arg?.GetNextPageArgs(), pages.Count < pages[^1].Data?.NbPages)">
     <ul class="post-list">
         @foreach (var query in context.Pages)
         {
diff --git a/src/Phetch.Blazor/Experimental/UseEndpointInfinite.razor.cs b/src/Phetch.Blazor/Experimental/UseEndpointInfinite.razor.cs
index 69bd3b8..2f04293 100644
--- a/src/Phetch.Blazor/Experimental/UseEndpointInfinite.razor.cs
+++ b/src/Phetch.Blazor/Experimental/UseEndpointInfinite.razor.cs
@@ -2,6 +2,7 @@
 
 using Microsoft.AspNetCore.Components;
 using Phetch.Core;
+using System.Diagnostics;
 
 /// <summary>
 /// An experimental component for creating "infinite scroll" features.
@@ -39,9 +40,9 @@ public Endpoint<TArg, TResult> Endpoint
     private TArg? _arg;
 
     /// <summary>
-    /// The argument to supply to the query. If not supplied, the query will not be run automatically.
+    /// The argument to supply to the query when fetching the first page. This is required.
     /// </summary>
-    [Parameter]
+    [Parameter, EditorRequired]
     public TArg Arg
     {
         get => _arg!;
@@ -58,11 +59,38 @@ public TArg Arg
     /// <summary>
     /// A function to choose the Arg for the next page, based on the last page and number of pages.
     /// </summary>
+    /// <remarks>
+    /// When this function is called, the following are guaranteed:
+    /// <list type="bullet">
+    /// <item/>
+    /// The <c>pages</c> list contains at least one page.
+    /// <item/>
+    /// The last page in the list has succeeded.
+    /// </list>
+    /// </remarks>
     [Parameter, EditorRequired]
     public GetNextPageFunc GetNextPageArg { get; set; } = null!;
 
+    /// <summary>
+    /// A function to choose the <see cref="Arg">Arg</see> for the next page, based on the last page
+    /// and number of pages.
+    /// </summary>
+    /// <param name="pages">
+    /// A list of query instances for the previous pages. When <see cref="GetNextPageArg"/> is
+    /// called, the following are guaranteed:
+    /// <list type="bullet">
+    /// <item/>
+    /// This list contains at least one page.
+    /// <item/>
+    /// The last page in the list has succeeded.
+    /// </list>
+    /// </param>
+    /// <returns>
+    /// A tuple containing the <see cref="Arg">Arg</see> for the next page, and a flag for whether
+    /// there are any more pages.
+    /// </returns>
     // Using an explicit delegate here, because otherwise the compiler doesn't understand nullable return type
-    public delegate TArg? GetNextPageFunc(Query<TArg, TResult> lastPage, int pageCount);
+    public delegate (TArg? nextPageArg, bool hasNextPage) GetNextPageFunc(IReadOnlyList<Query<TArg, TResult>> pages);
 
     private QueryOptions<TArg, TResult>? _options;
 
@@ -82,28 +110,23 @@ public QueryOptions<TArg, TResult>? Options
         }
     }
 
-    public async Task LoadNextPageAsync()
+    public async Task<TResult> LoadNextPageAsync()
     {
-        var lastQuery = _queries.LastOrDefault();
-        if (lastQuery is null)
-        {
-            // This might make more sense as a throw, it shouldn't be possible to get here in normal use.
-            ResetQueries();
-            return;
-        }
+        Debug.Assert(_isInitialized);
+        var lastQuery = _queries.LastOrDefault()
+            ?? throw new InvalidOperationException($"{nameof(LoadNextPageAsync)} was called before the component was initialized");
         if (!lastQuery.IsSuccess)
         {
-            return;
+            throw new InvalidOperationException("Cannot load next page because last page hasn't succeeded");
         }
-        var nextArg = GetNextPageArg(lastQuery, _queries.Count);
-        if (nextArg is null)
+        var (nextArg, hasNextPage) = GetNextPageArg(_queries);
+        if (!hasNextPage)
         {
-            return;
+            throw new InvalidOperationException($"Cannot load next page because {nameof(GetNextPageArg)} returned hasNextPage=false");
         }
         var newQuery = GetQuery(_endpoint);
         _queries.Add(newQuery);
-        // TODO: Return SetArgAsync result
-        await newQuery.SetArgAsync(nextArg).ConfigureAwait(false);
+        return await newQuery.SetArgAsync(nextArg!).ConfigureAwait(false);
     }
 
     protected override void OnInitialized()
@@ -134,7 +157,7 @@ private void UpdateQueries()
         for (var i = 0; i < _queries.Count; i++)
         {
             var query = _queries[i];
-            query.SetArg(nextArg);
+            query.SetArg(nextArg!); // TODO: Improve null handling
 
             if (!query.IsSuccess)
             {
@@ -142,13 +165,13 @@ private void UpdateQueries()
                 return;
             }
 
-            nextArg = GetNextPageArg(query, i + 1);
-            if (nextArg is null)
+            (nextArg, var hasNextPage) = GetNextPageArg(_queries.GetRange(0, i + 1));
+            if (!hasNextPage)
             {
                 RemoveQueriesAfter(i);
                 return;
             }
-            var nextIsCached = _endpoint!.GetCachedQuery(nextArg) is not null;
+            var nextIsCached = _endpoint!.GetCachedQuery(nextArg!) is not null;
 
             var isLastPage = i == _queries.Count - 1;
             if (isLastPage && query.IsSuccess && nextIsCached)
@@ -207,7 +230,7 @@ internal UseEndpointInfiniteContext(UseEndpointInfinite<TArg, TResult> component
         var lastPage = pages.LastOrDefault();
         HasNextPage = lastPage is not null
             && lastPage.IsSuccess
-            && component.GetNextPageArg(lastPage, pages.Count) != null;
+            && component.GetNextPageArg(pages).hasNextPage;
     }
 
     private readonly UseEndpointInfinite<TArg, TResult> _component;
@@ -217,5 +240,5 @@ internal UseEndpointInfiniteContext(UseEndpointInfinite<TArg, TResult> component
     public bool IsLoadingNextPage => Pages.Count > 0 && Pages[^1].IsFetching;
 
     public void LoadNextPage() => _ = _component.LoadNextPageAsync();
-    public Task LoadNextPageAsync() => _component.LoadNextPageAsync();
+    public Task<TResult> LoadNextPageAsync() => _component.LoadNextPageAsync();
 }
diff --git a/test/Phetch.Blazor.Tests/UseEndpointInfinite/PageResponse.cs b/test/Phetch.Blazor.Tests/UseEndpointInfinite/PageResponse.cs
new file mode 100644
index 0000000..6459ea8
--- /dev/null
+++ b/test/Phetch.Blazor.Tests/UseEndpointInfinite/PageResponse.cs
@@ -0,0 +1,3 @@
+namespace Phetch.Blazor.Tests.UseEndpointInfinite;
+
+public record PageResponse(int? NextCursor);
diff --git a/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTestContent.razor b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTestContent.razor
new file mode 100644
index 0000000..9f16c8e
--- /dev/null
+++ b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTestContent.razor
@@ -0,0 +1,6 @@
+@using Phetch.Blazor.Experimental
+
+@code {
+    [Parameter, EditorRequired]
+    public UseEndpointInfiniteContext<int, PageResponse> Context { get; set; } = null!;
+}
\ No newline at end of file
diff --git a/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor
new file mode 100644
index 0000000..946b738
--- /dev/null
+++ b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor
@@ -0,0 +1,16 @@
+@using Phetch.Blazor.Experimental
+@inherits TestContext
+
+@code {
+    RenderFragment EndpointFragment1(Endpoint<int, PageResponse> endpoint, int arg)
+    {
+        return @<UseEndpointInfinite
+            Endpoint="endpoint"
+            GetNextPageArg="pages => (pages[^1].Data!.NextCursor ?? 0, pages[^1].Data!.NextCursor is not null)"
+            Arg="arg"
+            Options="new()"
+        >
+            <UseEndpointInfiniteTestContent Context="@context"/>
+        </UseEndpointInfinite>;
+    }
+}
\ No newline at end of file
diff --git a/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor.cs b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor.cs
new file mode 100644
index 0000000..91a638f
--- /dev/null
+++ b/test/Phetch.Blazor.Tests/UseEndpointInfinite/UseEndpointInfiniteTests.razor.cs
@@ -0,0 +1,325 @@
+namespace Phetch.Blazor.Tests.UseEndpointInfinite;
+
+using Bunit.Rendering;
+using FluentAssertions;
+using FluentAssertions.Execution;
+using Phetch.Blazor.Experimental;
+using Phetch.Core;
+using Phetch.Tests;
+using System;
+using System.Threading.Tasks;
+
+public partial class UseEndpointInfiniteTests : TestContext
+{
+    [Fact]
+    public void Should_render_child_content()
+    {
+        var endpoint = new Endpoint<int, string>(
+            val => TestHelpers.ReturnAsync(val.ToString())
+        );
+        var cut = RenderComponent<UseEndpointInfinite<int, string>>(parameters => parameters
+            .Add(p => p.Endpoint, endpoint)
+            .Add(p => p.GetNextPageArg, pages => (pages.Count + 1, true))
+            .Add(p => p.Arg, 1)
+            .Add(p => p.ChildContent, p => "Hello, world!")
+        );
+        cut.MarkupMatches("Hello, world!");
+    }
+
+    [Fact]
+    public async Task Basic_usage()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(2);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query);
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        // Initial render (while loading)
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.Should().NotBeNull();
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeTrue();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsLoading.Should().BeTrue();
+                page.Arg.Should().Be(0);
+            });
+        }
+
+        await cut.InvokeAsync(() => qf.SetResult(0, new PageResponse(2)));
+
+        // Loaded first page
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeTrue();
+            ctx.IsLoadingNextPage.Should().BeFalse();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsSuccess.Should().BeTrue();
+                page.Data.Should().Be(new PageResponse(2));
+            });
+        }
+
+        // Explicit <Task> to ensure we don't wait for LoadNextPageAsync to complete
+        var loadTask = await cut.InvokeAsync<Task<PageResponse>>(content.Instance.Context.LoadNextPageAsync);
+
+        // Loading second and final page
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeTrue();
+            ctx.Pages.Should().SatisfyRespectively(
+                page =>
+                {
+                    page.IsSuccess.Should().BeTrue();
+                    page.Data.Should().Be(new PageResponse(2));
+                },
+                page =>
+                {
+                    page.IsLoading.Should().BeTrue();
+                    page.Arg.Should().Be(2);
+                });
+        }
+        await cut.InvokeAsync(() => qf.SetResult(1, new PageResponse(null)));
+        var loadTaskResult = await loadTask;
+        loadTaskResult.Should().Be(new PageResponse(null));
+
+        // Loaded final page
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeFalse();
+            ctx.Pages[1].IsSuccess.Should().BeTrue();
+            ctx.Pages[1].Data.Should().Be(new PageResponse(null));
+
+            // LoadNextPageAsync should fail
+            await content.Instance.Context.Invoking(c => c.LoadNextPageAsync()).Should()
+                .ThrowAsync<InvalidOperationException>()
+                .WithMessage("Cannot load next page because GetNextPageArg returned hasNextPage=false");
+            // LoadNextPage should swallow error
+            content.Instance.Context.Invoking(c => c.LoadNextPage()).Should().NotThrow();
+        }
+    }
+
+    [Fact]
+    public async Task Error_on_first_page()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(2);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query);
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        var ex = new ArgumentOutOfRangeException("boom");
+        await cut.InvokeAsync(() => qf.Sources[0].SetException(ex));
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeFalse();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsError.Should().BeTrue();
+                page.Error.Should().Be(ex);
+            });
+            // LoadNextPageAsync should fail
+            await content.Instance.Context.Invoking(c => c.LoadNextPageAsync()).Should()
+                .ThrowAsync<InvalidOperationException>()
+                .WithMessage("Cannot load next page because last page hasn't succeeded");
+        }
+
+
+        // Retry first page
+        // Explicit <Task> to ensure we don't wait for RefetchAsync to complete
+        await cut.InvokeAsync<Task>(content.Instance.Context.Pages[0].RefetchAsync);
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeTrue();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsLoading.Should().BeTrue();
+            });
+        }
+
+        // Successful first page
+        await cut.InvokeAsync(() => qf.SetResult(1, new PageResponse(2)));
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeTrue();
+            ctx.IsLoadingNextPage.Should().BeFalse();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsSuccess.Should().BeTrue();
+                page.Data.Should().Be(new PageResponse(2));
+            });
+        }
+    }
+
+    [Fact]
+    public async Task Should_refetch_when_Arg_changes_single_page()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(2);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query);
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        await cut.InvokeAsync(() => qf.SetResult(0, new PageResponse(null)));
+
+        cut.SetParametersAndRender(parameters => parameters
+            .Add(p => p.Arg, 7)
+        );
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeTrue();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsLoading.Should().BeTrue();
+                page.Arg.Should().Be(7);
+            });
+        }
+
+        await cut.InvokeAsync(() => qf.SetResult(1, new PageResponse(null)));
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsSuccess.Should().BeTrue();
+            });
+            qf.Calls.Should().Equal(0, 7);
+        }
+    }
+
+    [Fact]
+    public async Task Should_refetch_when_Arg_changes_multiple_pages()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(3);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query);
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        // Load first two pages
+        await cut.InvokeAsync(() => qf.SetResult(0, new PageResponse(2)));
+        await cut.InvokeAsync<Task>(content.Instance.Context.LoadNextPageAsync);
+        await cut.InvokeAsync(() => qf.SetResult(1, new PageResponse(null)));
+
+        content.Instance.Context.Pages.Should().HaveCount(2);
+
+        cut.SetParametersAndRender(parameters => parameters
+            .Add(p => p.Arg, 7)
+        );
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeTrue();
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsLoading.Should().BeTrue();
+                page.Arg.Should().Be(7);
+            });
+        }
+
+        await cut.InvokeAsync(() => qf.SetResult(2, new PageResponse(null)));
+
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.Pages.Should().SatisfyRespectively(page =>
+            {
+                page.IsSuccess.Should().BeTrue();
+            });
+            qf.Calls.Should().Equal(0, 2, 7);
+        }
+    }
+
+    [Fact]
+    public async Task Should_refetch_when_Arg_changes_precached()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(1);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query, new()
+        {
+            DefaultStaleTime = TimeSpan.MaxValue,
+        });
+        endpoint.UpdateQueryData(7, new PageResponse(9), addIfNotExists: true);
+        endpoint.UpdateQueryData(9, new PageResponse(null), addIfNotExists: true);
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        await cut.InvokeAsync(() => qf.SetResult(0, new PageResponse(null)));
+
+        cut.SetParametersAndRender(parameters => parameters
+            .Add(p => p.Arg, 7)
+        );
+
+        // It should detect that the first two pages are in the cache and load them immediately.
+        using (new AssertionScope())
+        {
+            var ctx = content.Instance.Context;
+            ctx.HasNextPage.Should().BeFalse();
+            ctx.IsLoadingNextPage.Should().BeFalse();
+            ctx.Pages.Should().SatisfyRespectively(
+                page =>
+                {
+                    page.IsSuccess.Should().BeTrue();
+                    page.Data.Should().Be(new PageResponse(9));
+                },
+                page =>
+                {
+                    page.IsSuccess.Should().BeTrue();
+                    page.Arg.Should().Be(9);
+                    page.Data.Should().Be(new PageResponse(null));
+                }
+            );
+            qf.Calls.Should().Equal(0);
+        }
+    }
+
+    [Fact]
+    public async Task Should_dispose_Query_when_disposed()
+    {
+        var qf = new MockQueryFunction<int, PageResponse>(1);
+        var endpoint = new Endpoint<int, PageResponse>(qf.Query, options: new()
+        {
+            CacheTime = TimeSpan.Zero,
+        });
+        using var cut = Render<UseEndpointInfinite<int, PageResponse>>(EndpointFragment1(endpoint, arg: 0));
+        var content = cut.FindComponent<UseEndpointInfiniteTestContent>();
+
+        await cut.InvokeAsync(() => qf.SetResult(0, new PageResponse(2)));
+
+        endpoint.GetCachedQuery(0).Should().NotBeNull();
+
+        DisposeComponents();
+        endpoint.GetCachedQuery(0).Should().BeNull();
+    }
+}
+
+static class RenderedFragmentExtensions
+{
+    /// Version of
+    /// <see cref="RenderedFragmentInvokeAsyncExtensions.InvokeAsync(IRenderedFragmentBase, Action)"/>
+    /// with a return value.
+    public static Task<T> InvokeAsync<T>(this IRenderedFragmentBase renderedFragment, Func<T> workItem)
+    {
+        ArgumentNullException.ThrowIfNull(renderedFragment);
+
+        return renderedFragment.Services.GetRequiredService<ITestRenderer>()
+            .Dispatcher.InvokeAsync(workItem);
+    }
+}
diff --git a/test/Phetch.Tests/MockQueryFunction.cs b/test/Phetch.Tests/MockQueryFunction.cs
index 4158cdf..69d005b 100644
--- a/test/Phetch.Tests/MockQueryFunction.cs
+++ b/test/Phetch.Tests/MockQueryFunction.cs
@@ -35,6 +35,16 @@ public async Task<TResult> Query(TArg arg)
         Calls.Add(arg);
         return await resultTask;
     }
+
+    public void SetResult(int call, TResult result)
+    {
+        Sources[call].SetResult(result);
+    }
+
+    public void SetResult(int call, Func<TArg, TResult> queryFn)
+    {
+        Sources[call].SetResult(queryFn(Calls[call]));
+    }
 }
 
 public class MockQueryFunction<TResult>(int numSources) : MockQueryFunction<Unit, TResult>(numSources)