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.

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

Added GetRateLimits to MiscellaneousClient #848

Merged
merged 6 commits into from
Jul 27, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions Octokit.Reactive/Clients/IObservableMiscellaneousClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,14 @@ public interface IObservableMiscellaneousClient
/// <param name="key"></param>
/// <returns>A <see cref="License" /> that includes the license key, text, and attributes of the license.</returns>
IObservable<License> GetLicense(string key);

/// <summary>
/// Gets API Rate Limits (API service rather than header info).
/// </summary>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>An <see cref="MiscRateLimits"/> of Rate Limits.</returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")]
IObservable<MiscRateLimits> GetRateLimits();

}
}
11 changes: 11 additions & 0 deletions Octokit.Reactive/Clients/ObservableMiscellaneousClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,16 @@ public IObservable<License> GetLicense(string key)
{
return _client.GetLicense(key).ToObservable();
}

/// <summary>
/// Gets API Rate Limits (API service rather than header info).
/// </summary>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>An <see cref="MiscRateLimits"/> of Rate Limits.</returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")]
public IObservable<MiscRateLimits> GetRateLimits()
{
return _client.GetRateLimits().ToObservable();
}
}
}
42 changes: 42 additions & 0 deletions Octokit.Tests.Integration/Clients/MiscellaneousClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,46 @@ public async Task CanRetrieveListOfLicenses()
Assert.Equal("MIT License", result.Name);
}
}

public class TheGetResourceRateLimitsMethod
{
[IntegrationTest]
public async Task CanRetrieveResourceRateLimits()
{
var github = Helper.GetAuthenticatedClient();

var result = await github.Miscellaneous.GetRateLimits();

// Test the high level object
Assert.NotNull(result);

// Test the resources level
Assert.NotNull(result.Resources);

// Test the core limits
Assert.NotNull(result.Resources.Core);
Assert.True(result.Resources.Core.Limit > 0);
Assert.True(result.Resources.Core.Remaining > -1);
Assert.True(result.Resources.Core.Remaining <= result.Resources.Core.Limit);
Assert.True(result.Resources.Core.Reset > 0);
Assert.NotNull(result.Resources.Core.ResetAsDateTimeOffset);

// Test the search limits
Assert.NotNull(result.Resources.Search);
Assert.True(result.Resources.Search.Limit > 0);
Assert.True(result.Resources.Search.Remaining > -1);
Assert.True(result.Resources.Search.Remaining <= result.Resources.Search.Limit);
Assert.True(result.Resources.Search.Reset > 0);
Assert.NotNull(result.Resources.Search.ResetAsDateTimeOffset);

// Test the depreciated rate limits
Assert.NotNull(result.Rate);
Assert.True(result.Rate.Limit > 0);
Assert.True(result.Rate.Remaining > -1);
Assert.True(result.Rate.Remaining <= result.Rate.Limit);
Assert.True(result.Resources.Search.Reset > 0);
Assert.NotNull(result.Resources.Search.ResetAsDateTimeOffset);

}
}
}
67 changes: 67 additions & 0 deletions Octokit.Tests/Clients/MiscellaneousClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using NSubstitute;
using Octokit.Internal;
using Xunit;
using System.Globalization;

namespace Octokit.Tests.Clients
{
Expand Down Expand Up @@ -58,6 +59,72 @@ public async Task RequestsTheEmojiEndpoint()
}
}

public class TheGetResourceRateLimitsMethod
{
[Fact]
public async Task RequestsTheRecourceRateLimitEndpoint()
{
IApiResponse<MiscRateLimits> response = new ApiResponse<MiscRateLimits>
(
new Response(),
new MiscRateLimits(
new ResourceRateLimits(
new ResourceRateLimit(5000, 4999, 1372700873),
new ResourceRateLimit(30, 18, 1372700873)
),
new ResourceRateLimit(100, 75, 1372700873)
)
);
var connection = Substitute.For<IConnection>();
connection.Get<MiscRateLimits>(Args.Uri, null, null).Returns(Task.FromResult(response));
var client = new MiscellaneousClient(connection);

var result = await client.GetRateLimits();

// Test the high level object
Assert.NotNull(result);
Copy link
Member

Choose a reason for hiding this comment

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

While I love the explicitness here, I think letting this fall through to the Assert.Equals is totally fine to simplify the tests...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify, is this the NotNull check? What about the other NotNull tests further down - lose them as well to make more readable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed the above was yes & yes


// Test the resource object
Assert.NotNull(result.Resources);

// Test the core limits
Assert.NotNull(result.Resources.Core);
Assert.Equal(5000, result.Resources.Core.Limit);
Assert.Equal(4999, result.Resources.Core.Remaining);
Assert.Equal(1372700873, result.Resources.Core.Reset);
var expectedReset = DateTimeOffset.ParseExact(
"Mon 01 Jul 2013 5:47:53 PM -00:00",
"ddd dd MMM yyyy h:mm:ss tt zzz",
CultureInfo.InvariantCulture);
Assert.Equal(expectedReset, result.Resources.Core.ResetAsDateTimeOffset);

// Test the search limits
Assert.NotNull(result.Resources.Search);
Assert.Equal(30, result.Resources.Search.Limit);
Assert.Equal(18, result.Resources.Search.Remaining);
Assert.Equal(1372700873, result.Resources.Search.Reset);
expectedReset = DateTimeOffset.ParseExact(
"Mon 01 Jul 2013 5:47:53 PM -00:00",
"ddd dd MMM yyyy h:mm:ss tt zzz",
CultureInfo.InvariantCulture);
Assert.Equal(expectedReset, result.Resources.Search.ResetAsDateTimeOffset);

// Test the depreciated rate limits
Assert.NotNull(result.Rate);
Assert.Equal(100, result.Rate.Limit);
Assert.Equal(75, result.Rate.Remaining);
Assert.Equal(1372700873, result.Rate.Reset);
expectedReset = DateTimeOffset.ParseExact(
"Mon 01 Jul 2013 5:47:53 PM -00:00",
"ddd dd MMM yyyy h:mm:ss tt zzz",
CultureInfo.InvariantCulture);
Assert.Equal(expectedReset, result.Rate.ResetAsDateTimeOffset);

connection.Received()
.Get<MiscRateLimits>(Arg.Is<Uri>(u => u.ToString() == "rate_limit"), null, null);
}
}

public class TheCtor
{
[Fact]
Expand Down
8 changes: 8 additions & 0 deletions Octokit/Clients/IMiscellaneousClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,13 @@ public interface IMiscellaneousClient
/// <param name="key"></param>
/// <returns>A <see cref="License" /> that includes the license key, text, and attributes of the license.</returns>
Task<License> GetLicense(string key);

/// <summary>
/// Gets API Rate Limits (API service rather than header info).
/// </summary>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>An <see cref="MiscRateLimits"/> of Rate Limits.</returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")]
Task<MiscRateLimits> GetRateLimits();
}
}
15 changes: 15 additions & 0 deletions Octokit/Clients/MiscellaneousClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,20 @@ public async Task<License> GetLicense(string key)
.ConfigureAwait(false);
return response.Body;
}

/// <summary>
/// Gets API Rate Limits (API service rather than header info).
/// </summary>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>An <see cref="MiscRateLimits"/> of Rate Limits.</returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")]
public async Task<MiscRateLimits> GetRateLimits()
{
var endpoint = new Uri("rate_limit", UriKind.Relative);
var response = await _connection.Get<MiscRateLimits>(endpoint, null, null).ConfigureAwait(false);
//var response = await _connection.Get<JsonObject>(endpoint, null, null).ConfigureAwait(false);
return response.Body;
//return null;
}
}
}
43 changes: 43 additions & 0 deletions Octokit/Models/Response/MiscRateLimits.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Octokit
{
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class MiscRateLimits
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to avoid abbreviations. So I'd call this MiscellaneousRateLimit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested

{
public MiscRateLimits() {}

public MiscRateLimits(ResourceRateLimits resources, ResourceRateLimit rate)
{
Ensure.ArgumentNotNull(resources, "resource");
Ensure.ArgumentNotNull(rate, "rate");

Resources = resources;
Rate = rate;
}

/// <summary>
/// Object of resources rate limits
/// </summary>
public ResourceRateLimits Resources { get; private set; }

/// <summary>
/// Legacy rate limit - to be depreciated - https://developer.github.com/v3/rate_limit/#deprecation-notice
/// </summary>
public ResourceRateLimit Rate { get; private set; }

internal string DebuggerDisplay
{
get
{
return Resources == null ? "No rates found" : String.Format(CultureInfo.InvariantCulture, Resources.DebuggerDisplay);
}
}
}
}
55 changes: 55 additions & 0 deletions Octokit/Models/Response/ResourceRateLimit.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using System;
using System.Diagnostics;
using System.Globalization;

using Octokit.Helpers;

namespace Octokit
{
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class ResourceRateLimit
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather we just use RateLimit and not this class. The name of this class is too similar to ResourceRateLimits. The data we show in this response and in the headers are the same and not likely to change (as we try and avoid breaking changes).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed now to use RateLimit

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure where we ended up with this - is it ResourceRateLimit or RateLimit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Three models in use:

  1. Originally MiscRateLimit -> Changed to MiscellaneousRateLimit to avoid abbreviation
  2. Originally ResourceRateLimits -> Changes to ResourceRateLimit
  3. Originally ResourceRateLimit -> Changes to use existing RateLimit

Hopefully that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

{
public ResourceRateLimit() { }

public ResourceRateLimit(int limit, int remaining, long reset)
{
Ensure.ArgumentNotNull(limit, "limit");
Ensure.ArgumentNotNull(remaining, "remaining");
Ensure.ArgumentNotNull(reset, "reset");

Limit = limit;
Remaining = remaining;
Reset = reset;
}


/// <summary>
/// The maximum number of requests that the consumer is permitted to make per hour.
/// </summary>
public int Limit { get; private set; }

/// <summary>
/// The number of requests remaining in the current rate limit window.
/// </summary>
public int Remaining { get; private set; }

/// <summary>
/// The date and time at which the current rate limit window resets - in UTC epoch seconds
/// </summary>
public long Reset { get; private set; }

/// <summary>
/// The date and time at which the current rate limit window resets - as DateTimeOffset
/// </summary>
public DateTimeOffset ResetAsDateTimeOffset { get { return Reset.FromUnixTime(); } }


internal string DebuggerDisplay
{
get
{
return String.Format(CultureInfo.InvariantCulture, "Limit {0}, Remaining {1}, Reset {2} ", Limit, Remaining, Reset);
}
}
}
}
42 changes: 42 additions & 0 deletions Octokit/Models/Response/ResourceRateLimits.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System;
using System.Diagnostics;
using System.Globalization;

using Octokit.Helpers;

namespace Octokit
{
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class ResourceRateLimits
Copy link
Contributor

Choose a reason for hiding this comment

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

Per my previous suggestion, I think this should be ResourceRateLimit and the property types should be RateLimit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

{
public ResourceRateLimits() {}

public ResourceRateLimits(ResourceRateLimit core, ResourceRateLimit search)
{
Ensure.ArgumentNotNull(core, "core");
Ensure.ArgumentNotNull(search, "search");

Core = core;
Search = search;
}

/// <summary>
/// Rate limits for core API (rate limit for everything except Search API)
/// </summary>
public ResourceRateLimit Core { get; private set; }

/// <summary>
/// Rate Limits for Search API
/// </summary>
public ResourceRateLimit Search { get; private set; }

internal string DebuggerDisplay
{
get
{
return String.Format(CultureInfo.InvariantCulture, "Core: {0}; Search: {1} ", Core.DebuggerDisplay, Search.DebuggerDisplay);
}
}
}

}
3 changes: 3 additions & 0 deletions Octokit/Octokit-Mono.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@
<Compile Include="Exceptions\TwoFactorAuthorizationException.cs" />
<Compile Include="Http\HttpMessageHandlerFactory.cs" />
<Compile Include="Models\Response\TeamMembership.cs" />
<Compile Include="Models\Response\ResourceRateLimits.cs" />
<Compile Include="Models\Response\ResourceRateLimit.cs" />
<Compile Include="Models\Response\MiscRateLimits.cs" />
</ItemGroup>
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
</Project>
5 changes: 4 additions & 1 deletion Octokit/Octokit-MonoAndroid.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
Expand Down Expand Up @@ -412,6 +412,9 @@
<Compile Include="Models\Request\RepositoryHookTestRequest.cs" />
<Compile Include="Http\HttpMessageHandlerFactory.cs" />
<Compile Include="Models\Response\TeamMembership.cs" />
<Compile Include="Models\Response\ResourceRateLimits.cs" />
<Compile Include="Models\Response\ResourceRateLimit.cs" />
<Compile Include="Models\Response\MiscRateLimits.cs" />
</ItemGroup>
<Import Project="$(MSBuildExtensionsPath)\Novell\Novell.MonoDroid.CSharp.targets" />
</Project>
5 changes: 4 additions & 1 deletion Octokit/Octokit-Monotouch.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
Expand Down Expand Up @@ -405,6 +405,9 @@
<Compile Include="Models\Request\RepositoryHookTestRequest.cs" />
<Compile Include="Http\HttpMessageHandlerFactory.cs" />
<Compile Include="Models\Response\TeamMembership.cs" />
<Compile Include="Models\Response\ResourceRateLimits.cs" />
<Compile Include="Models\Response\ResourceRateLimit.cs" />
<Compile Include="Models\Response\MiscRateLimits.cs" />
</ItemGroup>
<Import Project="$(MSBuildExtensionsPath)\Xamarin\iOS\Xamarin.MonoTouch.CSharp.targets" />
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
Expand Down
3 changes: 3 additions & 0 deletions Octokit/Octokit-Portable.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@
<Compile Include="Exceptions\TwoFactorAuthorizationException.cs" />
<Compile Include="Http\HttpMessageHandlerFactory.cs" />
<Compile Include="Models\Response\TeamMembership.cs" />
<Compile Include="Models\Response\ResourceRateLimits.cs" />
<Compile Include="Models\Response\ResourceRateLimit.cs" />
<Compile Include="Models\Response\MiscRateLimits.cs" />
</ItemGroup>
<ItemGroup>
<CodeAnalysisDictionary Include="..\CustomDictionary.xml">
Expand Down
Loading