Skip to content

Commit

Permalink
Merge pull request #166 from octokit/fixes/sort-args
Browse files Browse the repository at this point in the history
Sort method arguments.
  • Loading branch information
StanleyGoldman committed Oct 31, 2018
2 parents 339854b + 75f6bc5 commit 5c5615d
Show file tree
Hide file tree
Showing 45 changed files with 940 additions and 179 deletions.
75 changes: 71 additions & 4 deletions Octokit.GraphQL.Core.Generation.UnitTests/EntityGenerationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,72 @@ public void Args_With_Default_Values_Come_After_Args_With_No_Default_Values()
CompareModel("Entity.cs", expected, result);
}

[Fact]
public void Paging_Args_Are_Ordered_Correctly()
{
var expected = FormatMemberTemplate(
"public IOther Foo(Arg<int>? first = null, Arg<string>? after = null, Arg<int>? last = null, Arg<string>? before = null) => " +
"this.CreateMethodCall(x => x.Foo(first, after, last, before), Test.Internal.StubIOther.Create);");

var model = new TypeModel
{
Name = "Entity",
Kind = TypeKind.Object,
Fields = new[]
{
new FieldModel
{
Name = "foo",
Type = TypeModel.Interface("Other"),
Args = new[]
{
new InputValueModel { Name = "after", Type = TypeModel.String() },
new InputValueModel { Name = "first", Type = TypeModel.Int() },
new InputValueModel { Name = "before", Type = TypeModel.String() },
new InputValueModel { Name = "last", Type = TypeModel.Int() },
}
},
}
};

var result = CodeGenerator.Generate(model, "Test", null);

CompareModel("Entity.cs", expected, result);
}

[Fact]
public void Non_Paging_Args_Ordered_Correctly()
{
var expected = FormatMemberTemplate(
"public IOther Foo(Arg<int>? first = null, Arg<string>? after = null, Arg<int>? bar = null, Arg<string>? foo = null) => " +
"this.CreateMethodCall(x => x.Foo(first, after, bar, foo), Test.Internal.StubIOther.Create);");

var model = new TypeModel
{
Name = "Entity",
Kind = TypeKind.Object,
Fields = new[]
{
new FieldModel
{
Name = "foo",
Type = TypeModel.Interface("Other"),
Args = new[]
{
new InputValueModel { Name = "after", Type = TypeModel.String() },
new InputValueModel { Name = "first", Type = TypeModel.Int() },
new InputValueModel { Name = "foo", Type = TypeModel.String() },
new InputValueModel { Name = "bar", Type = TypeModel.Int() },
}
},
}
};

var result = CodeGenerator.Generate(model, "Test", null);

CompareModel("Entity.cs", expected, result);
}

[Fact]
public void Generates_Doc_Comments_For_Class()
{
Expand Down Expand Up @@ -1589,16 +1655,17 @@ public void Generates_Doc_Comments_For_Method()
Type = TypeModel.Object("Other"),
Args = new[]
{
// Define these in the wrong order to check arg sorting.
new InputValueModel
{
Name = "arg1",
Description = "The first argument.",
Name = "arg2",
Description = "The second argument.\r\nWith a windows newline. Ending with a space. ",
Type = TypeModel.Int(),
},
new InputValueModel
{
Name = "arg2",
Description = "The second argument.\r\nWith a windows newline. Ending with a space. ",
Name = "arg1",
Description = "The first argument.",
Type = TypeModel.Int(),
},
}
Expand Down
2 changes: 1 addition & 1 deletion Octokit.GraphQL.Core.Generation/EntityGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private static string GenerateDocComments(FieldModel field, bool generate)

if (field.Args != null)
{
foreach (var arg in field.Args)
foreach (var arg in BuildUtilities.SortArgs(field.Args))
{
if (!string.IsNullOrWhiteSpace(arg.Description))
{
Expand Down
2 changes: 1 addition & 1 deletion Octokit.GraphQL.Core.Generation/InterfaceGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private static string GenerateDocComments(FieldModel field)

if (field.Args != null)
{
foreach (var arg in field.Args)
foreach (var arg in BuildUtilities.SortArgs(field.Args))
{
if (!string.IsNullOrWhiteSpace(arg.Description))
{
Expand Down
22 changes: 21 additions & 1 deletion Octokit.GraphQL.Core.Generation/Utilities/BuildUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ namespace Octokit.GraphQL.Core.Generation.Utilities
{
internal static class BuildUtilities
{
private static readonly string[] pagingFields = new[] { "first", "after", "last", "before" };

public static IEnumerable<InputValueModel> SortArgs(IEnumerable<InputValueModel> args)
{
return args.OrderBy(x => x, new DefaultValueComparer());
Expand All @@ -17,7 +19,25 @@ private class DefaultValueComparer : IComparer<InputValueModel>
{
public int Compare(InputValueModel x, InputValueModel y)
{
return DefaultValueWeight(x) - DefaultValueWeight(y);
var defaultValueWeightX = DefaultValueWeight(x);
var defaultValueWeightY = DefaultValueWeight(y);
var pagingX = Array.IndexOf(pagingFields, x.Name);
var pagingY = Array.IndexOf(pagingFields, y.Name);

if (defaultValueWeightX != defaultValueWeightY)
{
return DefaultValueWeight(x) - DefaultValueWeight(y);
}
else if (pagingX != pagingY)
{
if (pagingX == -1) pagingX = int.MaxValue;
if (pagingY == -1) pagingY = int.MaxValue;
return pagingX - pagingY;
}
else
{
return StringComparer.OrdinalIgnoreCase.Compare(x.Name, y.Name);
}
}

private static int DefaultValueWeight(InputValueModel i)
Expand Down
7 changes: 5 additions & 2 deletions Octokit.GraphQL.IntegrationTests/Mutations/MutationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public MutationTests()
_gitHubClient = GetV3GitHubClient();
_repository = _gitHubClient.Repository.Create(new NewRepository(_repoName)).Result;

var repositoryQuery = new Query().Repository(Helper.Username, _repoName)
var repositoryQuery = new Query()
.Repository(owner: Helper.Username, name: _repoName)
.Select(r => r.Id );

_repositoryId = Connection.Run(repositoryQuery).Result;
Expand Down Expand Up @@ -81,7 +82,9 @@ public void Create_And_Delete_Project()
[IntegrationTest]
public void Star_And_Unstar_Project()
{
var viewerHasStarredQuery = new Query().Repository(Helper.Username, _repoName).Select(repository => repository.ViewerHasStarred);
var viewerHasStarredQuery = new Query()
.Repository(owner: Helper.Username, name: _repoName)
.Select(repository => repository.ViewerHasStarred);

var viewerHasStarred = Connection.Run(viewerHasStarredQuery).Result;
Assert.False(viewerHasStarred);
Expand Down
14 changes: 10 additions & 4 deletions Octokit.GraphQL.IntegrationTests/Queries/ErrorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ public class ErrorTests : IntegrationTestBase
[IntegrationTest]
public async Task Should_Throw_Correct_Error_For_Name_Resolution_Failure()
{
var query = new GraphQL.Query().Repository("octokit", "octokit.net")
.Issues(first: 3).Nodes.Select(i => new
var query = new Query()
.Repository(owner: "octokit", name: "octokit.net")
.Issues(first: 3)
.Nodes
.Select(i => new
{
i.Title,
RepositoryName = i.Repository.Name,
Expand All @@ -38,8 +41,11 @@ public async Task Should_Throw_Correct_Error_For_Name_Resolution_Failure()
[IntegrationTest]
public async Task Should_Throw_Correct_Error_For_Invalid_Repository_Name()
{
var query = new GraphQL.Query().Repository("octokit", "bad_repository")
.Issues(first: 3).Nodes.Select(i => new
var query = new Query()
.Repository(owner: "octokit", name: "bad_repository")
.Issues(first: 3)
.Nodes
.Select(i => new
{
i.Title,
RepositoryName = i.Repository.Name,
Expand Down
24 changes: 16 additions & 8 deletions Octokit.GraphQL.IntegrationTests/Queries/IssueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ public class IssueTests : IntegrationTestBase
[IntegrationTest]
public async Task Should_Query_Issues_By_Repository()
{
var query = new GraphQL.Query().Repository("octokit", "octokit.net").Issues(first: 3).Nodes.Select(i => new
var query = new Query()
.Repository(owner: "octokit", name: "octokit.net")
.Issues(first: 3)
.Nodes
.Select(i => new
{
i.Title,
RepositoryName = i.Repository.Name,
Expand All @@ -30,7 +34,11 @@ public async Task Should_Query_Issues_By_Repository()
public async Task Should_Query_Issues_By_State_And_Repository()
{
var states = new[] { IssueState.Closed };
var query = new GraphQL.Query().Repository("octokit", "octokit.net").Issues(first: 3, states: states).Nodes.Select(i => new
var query = new Query()
.Repository(owner: "octokit", name: "octokit.net")
.Issues(first: 3, states: states)
.Nodes
.Select(i => new
{
i.Title,
i.State,
Expand All @@ -50,8 +58,8 @@ public async Task Should_Query_Issues_With_Variable()
{
var states = new[] { IssueState.Closed };
var query = new Query()
.Repository("octokit", "octokit.net")
.Issues(Var("first"), states: states)
.Repository(owner: "octokit", name: "octokit.net")
.Issues(first: Var("first"), states: states)
.Nodes
.Select(i => new
{
Expand All @@ -74,7 +82,7 @@ public async Task Should_Query_Issues_With_Variable()
public async Task Should_Query_Issue_Page_With_Author_Model()
{
var query = new Query()
.Repository("octokit", "octokit.net")
.Repository(owner: "octokit", name: "octokit.net")
.Issues(first: 100, after: Var("after"))
.Select(connection => new
{
Expand Down Expand Up @@ -109,7 +117,7 @@ public async Task Should_Query_Issue_Page_With_Author_Model()
public async Task Can_Manually_Page_Issue_Comments_By_Node_Id()
{
var masterQuery = new Query()
.Repository("octokit", "octokit.net")
.Repository(owner: "octokit", name: "octokit.net")
.Issue(405)
.Select(issue => new
{
Expand Down Expand Up @@ -158,7 +166,7 @@ public async Task Can_Manually_Page_Issue_Comments_By_Node_Id()
public async Task Can_AutoPage_Issue_Comments()
{
var query = new Query()
.Repository("octokit", "octokit.net")
.Repository(owner: "octokit", name: "octokit.net")
.Issue(405)
.Select(issue => new
{
Expand All @@ -175,7 +183,7 @@ public async Task Can_AutoPage_Issue_Comments()
public async Task Can_AutoPage_Issues_Comments()
{
var query = new Query()
.Repository("octokit", "octokit.net")
.Repository(owner: "octokit", name: "octokit.net")
.Issues().AllPages()
.Select(issue => new
{
Expand Down
26 changes: 15 additions & 11 deletions Octokit.GraphQL.IntegrationTests/Queries/PullRequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ public class PullRequestTests : IntegrationTestBase
[IntegrationTest]
public async Task Should_Query_Commits()
{
var query = new GraphQL.Query().Repository("octokit", "octokit.net").PullRequest(1).Commits(3).Nodes
var query = new Query()
.Repository(owner: "octokit", name: "octokit.net")
.PullRequest(number: 1)
.Commits(first: 3)
.Nodes
.Select(pullRequestCommit => new
{
pullRequestCommit.Commit.Id,
Expand All @@ -33,8 +37,8 @@ public async Task Should_Query_Commits()
public async Task Should_Query_Title_With_Vars()
{
var query = new Query()
.Repository(Var("owner"), Var("name"))
.PullRequest(Var("number"))
.Repository(owner: Var("owner"), name: Var("name"))
.PullRequest(number: Var("number"))
.Select(x => x.Title)
.Compile();

Expand All @@ -54,8 +58,8 @@ public async Task Should_Query_Title_With_Vars()
public async Task Can_Use_Conditional_In_BaseRef_Query()
{
var query = new Query()
.Repository("octokit", "octokit.net")
.PullRequest(1)
.Repository(owner: "octokit", name: "octokit.net")
.PullRequest(number: 1)
.Select(pr => pr.BaseRef != null ? pr.BaseRef.Name : null);

var result = await Connection.Run(query);
Expand All @@ -67,8 +71,8 @@ public async Task Can_Use_Conditional_In_BaseRef_Query()
public async Task Can_Use_Conditional_When_Selecting_Base_Repository_Owner()
{
var query = new Query()
.Repository("octokit", "octokit.net")
.PullRequest(1)
.Repository(owner: "octokit", name: "octokit.net")
.PullRequest(number: 1)
.Select(pr => pr.BaseRef != null ? pr.BaseRef.Repository.Owner.Login : null);

var result = await Connection.Run(query);
Expand All @@ -81,8 +85,8 @@ public async Task Can_AutoPage_Reviews_Comments()
{
// nodegit/nodegit#1331 has a review with >100 comments.
var query = new Query()
.Repository("nodegit", "nodegit")
.PullRequest(1331)
.Repository(owner: "nodegit", name: "nodegit")
.PullRequest(number: 1331)
.Select(pr => new
{
pr.Title,
Expand All @@ -106,8 +110,8 @@ public async Task Can_AutoPage_Reviews_Comments_2()
{
// Microsoft/MixedRealityToolkit-Unity#1884 has >100 reviews, one of which has >100 comments.
var query = new Query()
.Repository("Microsoft", "MixedRealityToolkit-Unity")
.PullRequest(1884)
.Repository(owner: "Microsoft", name: "MixedRealityToolkit-Unity")
.PullRequest(number: 1884)
.Select(pr => new
{
pr.Title,
Expand Down
Loading

0 comments on commit 5c5615d

Please sign in to comment.