From 2a2a4b6d224667db02d0c5eec610c94bee060b20 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 21:08:03 +1000 Subject: [PATCH 01/17] Add Protected Branches to Branch object - Create BranchProtection models - Add Protection element to Branch model - Adjust RepositoryClient GetBranch and GetBranches calls to set the special preview accepts header to enable Protected Branches API (currently in preview) - Adjust tests to include preview accepts header - Add test for Protection element --- .../Clients/BranchesClientTests.cs | 15 +++ .../Clients/RepositoriesClientTests.cs | 4 +- Octokit/Clients/RepositoriesClient.cs | 7 +- Octokit/Models/Response/Branch.cs | 6 ++ Octokit/Models/Response/BranchProtection.cs | 94 +++++++++++++++++++ Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 9 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 Octokit/Models/Response/BranchProtection.cs diff --git a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs index 74507996ce..ab61a236d2 100644 --- a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs @@ -24,5 +24,20 @@ public async Task ReturnsBranches() Assert.Equal(branches[0].Name, "master"); } } + + [IntegrationTest] + public async Task ReturnsBranchesWithProtectionStatus() + { + var github = Helper.GetAuthenticatedClient(); + + using (var context = await github.CreateRepositoryContext("public-repo")) + { + var branches = await github.Repository.GetAllBranches(context.Repository.Owner.Login, context.Repository.Name); + + Assert.NotEmpty(branches); + Assert.Equal(branches[0].Name, "master"); + Assert.NotNull(branches[0].Protection); + } + } } } \ No newline at end of file diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index 35b8a0bf77..98a68c5a0c 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -435,7 +435,7 @@ public void ReturnsBranches() client.GetAllBranches("owner", "name"); connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "repos/owner/name/branches")); + .GetAll(Arg.Is(u => u.ToString() == "repos/owner/name/branches"), null, "application/vnd.github.loki-preview+json"); } [Fact] @@ -565,7 +565,7 @@ public void GetsCorrectUrl() client.GetBranch("owner", "repo", "branch"); connection.Received() - .Get(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), null); + .Get(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), null, "application/vnd.github.loki-preview+json"); } [Fact] diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 6c29656aa6..0ebd979c3b 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -389,8 +389,8 @@ public Task> GetAllBranches(string owner, string name) Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(name, "name"); - var endpoint = ApiUrls.RepoBranches(owner, name); - return ApiConnection.GetAll(endpoint); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + return ApiConnection.GetAll(ApiUrls.RepoBranches(owner, name), null, previewAcceptsHeader); } @@ -502,7 +502,8 @@ public Task GetBranch(string owner, string repositoryName, string branch Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); - return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName)); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName), null, previewAcceptsHeader); } } } diff --git a/Octokit/Models/Response/Branch.cs b/Octokit/Models/Response/Branch.cs index 00500701fe..29507be8e2 100644 --- a/Octokit/Models/Response/Branch.cs +++ b/Octokit/Models/Response/Branch.cs @@ -20,6 +20,12 @@ public Branch(string name, GitReference commit) /// public string Name { get; protected set; } + /// + /// The details for this . + /// Note: this is a PREVIEW api: https://developer.github.com/changes/2015-11-11-protected-branches-api/ + /// + public BranchProtection Protection { get; protected set; } + /// /// The history for this . /// diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs new file mode 100644 index 0000000000..ea6c86615c --- /dev/null +++ b/Octokit/Models/Response/BranchProtection.cs @@ -0,0 +1,94 @@ +using System.Collections.Generic; +using System.Diagnostics; + +namespace Octokit +{ + /// + /// Protection details for a . + /// Note: this is a PREVIEW api: https://developer.github.com/changes/2015-11-11-protected-branches-api/ + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class BranchProtection + { + /// + /// Should this branch be protected or not + /// + public bool Enabled { get; set; } + + /// + /// The information for this . + /// + public RequiredStatusChecks RequiredStatusChecks { get; private set; } + + public BranchProtection() + { + RequiredStatusChecks = new RequiredStatusChecks(); + } + } + + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class RequiredStatusChecks + { + /// + /// Who required status checks apply to + /// + public EnforcementLevel EnforcementLevel { get; set; } + + /// + /// The list of status checks to require in order to merge into this + /// + public ICollection Contexts { get; private set; } + + /// + /// Adds the specified context to the required status checks. + /// + /// The name of the context. + public void AddContext(string name) + { + // lazily create the contexts array + if (Contexts == null) + { + Contexts = new List(); + } + + Contexts.Add(name); + } + + /// + /// Clears all the contexts. + /// + public void ClearContexts() + { + // lazily create the contexts array + if (Contexts == null) + { + Contexts = new List(); + } + else + { + Contexts.Clear(); + } + } + } + + /// + /// The enforcement levels that are available + /// + public enum EnforcementLevel + { + /// + /// Turn off required status checks for this . + /// + Off, + + /// + /// Required status checks will be enforeced for non-admins. + /// + NonAdmins, + + /// + /// Required status checks will be enforced for everyone (including admins). + /// + Everyone + } +} diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index a1ecc399ed..089d49df2a 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -142,6 +142,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 133952d82d..9ab4e5e225 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -236,6 +236,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 2f6e1698e3..2c45e3249b 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -243,6 +243,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index 58b1f65339..b893dac6ba 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -117,6 +117,7 @@ + From d7472d669965e7f454ca1d790559a762743cb7e0 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 21:09:29 +1000 Subject: [PATCH 02/17] Add Protect/Unprotect Branch functionality - Create BranchUpdate model - Add ProtectBranch and UnprotectBranch calls to RepositoryClient - Add tests --- .../Clients/RepositoriesClientTests.cs | 88 +++++++++++++++++++ Octokit/Clients/IRepositoriesClient.cs | 19 ++++ Octokit/Clients/RepositoriesClient.cs | 41 +++++++++ Octokit/Models/Request/BranchUpdate.cs | 22 +++++ Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 3 +- Octokit/Octokit.csproj | 1 + 8 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 Octokit/Models/Request/BranchUpdate.cs diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index 98a68c5a0c..65ed8274ac 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -717,5 +717,93 @@ public void GetsCorrectUrl() Arg.Any>()); } } + + public class TheProtectBranchMethod + { + [Fact] + public void GetsCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + var update = new BranchUpdate(); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.ProtectBranch("owner", "repo", "branch", update); + + connection.Received() + .Patch(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public void EnablesProtection() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + var update = new BranchUpdate(); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.ProtectBranch("owner", "repo", "branch", update); + + connection.Received() + .Patch(Arg.Any(), Arg.Is(b => b.Protection.Enabled == true), previewAcceptsHeader); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new RepositoriesClient(Substitute.For()); + var update = new BranchUpdate(); + + await Assert.ThrowsAsync(() => client.ProtectBranch(null, "repo", "branch", update)); + await Assert.ThrowsAsync(() => client.ProtectBranch("owner", null, "branch", update)); + await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", null, update)); + await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", "branch", null)); + await Assert.ThrowsAsync(() => client.ProtectBranch("", "repo", "branch", update)); + await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "", "branch", update)); + await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", "", update)); + } + } + + public class TheUnprotectBranchMethod + { + [Fact] + public void GetsCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.UnprotectBranch("owner", "repo", "branch"); + + connection.Received() + .Patch(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public void DisablesProtection() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.UnprotectBranch("owner", "repo", "branch"); + + connection.Received() + .Patch(Arg.Any(), Arg.Is(b => b.Protection.Enabled == false), previewAcceptsHeader); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new RepositoriesClient(Substitute.For()); + + await Assert.ThrowsAsync(() => client.UnprotectBranch(null, "repo", "branch")); + await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", null, "branch")); + await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "repo", null)); + await Assert.ThrowsAsync(() => client.UnprotectBranch("", "repo", "branch")); + await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "", "branch")); + await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "repo", "")); + } + } } } diff --git a/Octokit/Clients/IRepositoriesClient.cs b/Octokit/Clients/IRepositoriesClient.cs index c6774cd66f..67954bf426 100644 --- a/Octokit/Clients/IRepositoriesClient.cs +++ b/Octokit/Clients/IRepositoriesClient.cs @@ -327,5 +327,24 @@ public interface IRepositoriesClient /// New values to update the repository with /// The updated Task Edit(string owner, string name, RepositoryUpdate update); + + /// + /// Protects the specified branch with the values given in + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// + /// The updated + Task ProtectBranch(string owner, string repositoryName, string branchName, BranchUpdate update); + + /// + /// Unprotects the specified branch + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// The updated + Task UnprotectBranch(string owner, string repositoryName, string branchName); } } diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 0ebd979c3b..46f42730ad 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -505,5 +505,46 @@ public Task GetBranch(string owner, string repositoryName, string branch const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName), null, previewAcceptsHeader); } + + /// + /// Protects the specified branch with the values given in + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// + /// The updated + public Task ProtectBranch(string owner, string repositoryName, string branchName, BranchUpdate update) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); + Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); + Ensure.ArgumentNotNull(update, "update"); + + update.Protection.Enabled = true; + + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + return ApiConnection.Patch(ApiUrls.RepoBranch(owner, repositoryName, branchName), update, previewAcceptsHeader); + } + + /// + /// Unprotects the specified branch + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// The updated + public Task UnprotectBranch(string owner, string repositoryName, string branchName) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); + Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); + + var update = new BranchUpdate(); + update.Protection.Enabled = false; + + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + return ApiConnection.Patch(ApiUrls.RepoBranch(owner, repositoryName, branchName), update, previewAcceptsHeader); + } } } diff --git a/Octokit/Models/Request/BranchUpdate.cs b/Octokit/Models/Request/BranchUpdate.cs new file mode 100644 index 0000000000..d3b43f1854 --- /dev/null +++ b/Octokit/Models/Request/BranchUpdate.cs @@ -0,0 +1,22 @@ +using System.Diagnostics; + +namespace Octokit +{ + /// + /// Specifies the values used to update a . + /// Note: this is a PREVIEW api: https://developer.github.com/changes/2015-11-11-protected-branches-api/ + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class BranchUpdate + { + /// + /// The details + /// + public BranchProtection Protection { get; private set; } + + public BranchUpdate() + { + Protection = new BranchProtection(); + } + } +} diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 089d49df2a..c0baead2d2 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -98,6 +98,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 9ab4e5e225..07fc629447 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -176,6 +176,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 2c45e3249b..89c5d6b328 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -183,6 +183,7 @@ + @@ -430,4 +431,4 @@ --> - + \ No newline at end of file diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index b893dac6ba..1ac42ce625 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -92,6 +92,7 @@ + From ef8270614f0504040641ebf2851aa19dcac6e308 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 22:07:48 +1000 Subject: [PATCH 03/17] Add DebuggerDisplay property to model classes --- Octokit/Models/Request/BranchUpdate.cs | 12 +++++++++++- Octokit/Models/Response/BranchProtection.cs | 20 +++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Octokit/Models/Request/BranchUpdate.cs b/Octokit/Models/Request/BranchUpdate.cs index d3b43f1854..3a93dbac81 100644 --- a/Octokit/Models/Request/BranchUpdate.cs +++ b/Octokit/Models/Request/BranchUpdate.cs @@ -1,4 +1,6 @@ -using System.Diagnostics; +using System; +using System.Diagnostics; +using System.Globalization; namespace Octokit { @@ -18,5 +20,13 @@ public BranchUpdate() { Protection = new BranchProtection(); } + + internal string DebuggerDisplay + { + get + { + return String.Format(CultureInfo.InvariantCulture, "Protection: {0}", Protection.DebuggerDisplay); + } + } } } diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index ea6c86615c..5c0857c62d 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; namespace Octokit { @@ -24,6 +26,14 @@ public BranchProtection() { RequiredStatusChecks = new RequiredStatusChecks(); } + + internal string DebuggerDisplay + { + get + { + return String.Format(CultureInfo.InvariantCulture, "Enabled: {0}", Enabled); + } + } } [DebuggerDisplay("{DebuggerDisplay,nq}")] @@ -69,6 +79,14 @@ public void ClearContexts() Contexts.Clear(); } } + + internal string DebuggerDisplay + { + get + { + return String.Format(CultureInfo.InvariantCulture, "EnforcementLevel: {0} Contexts: {1}", EnforcementLevel.ToString(), Contexts.Count); + } + } } /// From 1af5f3b1b92d7cbcc1420ff2035fca2b1450e088 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 22:33:55 +1000 Subject: [PATCH 04/17] Change ProtectBranch/UnprotectBranch into a single EditBranch function Add tests for various protected/unprotected scenarios --- .../Clients/BranchesClientTests.cs | 104 ++++++++++++++++-- .../Clients/RepositoriesClientTests.cs | 74 ++----------- Octokit/Clients/IRepositoriesClient.cs | 15 +-- Octokit/Clients/RepositoriesClient.cs | 28 +---- 4 files changed, 110 insertions(+), 111 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs index ab61a236d2..00b18bebe0 100644 --- a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs @@ -22,22 +22,108 @@ public async Task ReturnsBranches() Assert.NotEmpty(branches); Assert.Equal(branches[0].Name, "master"); + Assert.NotNull(branches[0].Protection); } } + } - [IntegrationTest] - public async Task ReturnsBranchesWithProtectionStatus() + public class TheEditBranchesMethod + { + private readonly IRepositoriesClient _fixture; + private readonly RepositoryContext _context; + + public TheEditBranchesMethod() { var github = Helper.GetAuthenticatedClient(); + _context = github.CreateRepositoryContext("source-repo").Result; + _fixture = github.Repository; + } - using (var context = await github.CreateRepositoryContext("public-repo")) - { - var branches = await github.Repository.GetAllBranches(context.Repository.Owner.Login, context.Repository.Name); + public async Task CreateTheWorld() + { + // Set master branch to be protected, with some status checks + var update = new BranchUpdate(); + update.Protection.Enabled = true; + update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; + update.Protection.RequiredStatusChecks.AddContext("check1"); + update.Protection.RequiredStatusChecks.AddContext("check2"); - Assert.NotEmpty(branches); - Assert.Equal(branches[0].Name, "master"); - Assert.NotNull(branches[0].Protection); - } + var newBranch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); + } + + [IntegrationTest] + public async Task ProtectsBranch() + { + // Set master branch to be protected, with some status checks + var update = new BranchUpdate(); + update.Protection.Enabled = true; + update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; + update.Protection.RequiredStatusChecks.AddContext("check1"); + update.Protection.RequiredStatusChecks.AddContext("check2"); + update.Protection.RequiredStatusChecks.AddContext("check3"); + + var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); + + // Ensure a branch object was returned + Assert.NotNull(branch); + + // Retrieve master branch + branch = await _fixture.GetBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master"); + + // Assert the changes were made + Assert.Equal(branch.Protection.Enabled, true); + Assert.Equal(branch.Protection.RequiredStatusChecks.EnforcementLevel, EnforcementLevel.Everyone); + Assert.Equal(branch.Protection.RequiredStatusChecks.Contexts.Count, 3); + } + + [IntegrationTest] + public async Task RemoveStatusCheckEnforcement() + { + await CreateTheWorld(); + + // Clear status checks + var update = new BranchUpdate(); + update.Protection.Enabled = true; + update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Off; + update.Protection.RequiredStatusChecks.AddContext("check1"); + var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); + + // Ensure a branch object was returned + Assert.NotNull(branch); + + // Retrieve master branch + branch = await _fixture.GetBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master"); + + // Assert the changes were made + Assert.Equal(branch.Protection.Enabled, true); + Assert.Equal(branch.Protection.RequiredStatusChecks.EnforcementLevel, EnforcementLevel.Off); + Assert.Equal(branch.Protection.RequiredStatusChecks.Contexts.Count, 1); + } + + [IntegrationTest] + public async Task UnprotectsBranch() + { + await CreateTheWorld(); + + // Unprotect branch + var update = new BranchUpdate(); + update.Protection.Enabled = false; + + // Deliberately set Enforcement and Contexts to some values (these should be ignored) + update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; + update.Protection.RequiredStatusChecks.AddContext("check1"); + var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); + + // Ensure a branch object was returned + Assert.NotNull(branch); + + // Retrieve master branch + branch = await _fixture.GetBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master"); + + // Assert the branch is unprotected, and enforcement/contexts are cleared + Assert.Equal(branch.Protection.Enabled, false); + Assert.Equal(branch.Protection.RequiredStatusChecks.EnforcementLevel, EnforcementLevel.Off); + Assert.Equal(branch.Protection.RequiredStatusChecks.Contexts.Count, 0); } } } \ No newline at end of file diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index 65ed8274ac..2328ada626 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -718,7 +718,7 @@ public void GetsCorrectUrl() } } - public class TheProtectBranchMethod + public class TheEditBranchMethod { [Fact] public void GetsCorrectUrl() @@ -728,81 +728,25 @@ public void GetsCorrectUrl() var update = new BranchUpdate(); const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - client.ProtectBranch("owner", "repo", "branch", update); + client.EditBranch("owner", "repo", "branch", update); connection.Received() .Patch(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), Arg.Any(), previewAcceptsHeader); } - [Fact] - public void EnablesProtection() - { - var connection = Substitute.For(); - var client = new RepositoriesClient(connection); - var update = new BranchUpdate(); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - - client.ProtectBranch("owner", "repo", "branch", update); - - connection.Received() - .Patch(Arg.Any(), Arg.Is(b => b.Protection.Enabled == true), previewAcceptsHeader); - } - [Fact] public async Task EnsuresNonNullArguments() { var client = new RepositoriesClient(Substitute.For()); var update = new BranchUpdate(); - await Assert.ThrowsAsync(() => client.ProtectBranch(null, "repo", "branch", update)); - await Assert.ThrowsAsync(() => client.ProtectBranch("owner", null, "branch", update)); - await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", null, update)); - await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", "branch", null)); - await Assert.ThrowsAsync(() => client.ProtectBranch("", "repo", "branch", update)); - await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "", "branch", update)); - await Assert.ThrowsAsync(() => client.ProtectBranch("owner", "repo", "", update)); - } - } - - public class TheUnprotectBranchMethod - { - [Fact] - public void GetsCorrectUrl() - { - var connection = Substitute.For(); - var client = new RepositoriesClient(connection); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - - client.UnprotectBranch("owner", "repo", "branch"); - - connection.Received() - .Patch(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch"), Arg.Any(), previewAcceptsHeader); - } - - [Fact] - public void DisablesProtection() - { - var connection = Substitute.For(); - var client = new RepositoriesClient(connection); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - - client.UnprotectBranch("owner", "repo", "branch"); - - connection.Received() - .Patch(Arg.Any(), Arg.Is(b => b.Protection.Enabled == false), previewAcceptsHeader); - } - - [Fact] - public async Task EnsuresNonNullArguments() - { - var client = new RepositoriesClient(Substitute.For()); - - await Assert.ThrowsAsync(() => client.UnprotectBranch(null, "repo", "branch")); - await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", null, "branch")); - await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "repo", null)); - await Assert.ThrowsAsync(() => client.UnprotectBranch("", "repo", "branch")); - await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "", "branch")); - await Assert.ThrowsAsync(() => client.UnprotectBranch("owner", "repo", "")); + await Assert.ThrowsAsync(() => client.EditBranch(null, "repo", "branch", update)); + await Assert.ThrowsAsync(() => client.EditBranch("owner", null, "branch", update)); + await Assert.ThrowsAsync(() => client.EditBranch("owner", "repo", null, update)); + await Assert.ThrowsAsync(() => client.EditBranch("owner", "repo", "branch", null)); + await Assert.ThrowsAsync(() => client.EditBranch("", "repo", "branch", update)); + await Assert.ThrowsAsync(() => client.EditBranch("owner", "", "branch", update)); + await Assert.ThrowsAsync(() => client.EditBranch("owner", "repo", "", update)); } } } diff --git a/Octokit/Clients/IRepositoriesClient.cs b/Octokit/Clients/IRepositoriesClient.cs index 67954bf426..a82880ea65 100644 --- a/Octokit/Clients/IRepositoriesClient.cs +++ b/Octokit/Clients/IRepositoriesClient.cs @@ -329,22 +329,13 @@ public interface IRepositoriesClient Task Edit(string owner, string name, RepositoryUpdate update); /// - /// Protects the specified branch with the values given in + /// Edit the specified branch with the values given in /// /// The owner of the repository /// The name of the repository /// The name of the branch - /// + /// New values to update the branch with /// The updated - Task ProtectBranch(string owner, string repositoryName, string branchName, BranchUpdate update); - - /// - /// Unprotects the specified branch - /// - /// The owner of the repository - /// The name of the repository - /// The name of the branch - /// The updated - Task UnprotectBranch(string owner, string repositoryName, string branchName); + Task EditBranch(string owner, string repositoryName, string branchName, BranchUpdate update); } } diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 46f42730ad..cf5cbae8e0 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -507,42 +507,20 @@ public Task GetBranch(string owner, string repositoryName, string branch } /// - /// Protects the specified branch with the values given in + /// Edit the specified branch with the values given in /// /// The owner of the repository /// The name of the repository /// The name of the branch - /// + /// New values to update the branch with /// The updated - public Task ProtectBranch(string owner, string repositoryName, string branchName, BranchUpdate update) + public Task EditBranch(string owner, string repositoryName, string branchName, BranchUpdate update) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); Ensure.ArgumentNotNull(update, "update"); - update.Protection.Enabled = true; - - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - return ApiConnection.Patch(ApiUrls.RepoBranch(owner, repositoryName, branchName), update, previewAcceptsHeader); - } - - /// - /// Unprotects the specified branch - /// - /// The owner of the repository - /// The name of the repository - /// The name of the branch - /// The updated - public Task UnprotectBranch(string owner, string repositoryName, string branchName) - { - Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); - Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); - Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); - - var update = new BranchUpdate(); - update.Protection.Enabled = false; - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; return ApiConnection.Patch(ApiUrls.RepoBranch(owner, repositoryName, branchName), update, previewAcceptsHeader); } From 56e4f15e626ca234ad4537d2fc8e43f3c75e6b29 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 22:37:31 +1000 Subject: [PATCH 05/17] Synchronize Mono projects --- Octokit/Octokit-MonoAndroid.csproj | 4 +++- Octokit/Octokit-Monotouch.csproj | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index a3491a787c..3db4560e0d 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -418,6 +418,8 @@ + + - + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 26f3ba4e20..bc95ef514c 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -414,6 +414,8 @@ + + From 34bb67003083c379e1dee600c70c24fcee4ca771 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 23:05:30 +1000 Subject: [PATCH 06/17] Make variable names consistent and move EditBranch() function to preserve alphabetical ordering --- Octokit/Clients/IRepositoriesClient.cs | 6 ++-- Octokit/Clients/RepositoriesClient.cs | 38 +++++++++++++------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Octokit/Clients/IRepositoriesClient.cs b/Octokit/Clients/IRepositoriesClient.cs index a82880ea65..2243dcd94b 100644 --- a/Octokit/Clients/IRepositoriesClient.cs +++ b/Octokit/Clients/IRepositoriesClient.cs @@ -332,10 +332,10 @@ public interface IRepositoriesClient /// Edit the specified branch with the values given in /// /// The owner of the repository - /// The name of the repository - /// The name of the branch + /// The name of the repository + /// The name of the branch /// New values to update the branch with /// The updated - Task EditBranch(string owner, string repositoryName, string branchName, BranchUpdate update); + Task EditBranch(string owner, string name, string branch, BranchUpdate update); } } diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index cf5cbae8e0..06cbdebd27 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -161,6 +161,25 @@ public Task Edit(string owner, string name, RepositoryUpdate update) return ApiConnection.Patch(ApiUrls.Repository(owner, name), update); } + /// + /// Edit the specified branch with the values given in + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// New values to update the branch with + /// The updated + public Task EditBranch(string owner, string name, string branch, BranchUpdate update) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "repositoryName"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branchName"); + Ensure.ArgumentNotNull(update, "update"); + + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + return ApiConnection.Patch(ApiUrls.RepoBranch(owner, name, branch), update, previewAcceptsHeader); + } + /// /// Gets the specified repository. /// @@ -505,24 +524,5 @@ public Task GetBranch(string owner, string repositoryName, string branch const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName), null, previewAcceptsHeader); } - - /// - /// Edit the specified branch with the values given in - /// - /// The owner of the repository - /// The name of the repository - /// The name of the branch - /// New values to update the branch with - /// The updated - public Task EditBranch(string owner, string repositoryName, string branchName, BranchUpdate update) - { - Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); - Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); - Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); - Ensure.ArgumentNotNull(update, "update"); - - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - return ApiConnection.Patch(ApiUrls.RepoBranch(owner, repositoryName, branchName), update, previewAcceptsHeader); - } } } From 7033108634555083e4d82e3035cfbc6a10b2ef17 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 23:11:43 +1000 Subject: [PATCH 07/17] Add EditBranch() to Octokit.Reactive --- .../Clients/IObservableRepositoriesClient.cs | 10 ++++++++++ .../Clients/ObservableRepositoriesClient.cs | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs index 7ba8c055c2..f587e23557 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs @@ -260,6 +260,16 @@ public interface IObservableRepositoriesClient /// The updated IObservable Edit(string owner, string name, RepositoryUpdate update); + /// + /// Edit the specified branch with the values given in + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// New values to update the branch with + /// The updated + IObservable EditBranch(string owner, string name, string branch, BranchUpdate update); + /// /// A client for GitHub's Repo Collaborators. /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs index 9e7ac33c7d..9d0bb12831 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs @@ -380,6 +380,19 @@ public IObservable Edit(string owner, string name, RepositoryUpdate return _client.Edit(owner, name, update).ToObservable(); } + /// + /// Edit the specified branch with the values given in + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + /// New values to update the branch with + /// The updated + public IObservable EditBranch(string owner, string name, string branch, BranchUpdate update) + { + return _client.EditBranch(owner, name, branch, update).ToObservable(); + } + /// /// Compare two references in a repository /// From b1542e26637acc5a86dab146a41491dad2cdc40a Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sun, 13 Dec 2015 23:29:46 +1000 Subject: [PATCH 08/17] Add new Protection field to Branch constructor for consistency --- Octokit/Models/Response/Branch.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Octokit/Models/Response/Branch.cs b/Octokit/Models/Response/Branch.cs index 29507be8e2..fbed15fd4d 100644 --- a/Octokit/Models/Response/Branch.cs +++ b/Octokit/Models/Response/Branch.cs @@ -9,10 +9,11 @@ public class Branch { public Branch() { } - public Branch(string name, GitReference commit) + public Branch(string name, GitReference commit, BranchProtection protection) { Name = name; Commit = commit; + Protection = protection; } /// From 894a6bceb202de5e0933ef85720781215f78a8c7 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 11:43:15 +1000 Subject: [PATCH 09/17] Request object can have public setter --- Octokit/Models/Request/BranchUpdate.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Octokit/Models/Request/BranchUpdate.cs b/Octokit/Models/Request/BranchUpdate.cs index 3a93dbac81..47196b69f1 100644 --- a/Octokit/Models/Request/BranchUpdate.cs +++ b/Octokit/Models/Request/BranchUpdate.cs @@ -14,12 +14,7 @@ public class BranchUpdate /// /// The details /// - public BranchProtection Protection { get; private set; } - - public BranchUpdate() - { - Protection = new BranchProtection(); - } + public BranchProtection Protection { get; set; } internal string DebuggerDisplay { From 6b8312276f0afc562dff8ca611462cf20e3a85f7 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 11:43:56 +1000 Subject: [PATCH 10/17] Response objects need protected setters and ctors with parameters Rework integration test examples --- .../Clients/BranchesClientTests.cs | 41 +++++++++++-------- Octokit/Models/Response/BranchProtection.cs | 15 +++++-- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs index 00b18bebe0..944ff64fd9 100644 --- a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs @@ -42,11 +42,12 @@ public TheEditBranchesMethod() public async Task CreateTheWorld() { // Set master branch to be protected, with some status checks + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); + requiredStatusChecks.AddContext("check1"); + requiredStatusChecks.AddContext("check2"); + var update = new BranchUpdate(); - update.Protection.Enabled = true; - update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; - update.Protection.RequiredStatusChecks.AddContext("check1"); - update.Protection.RequiredStatusChecks.AddContext("check2"); + update.Protection = new BranchProtection(true, requiredStatusChecks); var newBranch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); } @@ -55,12 +56,13 @@ public async Task CreateTheWorld() public async Task ProtectsBranch() { // Set master branch to be protected, with some status checks + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); + requiredStatusChecks.AddContext("check1"); + requiredStatusChecks.AddContext("check2"); + requiredStatusChecks.AddContext("check3"); + var update = new BranchUpdate(); - update.Protection.Enabled = true; - update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; - update.Protection.RequiredStatusChecks.AddContext("check1"); - update.Protection.RequiredStatusChecks.AddContext("check2"); - update.Protection.RequiredStatusChecks.AddContext("check3"); + update.Protection = new BranchProtection(true, requiredStatusChecks); var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); @@ -81,11 +83,13 @@ public async Task RemoveStatusCheckEnforcement() { await CreateTheWorld(); - // Clear status checks + // Remove status check enforcement + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Off, null); + requiredStatusChecks.AddContext("check1"); + var update = new BranchUpdate(); - update.Protection.Enabled = true; - update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Off; - update.Protection.RequiredStatusChecks.AddContext("check1"); + update.Protection = new BranchProtection(true, requiredStatusChecks); + var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); // Ensure a branch object was returned @@ -106,12 +110,13 @@ public async Task UnprotectsBranch() await CreateTheWorld(); // Unprotect branch - var update = new BranchUpdate(); - update.Protection.Enabled = false; - // Deliberately set Enforcement and Contexts to some values (these should be ignored) - update.Protection.RequiredStatusChecks.EnforcementLevel = EnforcementLevel.Everyone; - update.Protection.RequiredStatusChecks.AddContext("check1"); + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); + requiredStatusChecks.AddContext("check1"); + + var update = new BranchUpdate(); + update.Protection = new BranchProtection(false, requiredStatusChecks); + var branch = await _fixture.EditBranch(_context.Repository.Owner.Login, _context.Repository.Name, "master", update); // Ensure a branch object was returned diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index 5c0857c62d..4414de8faa 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -15,16 +15,17 @@ public class BranchProtection /// /// Should this branch be protected or not /// - public bool Enabled { get; set; } + public bool Enabled { get; protected set; } /// /// The information for this . /// public RequiredStatusChecks RequiredStatusChecks { get; private set; } - public BranchProtection() + public BranchProtection(bool enabled, RequiredStatusChecks requiredStatusChecks) { - RequiredStatusChecks = new RequiredStatusChecks(); + Enabled = enabled; + RequiredStatusChecks = requiredStatusChecks; } internal string DebuggerDisplay @@ -42,13 +43,19 @@ public class RequiredStatusChecks /// /// Who required status checks apply to /// - public EnforcementLevel EnforcementLevel { get; set; } + public EnforcementLevel EnforcementLevel { get; protected set; } /// /// The list of status checks to require in order to merge into this /// public ICollection Contexts { get; private set; } + public RequiredStatusChecks(EnforcementLevel enforcementLevel, ICollection contexts) + { + EnforcementLevel = enforcementLevel; + Contexts = contexts; + } + /// /// Adds the specified context to the required status checks. /// From 7cf55f33983492c75312087ba930befd96a279c0 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 11:44:17 +1000 Subject: [PATCH 11/17] potential fix to convention test problems with private setter on IGenericList --- Octokit.Tests.Conventions/ModelTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Octokit.Tests.Conventions/ModelTests.cs b/Octokit.Tests.Conventions/ModelTests.cs index f126e8cd57..c0b2dd8cc3 100644 --- a/Octokit.Tests.Conventions/ModelTests.cs +++ b/Octokit.Tests.Conventions/ModelTests.cs @@ -85,6 +85,12 @@ public void ResponseModelsHaveReadOnlyCollections(Type modelType) continue; } + // Let's skip collections that only have a private Setter + if (property.SetMethod != null && property.SetMethod.IsPrivate) + { + continue; + } + mutableCollectionProperties.Add(property); } } From 87bb4669e49c5725f223fef282c9ed67c237a7cb Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 11:51:58 +1000 Subject: [PATCH 12/17] Remove AddContext() and ClearContexts() in favour of just using the List passed into the ctor --- .../Clients/BranchesClientTests.cs | 18 ++++------- Octokit/Models/Response/BranchProtection.cs | 31 ------------------- 2 files changed, 6 insertions(+), 43 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs index 944ff64fd9..ced759cfbd 100644 --- a/Octokit.Tests.Integration/Clients/BranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/BranchesClientTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; using Octokit; using Octokit.Tests.Integration; @@ -42,9 +43,7 @@ public TheEditBranchesMethod() public async Task CreateTheWorld() { // Set master branch to be protected, with some status checks - var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); - requiredStatusChecks.AddContext("check1"); - requiredStatusChecks.AddContext("check2"); + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, new List() { "check1", "check2" }); var update = new BranchUpdate(); update.Protection = new BranchProtection(true, requiredStatusChecks); @@ -56,10 +55,7 @@ public async Task CreateTheWorld() public async Task ProtectsBranch() { // Set master branch to be protected, with some status checks - var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); - requiredStatusChecks.AddContext("check1"); - requiredStatusChecks.AddContext("check2"); - requiredStatusChecks.AddContext("check3"); + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, new List() { "check1", "check2", "check3" }); var update = new BranchUpdate(); update.Protection = new BranchProtection(true, requiredStatusChecks); @@ -84,9 +80,8 @@ public async Task RemoveStatusCheckEnforcement() await CreateTheWorld(); // Remove status check enforcement - var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Off, null); - requiredStatusChecks.AddContext("check1"); - + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Off, new List() { "check1" }); + var update = new BranchUpdate(); update.Protection = new BranchProtection(true, requiredStatusChecks); @@ -111,8 +106,7 @@ public async Task UnprotectsBranch() // Unprotect branch // Deliberately set Enforcement and Contexts to some values (these should be ignored) - var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, null); - requiredStatusChecks.AddContext("check1"); + var requiredStatusChecks = new RequiredStatusChecks(EnforcementLevel.Everyone, new List() { "check1" }); var update = new BranchUpdate(); update.Protection = new BranchProtection(false, requiredStatusChecks); diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index 4414de8faa..f3e7518345 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -56,37 +56,6 @@ public RequiredStatusChecks(EnforcementLevel enforcementLevel, ICollection - /// Adds the specified context to the required status checks. - /// - /// The name of the context. - public void AddContext(string name) - { - // lazily create the contexts array - if (Contexts == null) - { - Contexts = new List(); - } - - Contexts.Add(name); - } - - /// - /// Clears all the contexts. - /// - public void ClearContexts() - { - // lazily create the contexts array - if (Contexts == null) - { - Contexts = new List(); - } - else - { - Contexts.Clear(); - } - } - internal string DebuggerDisplay { get From 053248dec6878b341f601c7ec224de15eafcc784 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 14:20:02 +1000 Subject: [PATCH 13/17] Added EditBranch integration test for Octokit.Reactive project --- .../ObservableRepositoriesClientTests.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs index 78c9ad1eae..c0e66b8636 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs @@ -463,6 +463,39 @@ public void CallsIntoClient() } } + public class TheEditBranchMethod + { + [Fact] + public async Task EnsuresArguments() + { + var github = Substitute.For(); + var nonreactiveClient = new RepositoriesClient(Substitute.For()); + github.Repository.Returns(nonreactiveClient); + var client = new ObservableRepositoriesClient(github); + var update = new BranchUpdate(); + + Assert.Throws(() => client.EditBranch(null, "repo", "branch", update)); + Assert.Throws(() => client.EditBranch("owner", null, "branch", update)); + Assert.Throws(() => client.EditBranch("owner", "repo", null, update)); + Assert.Throws(() => client.EditBranch("owner", "repo", "branch", null)); + Assert.Throws(() => client.EditBranch("", "repo", "branch", update)); + Assert.Throws(() => client.EditBranch("owner", "", "branch", update)); + Assert.Throws(() => client.EditBranch("owner", "repo", "", update)); + } + + [Fact] + public void CallsIntoClient() + { + var github = Substitute.For(); + var client = new ObservableRepositoriesClient(github); + var update = new BranchUpdate(); + + client.EditBranch("owner", "repo", "branch", update); + + github.Repository.Received(1).EditBranch("owner", "repo", "branch", update); + } + } + static IResponse CreateResponseWithApiInfo(IDictionary links) { var response = Substitute.For(); From 7b2bf36e4ea2b6266f150d111c1a93dd878fc8dc Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 20:44:28 +1000 Subject: [PATCH 14/17] Revert "potential fix to convention test problems with private setter on IGenericList" This reverts commit eb0106711e201d4046d668d91cccf84009f62fa5. --- Octokit.Tests.Conventions/ModelTests.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Octokit.Tests.Conventions/ModelTests.cs b/Octokit.Tests.Conventions/ModelTests.cs index c0b2dd8cc3..f126e8cd57 100644 --- a/Octokit.Tests.Conventions/ModelTests.cs +++ b/Octokit.Tests.Conventions/ModelTests.cs @@ -85,12 +85,6 @@ public void ResponseModelsHaveReadOnlyCollections(Type modelType) continue; } - // Let's skip collections that only have a private Setter - if (property.SetMethod != null && property.SetMethod.IsPrivate) - { - continue; - } - mutableCollectionProperties.Add(property); } } From 7be9dc083894236a7a158b32033d08689bcc8608 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Mon, 14 Dec 2015 21:18:26 +1000 Subject: [PATCH 15/17] Make Contexts IReadonlyList implemented as per CommitActivity Tidy up to be consistent with other model classes --- Octokit/Models/Response/BranchProtection.cs | 32 ++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index f3e7518345..31a2f3f1da 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Diagnostics; using System.Globalization; +using System.Linq; namespace Octokit { @@ -12,6 +14,14 @@ namespace Octokit [DebuggerDisplay("{DebuggerDisplay,nq}")] public class BranchProtection { + public BranchProtection() { } + + public BranchProtection(bool enabled, RequiredStatusChecks requiredStatusChecks) + { + Enabled = enabled; + RequiredStatusChecks = requiredStatusChecks; + } + /// /// Should this branch be protected or not /// @@ -22,12 +32,6 @@ public class BranchProtection /// public RequiredStatusChecks RequiredStatusChecks { get; private set; } - public BranchProtection(bool enabled, RequiredStatusChecks requiredStatusChecks) - { - Enabled = enabled; - RequiredStatusChecks = requiredStatusChecks; - } - internal string DebuggerDisplay { get @@ -40,6 +44,14 @@ internal string DebuggerDisplay [DebuggerDisplay("{DebuggerDisplay,nq}")] public class RequiredStatusChecks { + public RequiredStatusChecks() { } + + public RequiredStatusChecks(EnforcementLevel enforcementLevel, IEnumerable contexts) + { + EnforcementLevel = enforcementLevel; + Contexts = new ReadOnlyCollection(contexts.ToList()); + } + /// /// Who required status checks apply to /// @@ -48,13 +60,7 @@ public class RequiredStatusChecks /// /// The list of status checks to require in order to merge into this /// - public ICollection Contexts { get; private set; } - - public RequiredStatusChecks(EnforcementLevel enforcementLevel, ICollection contexts) - { - EnforcementLevel = enforcementLevel; - Contexts = contexts; - } + public IReadOnlyList Contexts { get; private set; } internal string DebuggerDisplay { From 489cfdbbf6090d6ad7be769b89b97335ce3b39dd Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sat, 19 Dec 2015 19:57:17 +1000 Subject: [PATCH 16/17] Add AcceptHeaders helper class to efine the ProtectedBranch preview API header in one place, change calls to use this --- Octokit/Clients/RepositoriesClient.cs | 9 +++------ Octokit/Helpers/AcceptHeaders.cs | 8 ++++++++ Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 6 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 Octokit/Helpers/AcceptHeaders.cs diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 06cbdebd27..1f9f55e7ed 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -176,8 +176,7 @@ public Task EditBranch(string owner, string name, string branch, BranchU Ensure.ArgumentNotNullOrEmptyString(branch, "branchName"); Ensure.ArgumentNotNull(update, "update"); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - return ApiConnection.Patch(ApiUrls.RepoBranch(owner, name, branch), update, previewAcceptsHeader); + return ApiConnection.Patch(ApiUrls.RepoBranch(owner, name, branch), update, AcceptHeaders.ProtectedBranchesApiPreview); } /// @@ -408,8 +407,7 @@ public Task> GetAllBranches(string owner, string name) Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(name, "name"); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - return ApiConnection.GetAll(ApiUrls.RepoBranches(owner, name), null, previewAcceptsHeader); + return ApiConnection.GetAll(ApiUrls.RepoBranches(owner, name), null, AcceptHeaders.ProtectedBranchesApiPreview); } @@ -521,8 +519,7 @@ public Task GetBranch(string owner, string repositoryName, string branch Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); Ensure.ArgumentNotNullOrEmptyString(branchName, "branchName"); - const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; - return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName), null, previewAcceptsHeader); + return ApiConnection.Get(ApiUrls.RepoBranch(owner, repositoryName, branchName), null, AcceptHeaders.ProtectedBranchesApiPreview); } } } diff --git a/Octokit/Helpers/AcceptHeaders.cs b/Octokit/Helpers/AcceptHeaders.cs new file mode 100644 index 0000000000..05ad379daa --- /dev/null +++ b/Octokit/Helpers/AcceptHeaders.cs @@ -0,0 +1,8 @@ +namespace Octokit +{ + public static class AcceptHeaders + { + public const string ProtectedBranchesApiPreview = "application/vnd.github.loki-preview+json"; + } +} + \ No newline at end of file diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index c0baead2d2..3b4d98aecf 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -96,6 +96,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 07fc629447..ee19d52516 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -130,6 +130,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 89c5d6b328..11a51e179e 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -149,6 +149,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index 1ac42ce625..a8ed3a779b 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -80,6 +80,7 @@ + From a6dd0af9e537e90b2c53f19b4ead822bd77de70c Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sat, 19 Dec 2015 20:03:58 +1000 Subject: [PATCH 17/17] Fix mono projects --- Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 1 + 2 files changed, 2 insertions(+) diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 3db4560e0d..83b337ac6a 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -420,6 +420,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index bc95ef514c..79d05bde5d 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -416,6 +416,7 @@ +