Skip to content

Commit c99858e

Browse files
authored
Fix multi-paged queries (#297)
* Take a copy of variables passed into subquery * Fix paged queries Previously instances of PagedSubquery would not check their returned PageInfo to see if they had any more pages of results to load and so each subquery would only get run at most once. It also didn't always call the `addResult` delegate to add a page of results to the parent collection. * Simplify code change and fix issue on subsequent loops * webhooks-methods.js is no longer the last repo in the org * Treat Bio like Email and coalesce it to an empty string
1 parent cd23fc1 commit c99858e

File tree

5 files changed

+30
-18
lines changed

5 files changed

+30
-18
lines changed

Octokit.GraphQL.Core/Core/PagedQuery.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ protected class Runner : IQueryRunner<TResult>, ISubqueryRunner
7676
readonly ResponseDeserializer deserializer = new ResponseDeserializer();
7777
Stack<IQueryRunner> subqueryRunners;
7878
Dictionary<ISubquery, List<Action<object>>> subqueryResultSinks;
79+
private bool hasMore;
7980

8081
public Runner(
8182
PagedQuery<TResult> owner,
@@ -88,7 +89,7 @@ public Runner(
8889
}
8990

9091
/// <inheritdoc />
91-
public TResult Result { get; private set; }
92+
public TResult Result { get; protected set; }
9293

9394
/// <inheritdoc />
9495
object IQueryRunner.Result => Result;
@@ -98,12 +99,14 @@ public Runner(
9899
/// <inheritdoc />
99100
public virtual async Task<bool> RunPage(CancellationToken cancellationToken = default)
100101
{
101-
if (subqueryRunners == null)
102+
if (subqueryRunners == null || hasMore)
102103
{
103-
subqueryRunners = new Stack<IQueryRunner>();
104-
subqueryResultSinks = new Dictionary<ISubquery, List<Action<object>>>();
104+
if (subqueryRunners == null)
105+
{
106+
subqueryRunners = new Stack<IQueryRunner>();
107+
}
105108

106-
// This is the first run, so run the master page.
109+
subqueryResultSinks = new Dictionary<ISubquery, List<Action<object>>>();
107110
var master = owner.MasterQuery;
108111
var data = await connection.Run(master.GetPayload(Variables), cancellationToken).ConfigureAwait(false);
109112
var json = deserializer.Deserialize(data);
@@ -133,8 +136,15 @@ public virtual async Task<bool> RunPage(CancellationToken cancellationToken = de
133136
}
134137
}
135138
}
139+
140+
if (owner is ISubquery ownerAsSubquery)
141+
{
142+
var pageInfo = ownerAsSubquery.PageInfo(json);
143+
hasMore = (bool)pageInfo["hasNextPage"];
144+
Variables["__after"] = (string)pageInfo["endCursor"];
145+
}
136146
}
137-
else
147+
else if (subqueryRunners.Any())
138148
{
139149
// Get the next subquery runner.
140150
var runner = subqueryRunners.Peek();
@@ -146,7 +156,7 @@ public virtual async Task<bool> RunPage(CancellationToken cancellationToken = de
146156
}
147157
}
148158

149-
return subqueryRunners.Count > 0;
159+
return subqueryRunners.Count > 0 || hasMore;
150160
}
151161

152162
/// <inheritdoc />

Octokit.GraphQL.Core/Core/PagedSubquery.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ public SubqueryRunner(
115115
string after,
116116
IDictionary<string, object> variables,
117117
Action<object> addResult)
118-
: base(owner, connection, variables ?? new Dictionary<string, object>())
118+
: base(owner, connection, variables?.ToDictionary(x => x.Key, x => x.Value) ??
119+
new Dictionary<string, object>())
119120
{
120121
Variables["__id"] = id;
121122
Variables["__after"] = after;
@@ -127,14 +128,15 @@ public override async Task<bool> RunPage(CancellationToken cancellationToken = d
127128
{
128129
var more = await base.RunPage(cancellationToken).ConfigureAwait(false);
129130

130-
if (!more)
131+
if (Result == null) return more;
132+
133+
foreach (var i in (IList)Result)
131134
{
132-
foreach (var i in (IList)Result)
133-
{
134-
addResult(i);
135-
}
135+
addResult(i);
136136
}
137137

138+
Result = default;
139+
138140
return more;
139141
}
140142
}

Octokit.GraphQL.IntegrationTests/Queries/IssueTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public async Task Can_AutoPage_Issues_Comments_With_Subquery()
201201
{
202202
var query = new Query()
203203
.Repository(owner: "octokit", name: "octokit.net")
204-
.Issues().AllPages(50)
204+
.Issues().AllPages(100)
205205
.Select(issue => new
206206
{
207207
issue.Id,
@@ -222,7 +222,7 @@ public async Task Can_AutoPage_Issues_With_Subquery()
222222
{
223223
var query = new Query()
224224
.Repository(owner: "octokit", name: "octokit.net")
225-
.Issues().AllPages(50)
225+
.Issues().AllPages(100)
226226
.Select(issue => new
227227
{
228228
issue.Id,

Octokit.GraphQL.IntegrationTests/Queries/RepositoryTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public async Task Query_Organization_Repositories_Select_Simple_Fragment()
175175

176176
var repositoryName = (await Connection.Run(query)).OrderByDescending(s => s).First();
177177

178-
Assert.Equal("webhooks-methods.js", repositoryName);
178+
Assert.Equal("webhooks.net", repositoryName);
179179
}
180180

181181
[IntegrationTest]
@@ -260,7 +260,7 @@ public async Task Query_Organization_Repositories_Select_Object_Fragment()
260260

261261
var testModelObject = (await Connection.Run(query)).OrderByDescending(s => s.StringField1).First();
262262

263-
Assert.Equal("webhooks-methods.js", testModelObject.StringField1);
263+
Assert.Equal("webhooks.net", testModelObject.StringField1);
264264
}
265265

266266
[IntegrationTest]

Octokit.GraphQL.IntegrationTests/Queries/ViewerTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public async Task Viewer_By_GraphyQL_Matches_Api()
5555
Assert.NotNull(graphqlUser);
5656

5757
Assert.Equal(apiUser.AvatarUrl.Split("?").First(), graphqlUser.AvatarUrl.Split("?").First());
58-
Assert.Equal(apiUser.Bio, graphqlUser.Bio);
58+
Assert.Equal(apiUser.Bio ?? string.Empty, graphqlUser.Bio);
5959
Assert.Equal(apiUser.Company, graphqlUser.Company);
6060

6161
Assert.Equal(apiUser.CreatedAt.ToUniversalTime(), graphqlUser.CreatedAt.ToUniversalTime());

0 commit comments

Comments
 (0)