Skip to content

Commit

Permalink
BREAKING CHANGE: Use long type for reading PunchCard statistics, just…
Browse files Browse the repository at this point in the history
… in case (#2949)

* Use long type for PunchCard statistics in case just in case there are some really big counts

* Fixes for public API surface and unit tests

---------

Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
  • Loading branch information
TimLovellSmith and nickfloyd authored Jan 8, 2025
1 parent ffd1b40 commit 7fa5b0f
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 20 deletions.
35 changes: 27 additions & 8 deletions Octokit.Tests/Clients/StatisticsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ public async Task RequestsCorrectUrl()
var expectedEndPoint = new Uri("repos/owner/name/stats/punch_card", UriKind.Relative);

var client = Substitute.For<IApiConnection>();
IReadOnlyList<int[]> data = new ReadOnlyCollection<int[]>(new[] { new[] { 2, 8, 42 } });
client.GetQueuedOperation<int[]>(expectedEndPoint, Args.CancellationToken)
IReadOnlyList<long[]> data = new ReadOnlyCollection<long[]>(new[] { new[] { 2L, 8, 42 } });
client.GetQueuedOperation<long[]>(expectedEndPoint, Args.CancellationToken)
.Returns(Task.FromResult(data));
var statisticsClient = new StatisticsClient(client);

Expand All @@ -436,8 +436,8 @@ public async Task RequestsCorrectUrlWithRepositoryId()
var expectedEndPoint = new Uri("repositories/1/stats/punch_card", UriKind.Relative);

var client = Substitute.For<IApiConnection>();
IReadOnlyList<int[]> data = new ReadOnlyCollection<int[]>(new[] { new[] { 2, 8, 42 } });
client.GetQueuedOperation<int[]>(expectedEndPoint, Args.CancellationToken)
IReadOnlyList<long[]> data = new ReadOnlyCollection<long[]>(new[] { new[] { 2L, 8, 42 } });
client.GetQueuedOperation<long[]>(expectedEndPoint, Args.CancellationToken)
.Returns(Task.FromResult(data));
var statisticsClient = new StatisticsClient(client);

Expand All @@ -456,9 +456,9 @@ public async Task RequestsCorrectUrlWithCancellationToken()
var cancellationToken = new CancellationToken();

var connection = Substitute.For<IApiConnection>();
IReadOnlyList<int[]> data = new ReadOnlyCollection<int[]>(new[] { new[] { 2, 8, 42 } });
IReadOnlyList<long[]> data = new ReadOnlyCollection<long[]>(new[] { new[] { 2L, 8, 42 } });

connection.GetQueuedOperation<int[]>(expectedEndPoint, cancellationToken)
connection.GetQueuedOperation<long[]>(expectedEndPoint, cancellationToken)
.Returns(Task.FromResult(data));
var client = new StatisticsClient(connection);

Expand All @@ -477,9 +477,9 @@ public async Task RequestsCorrectUrlWithRepositoryIdWithCancellationToken()
var cancellationToken = new CancellationToken();

var connection = Substitute.For<IApiConnection>();
IReadOnlyList<int[]> data = new ReadOnlyCollection<int[]>(new[] { new[] { 2, 8, 42 } });
IReadOnlyList<long[]> data = new ReadOnlyCollection<long[]>(new[] { new[] { 2L, 8, 42 } });

connection.GetQueuedOperation<int[]>(expectedEndPoint, cancellationToken)
connection.GetQueuedOperation<long[]>(expectedEndPoint, cancellationToken)
.Returns(Task.FromResult(data));
var client = new StatisticsClient(connection);

Expand All @@ -503,5 +503,24 @@ public async Task EnsureNonNullArguments()
await Assert.ThrowsAsync<ArgumentException>(() => client.GetPunchCard("owner", ""));
}
}

[Fact]
public async Task HandlesGreatBigCountsIfGetQueuedOperationTemplateParameterIsLong()
{
var expectedEndPoint = new Uri("repos/owner/name/stats/punch_card", UriKind.Relative);

var client = Substitute.For<IApiConnection>();
IReadOnlyList<long[]> data = new ReadOnlyCollection<long[]>(new [] { new [] { 2L, 8L, 42424242424242L } });
client.GetQueuedOperation<long[]>(expectedEndPoint, Args.CancellationToken)
.Returns(Task.FromResult(data));
var statisticsClient = new StatisticsClient(client);

var result = await statisticsClient.GetPunchCard("owner", "name");

Assert.Single(result.PunchPoints);
Assert.Equal(DayOfWeek.Tuesday, result.PunchPoints[0].DayOfWeek);
Assert.Equal(8, result.PunchPoints[0].HourOfTheDay);
Assert.Equal(42424242424242L, result.PunchPoints[0].CommitCount);
}
}
}
16 changes: 8 additions & 8 deletions Octokit.Tests/Models/PunchCardTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ public void DoesNotThrowExceptionWhenPunchPointsHaveCorrectFormat()
[Fact]
public void CanQueryCommitsForDayAndHour()
{
IList<int> point1 = new[] { 1, 0, 3 };
IList<int> point2 = new[] { 1, 1, 4 };
IList<int> point3 = new[] { 1, 2, 0 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1, point2, point3 };
IList<long> point1 = new long[] { 1, 0, 3 };
IList<long> point2 = new long[] { 1, 1, 4 };
IList<long> point3 = new long[] { 1, 2, 0 };
IEnumerable<IList<long>> points = new List<IList<long>> { point1, point2, point3 };

var punchCard = new PunchCard(points);

Expand All @@ -54,10 +54,10 @@ public void CanQueryCommitsForDayAndHour()
[Fact]
public void SetsPunchPointsAsReadOnlyDictionary()
{
IList<int> point1 = new[] { 1, 0, 3 };
IList<int> point2 = new[] { 1, 1, 4 };
IList<int> point3 = new[] { 1, 2, 0 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1, point2, point3 };
IList<long> point1 = new long[] { 1, 0, 3 };
IList<long> point2 = new long[] { 1, 1, 4 };
IList<long> point3 = new long[] { 1, 2, 0 };
IEnumerable<IList<long>> points = new List<IList<long>> { point1, point2, point3 };

var punchCard = new PunchCard(points);

Expand Down
4 changes: 2 additions & 2 deletions Octokit/Clients/StatisticsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public async Task<PunchCard> GetPunchCard(string owner, string name, Cancellatio
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));

var punchCardData = await ApiConnection.GetQueuedOperation<int[]>(ApiUrls.StatsPunchCard(owner, name), cancellationToken).ConfigureAwait(false);
var punchCardData = await ApiConnection.GetQueuedOperation<long[]>(ApiUrls.StatsPunchCard(owner, name), cancellationToken).ConfigureAwait(false);
return new PunchCard(punchCardData);
}

Expand All @@ -277,7 +277,7 @@ public async Task<PunchCard> GetPunchCard(string owner, string name, Cancellatio
[ManualRoute("GET", "/repositories/{id}/stats/punch_card")]
public async Task<PunchCard> GetPunchCard(long repositoryId, CancellationToken cancellationToken)
{
var punchCardData = await ApiConnection.GetQueuedOperation<int[]>(ApiUrls.StatsPunchCard(repositoryId), cancellationToken).ConfigureAwait(false);
var punchCardData = await ApiConnection.GetQueuedOperation<long[]>(ApiUrls.StatsPunchCard(repositoryId), cancellationToken).ConfigureAwait(false);
return new PunchCard(punchCardData);
}
}
Expand Down
9 changes: 8 additions & 1 deletion Octokit/Models/Response/PunchCard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ public PunchCard(IEnumerable<IList<int>> punchCardData)
punchCardData.Select(point => new PunchCardPoint(point)).ToList());
}

public PunchCard(IEnumerable<IList<long>> punchCardData)
{
Ensure.ArgumentNotNull(punchCardData, nameof(punchCardData));
PunchPoints = new ReadOnlyCollection<PunchCardPoint>(
punchCardData.Select(point => new PunchCardPoint(point)).ToList());
}

public PunchCard(IEnumerable<PunchCardPoint> punchPoints)
{
PunchPoints = punchPoints?.ToList();
Expand All @@ -36,7 +43,7 @@ public PunchCard(IEnumerable<PunchCardPoint> punchPoints)
/// <param name="dayOfWeek">The day of the week to query</param>
/// <param name="hourOfDay">The hour in 24 hour time. 0-23.</param>
/// <returns>The total number of commits made.</returns>
public int GetCommitCountFor(DayOfWeek dayOfWeek, int hourOfDay)
public long GetCommitCountFor(DayOfWeek dayOfWeek, int hourOfDay)
{
var punchPoint = PunchPoints.SingleOrDefault(point => point.DayOfWeek == dayOfWeek && point.HourOfTheDay == hourOfDay);
return punchPoint == null ? 0 : punchPoint.CommitCount;
Expand Down
14 changes: 13 additions & 1 deletion Octokit/Models/Response/PunchCardPoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ public PunchCardPoint(IList<int> punchPoint)
CommitCount = punchPoint[2];
}

public PunchCardPoint(IList<long> punchPoint)
{
Ensure.ArgumentNotNull(punchPoint, nameof(punchPoint));
if (punchPoint.Count != 3)
{
throw new ArgumentException("Daily punch card must only contain three data points.");
}
DayOfWeek = (DayOfWeek)punchPoint[0];
HourOfTheDay = (int)punchPoint[1];
CommitCount = punchPoint[2];
}

public PunchCardPoint(DayOfWeek dayOfWeek, int hourOfTheDay, int commitCount)
{
DayOfWeek = dayOfWeek;
Expand All @@ -31,7 +43,7 @@ public PunchCardPoint(DayOfWeek dayOfWeek, int hourOfTheDay, int commitCount)

public StringEnum<DayOfWeek> DayOfWeek { get; private set; }
public int HourOfTheDay { get; private set; }
public int CommitCount { get; private set; }
public long CommitCount { get; private set; }

internal string DebuggerDisplay
{
Expand Down

0 comments on commit 7fa5b0f

Please sign in to comment.