Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for ThrowInt32OverflowException in Issue.Events.GetAllForRepository #2037

Closed
wants to merge 1 commit into from
+69 −69
Closed

Fix for ThrowInt32OverflowException in Issue.Events.GetAllForRepository #2037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Octokit.Reactive/Clients/IObservableIssuesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public interface IObservableIssuesClient
/// <param name="number">The issue number</param>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
IObservable<Issue> Get(string owner, string name, int number);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number used in this interface is not the one overflowing. Can we undo the changes in this file to simplify the diff?

IObservable<Issue> Get(string owner, string name, long number);

/// <summary>
/// Gets a single Issue by number.
Expand All @@ -67,7 +67,7 @@ public interface IObservableIssuesClient
/// <param name="number">The issue number</param>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
IObservable<Issue> Get(long repositoryId, int number);
IObservable<Issue> Get(long repositoryId, long number);

/// <summary>
/// Gets all open issues assigned to the authenticated user across all the authenticated user’s visible
Expand Down Expand Up @@ -304,7 +304,7 @@ public interface IObservableIssuesClient
/// <param name="number">The issue number</param>
/// <param name="issueUpdate">An <see cref="IssueUpdate"/> instance describing the changes to make to the issue
/// </param>
IObservable<Issue> Update(string owner, string name, int number, IssueUpdate issueUpdate);
IObservable<Issue> Update(string owner, string name, long number, IssueUpdate issueUpdate);

/// <summary>
/// Creates an issue for the specified repository. Any user with pull access to a repository can create an
Expand All @@ -315,7 +315,7 @@ public interface IObservableIssuesClient
/// <param name="number">The issue number</param>
/// <param name="issueUpdate">An <see cref="IssueUpdate"/> instance describing the changes to make to the issue
/// </param>
IObservable<Issue> Update(long repositoryId, int number, IssueUpdate issueUpdate);
IObservable<Issue> Update(long repositoryId, long number, IssueUpdate issueUpdate);

/// <summary>
/// Locks an issue for the specified repository. Issue owners and users with push access can lock an issue.
Expand All @@ -324,15 +324,15 @@ public interface IObservableIssuesClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
IObservable<Unit> Lock(string owner, string name, int number);
IObservable<Unit> Lock(string owner, string name, long number);

/// <summary>
/// Locks an issue for the specified repository. Issue owners and users with push access can lock an issue.
/// </summary>
/// <remarks>https://developer.github.com/v3/issues/#lock-an-issue</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
IObservable<Unit> Lock(long repositoryId, int number);
IObservable<Unit> Lock(long repositoryId, long number);

/// <summary>
/// Unlocks an issue for the specified repository. Issue owners and users with push access can unlock an issue.
Expand All @@ -341,14 +341,14 @@ public interface IObservableIssuesClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
IObservable<Unit> Unlock(string owner, string name, int number);
IObservable<Unit> Unlock(string owner, string name, long number);

/// <summary>
/// Unlocks an issue for the specified repository. Issue owners and users with push access can unlock an issue.
/// </summary>
/// <remarks>https://developer.github.com/v3/issues/#unlock-an-issue</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
IObservable<Unit> Unlock(long repositoryId, int number);
IObservable<Unit> Unlock(long repositoryId, long number);
}
}
12 changes: 6 additions & 6 deletions Octokit.Reactive/Clients/IObservableIssuesEventsClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public interface IObservableIssuesEventsClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
IObservable<EventInfo> GetAllForIssue(string owner, string name, int number);
IObservable<EventInfo> GetAllForIssue(string owner, string name, long number);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing for this file - number here is the issue number itself, no the EventInfo number that is being deserialized.


/// <summary>
/// Gets all events for the issue.
Expand All @@ -30,7 +30,7 @@ public interface IObservableIssuesEventsClient
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
IObservable<EventInfo> GetAllForIssue(long repositoryId, int number);
IObservable<EventInfo> GetAllForIssue(long repositoryId, long number);

/// <summary>
/// Gets all events for the issue.
Expand All @@ -42,7 +42,7 @@ public interface IObservableIssuesEventsClient
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
/// <param name="options">Options for changing the API response</param>
IObservable<EventInfo> GetAllForIssue(string owner, string name, int number, ApiOptions options);
IObservable<EventInfo> GetAllForIssue(string owner, string name, long number, ApiOptions options);

/// <summary>
/// Gets all events for the issue.
Expand All @@ -53,7 +53,7 @@ public interface IObservableIssuesEventsClient
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
/// <param name="options">Options for changing the API response</param>
IObservable<EventInfo> GetAllForIssue(long repositoryId, int number, ApiOptions options);
IObservable<EventInfo> GetAllForIssue(long repositoryId, long number, ApiOptions options);

/// <summary>
/// Gets all events for the repository.
Expand Down Expand Up @@ -106,7 +106,7 @@ public interface IObservableIssuesEventsClient
/// <param name="number">The event id</param>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
IObservable<IssueEvent> Get(string owner, string name, int number);
IObservable<IssueEvent> Get(string owner, string name, long number);

/// <summary>
/// Gets a single event
Expand All @@ -118,6 +118,6 @@ public interface IObservableIssuesEventsClient
/// <param name="number">The event id</param>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
IObservable<IssueEvent> Get(long repositoryId, int number);
IObservable<IssueEvent> Get(long repositoryId, long number);
}
}
16 changes: 8 additions & 8 deletions Octokit.Reactive/Clients/ObservableIssuesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public ObservableIssuesClient(IGitHubClient client)
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Issue> Get(string owner, string name, int number)
public IObservable<Issue> Get(string owner, string name, long number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -87,7 +87,7 @@ public IObservable<Issue> Get(string owner, string name, int number)
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Issue> Get(long repositoryId, int number)
public IObservable<Issue> Get(long repositoryId, long number)
{
return _client.Get(repositoryId, number).ToObservable();
}
Expand Down Expand Up @@ -448,7 +448,7 @@ public IObservable<Issue> Create(long repositoryId, NewIssue newIssue)
/// <param name="number">The issue number</param>
/// <param name="issueUpdate">An <see cref="IssueUpdate"/> instance describing the changes to make to the issue
/// </param>
public IObservable<Issue> Update(string owner, string name, int number, IssueUpdate issueUpdate)
public IObservable<Issue> Update(string owner, string name, long number, IssueUpdate issueUpdate)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -466,7 +466,7 @@ public IObservable<Issue> Update(string owner, string name, int number, IssueUpd
/// <param name="number">The issue number</param>
/// <param name="issueUpdate">An <see cref="IssueUpdate"/> instance describing the changes to make to the issue
/// </param>
public IObservable<Issue> Update(long repositoryId, int number, IssueUpdate issueUpdate)
public IObservable<Issue> Update(long repositoryId, long number, IssueUpdate issueUpdate)
{
Ensure.ArgumentNotNull(issueUpdate, nameof(issueUpdate));

Expand All @@ -480,7 +480,7 @@ public IObservable<Issue> Update(long repositoryId, int number, IssueUpdate issu
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Unit> Lock(string owner, string name, int number)
public IObservable<Unit> Lock(string owner, string name, long number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -494,7 +494,7 @@ public IObservable<Unit> Lock(string owner, string name, int number)
/// <remarks>https://developer.github.com/v3/issues/#lock-an-issue</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Unit> Lock(long repositoryId, int number)
public IObservable<Unit> Lock(long repositoryId, long number)
{
return _client.Lock(repositoryId, number).ToObservable();
}
Expand All @@ -506,7 +506,7 @@ public IObservable<Unit> Lock(long repositoryId, int number)
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Unit> Unlock(string owner, string name, int number)
public IObservable<Unit> Unlock(string owner, string name, long number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -520,7 +520,7 @@ public IObservable<Unit> Unlock(string owner, string name, int number)
/// <remarks>https://developer.github.com/v3/issues/#unlock-an-issue</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<Unit> Unlock(long repositoryId, int number)
public IObservable<Unit> Unlock(long repositoryId, long number)
{
return _client.Unlock(repositoryId, number).ToObservable();
}
Expand Down
12 changes: 6 additions & 6 deletions Octokit.Reactive/Clients/ObservableIssuesEventsClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public ObservableIssuesEventsClient(IGitHubClient client)
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<EventInfo> GetAllForIssue(string owner, string name, int number)
public IObservable<EventInfo> GetAllForIssue(string owner, string name, long number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -48,7 +48,7 @@ public IObservable<EventInfo> GetAllForIssue(string owner, string name, int numb
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
public IObservable<EventInfo> GetAllForIssue(long repositoryId, int number)
public IObservable<EventInfo> GetAllForIssue(long repositoryId, long number)
{
return GetAllForIssue(repositoryId, number, ApiOptions.None);
}
Expand All @@ -63,7 +63,7 @@ public IObservable<EventInfo> GetAllForIssue(long repositoryId, int number)
/// <param name="name">The name of the repository</param>
/// <param name="number">The issue number</param>
/// <param name="options">Options for changing the API response</param>
public IObservable<EventInfo> GetAllForIssue(string owner, string name, int number, ApiOptions options)
public IObservable<EventInfo> GetAllForIssue(string owner, string name, long number, ApiOptions options)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -81,7 +81,7 @@ public IObservable<EventInfo> GetAllForIssue(string owner, string name, int numb
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The issue number</param>
/// <param name="options">Options for changing the API response</param>
public IObservable<EventInfo> GetAllForIssue(long repositoryId, int number, ApiOptions options)
public IObservable<EventInfo> GetAllForIssue(long repositoryId, long number, ApiOptions options)
{
Ensure.ArgumentNotNull(options, nameof(options));

Expand Down Expand Up @@ -158,7 +158,7 @@ public IObservable<IssueEvent> GetAllForRepository(long repositoryId, ApiOptions
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The event id</param>
public IObservable<IssueEvent> Get(string owner, string name, int number)
public IObservable<IssueEvent> Get(string owner, string name, long number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand All @@ -174,7 +174,7 @@ public IObservable<IssueEvent> Get(string owner, string name, int number)
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The event id</param>
public IObservable<IssueEvent> Get(long repositoryId, int number)
public IObservable<IssueEvent> Get(long repositoryId, long number)
{
return _client.Get(repositoryId, number).ToObservable();
}
Expand Down
4 changes: 2 additions & 2 deletions Octokit.Tests.Integration/Clients/IssuesEventsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public async Task CanRetrieveIssueEventById()
var closed = await _issuesClient.Update(_context.RepositoryOwner, _context.RepositoryName, issue.Number, new IssueUpdate { State = ItemState.Closed });
Assert.NotNull(closed);
var issueEvents = await _issuesEventsClient.GetAllForRepository(_context.RepositoryOwner, _context.RepositoryName);
int issueEventId = issueEvents[0].Id;
long issueEventId = issueEvents[0].Id;

var issueEventLookupById = await _issuesEventsClient.Get(_context.RepositoryOwner, _context.RepositoryName, issueEventId);

Expand All @@ -418,7 +418,7 @@ public async Task CanRetrieveIssueEventByIdWithRepositoryId()
var closed = await _issuesClient.Update(_context.RepositoryOwner, _context.RepositoryName, issue.Number, new IssueUpdate { State = ItemState.Closed });
Assert.NotNull(closed);
var issueEvents = await _issuesEventsClient.GetAllForRepository(_context.Repository.Id);
int issueEventId = issueEvents[0].Id;
long issueEventId = issueEvents[0].Id;

var issueEventLookupById = await _issuesEventsClient.Get(_context.Repository.Id, issueEventId);

Expand Down
Loading