From 08a47afc204526124dbce0da7525db1f1cbd025a Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 29 Sep 2015 00:21:51 +1000 Subject: [PATCH 1/8] Add helper class to create a web hook. Fixes octokit/octokit.net#914 --- .../Models/Request/NewRepositoryWebHook.cs | 168 ++++++++++++++++++ Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 7 files changed, 174 insertions(+) create mode 100644 Octokit/Models/Request/NewRepositoryWebHook.cs diff --git a/Octokit/Models/Request/NewRepositoryWebHook.cs b/Octokit/Models/Request/NewRepositoryWebHook.cs new file mode 100644 index 0000000000..ee680f78b6 --- /dev/null +++ b/Octokit/Models/Request/NewRepositoryWebHook.cs @@ -0,0 +1,168 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; + +namespace Octokit +{ + /// + /// Creates a Webhook for the repository. + /// + /// + /// To create a webhook, the following fields are required by the config: + /// + /// + /// url + /// A required string defining the URL to which the payloads will be delivered. + /// + /// + /// content_type + /// + /// An optional string defining the media type used to serialize the payloads.Supported values include json and + /// form.The default is form. + /// + /// + /// + /// secret + /// + /// An optional string that’s passed with the HTTP requests as an X-Hub-Signature header. The value of this + /// header is computed as the HMAC hex digest of the body, using the secret as the key. + /// + /// + /// + /// insecure_ssl: + /// + /// An optional string that determines whether the SSL certificate of the host for url will be verified when + /// delivering payloads.Supported values include "0" (verification is performed) and "1" (verification is not + /// performed). The default is "0". + /// + /// + /// + /// + /// API: https://developer.github.com/v3/repos/hooks/#create-a-hook + /// + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class NewRepositoryWebHook : NewRepositoryHook + { + /// + /// Initializes a new instance of the class. + /// Using default values for ContentType, Secret and InsecureSsl. + /// + /// Use web for a webhook or use the name of a valid service. (See /hooks for the list of valid service names.) + /// Use "web" for a webhook or use the name of a valid service. (See + /// https://api.github.com/hooks for the list of valid service + /// names.) + /// + /// + /// Key/value pairs to provide settings for this hook. These settings vary between the services and are + /// defined in the github-services repository. Booleans are stored internally as “1” for true, and “0” for + /// false. Any JSON true/false values will be converted automatically. + /// + /// + /// A required string defining the URL to which the payloads will be delivered. + /// + public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url) : this(name, config, url, WebHookContentType.Form, string.Empty, false) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// Use web for a webhook or use the name of a valid service. (See /hooks for the list of valid service names.) + /// Use "web" for a webhook or use the name of a valid service. (See + /// https://api.github.com/hooks for the list of valid service + /// names.) + /// + /// + /// Key/value pairs to provide settings for this hook. These settings vary between the services and are + /// defined in the github-services repository. Booleans are stored internally as “1” for true, and “0” for + /// false. Any JSON true/false values will be converted automatically. + /// + /// + /// A required string defining the URL to which the payloads will be delivered. + /// + /// + /// An optional string defining the media type used to serialize the payloads. + /// Supported values include json and form. + /// The default is form. + /// + /// + /// An optional string that’s passed with the HTTP requests as an X-Hub-Signature header. + /// The value of this header is computed as the + /// + /// HMAC hex digest of the body, using the secret as the key + /// . + /// + /// + /// An optional string that determines whether the SSL certificate of the host for url will be verified when delivering payloads. + /// Supported values include "0" (verification is performed) and "1" (verification is not performed). + /// The default is "0". + /// + public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url, WebHookContentType contentType, string secret, bool insecureSsl) + : base(name, GetWebHookConfig(url, contentType, secret, insecureSsl, config)) + { + Ensure.ArgumentNotNullOrEmptyString(url, "url"); + + Url = url; + ContentType = contentType; + Secret = secret; + InsecureSsl = insecureSsl; + } + + static Dictionary GetWebHookConfig(string url, WebHookContentType contentType, string secret, bool insecureSsl, IReadOnlyDictionary config) + { + var webHookConfig = new Dictionary + { + { "url", url }, + { "content_type", contentType.ToParameter() }, + { "secret", secret }, + { "insecure_ssl", insecureSsl ? "1" : "0" } + }; + return config.Union(webHookConfig).ToDictionary(k => k.Key, v => v.Value); + } + + /// + /// Gets the URL of the hook to create. + /// + /// + /// The URL. + /// + public string Url { get; protected set; } + + /// + /// Gets the content type used to serialize the payload. + /// + /// + /// The content type. + /// + public WebHookContentType ContentType { get; set; } + + /// + /// Gets the secret used as the key for the HMAC hex digest + /// of the body passed with the HTTP requests as an X-Hub-Signature header. + /// + /// + /// The secret. + /// + public string Secret { get; set; } + + /// + /// Gets wether the SSL certificate of the host will be verified when + /// delivering payloads. + /// + /// + /// true if SSL certificate verification is not performed; + /// otherwise, false. + /// + public bool InsecureSsl { get; set; } + } + + /// + /// The supported content types for payload serialization. + /// + public enum WebHookContentType + { + Form, + Json + } +} diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 535992f9a6..ec28e2938a 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -406,6 +406,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index b3cd6ca899..555f68cd2f 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -413,6 +413,7 @@ + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 1b7629f554..9ffaf4bafa 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -409,6 +409,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index da5eba1682..5d4fb93279 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -403,6 +403,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index caef09227c..c5f7244136 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -410,6 +410,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index c5460f1f69..e559317dfc 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -91,6 +91,7 @@ + From c740639249e5de4fd2d2861e2b97d2a412924a37 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 29 Sep 2015 00:22:34 +1000 Subject: [PATCH 2/8] Add unit tests --- .../Models/NewRepositoryWebHookTests.cs | 71 +++++++++++++++++++ Octokit.Tests/Octokit.Tests.csproj | 1 + 2 files changed, 72 insertions(+) create mode 100644 Octokit.Tests/Models/NewRepositoryWebHookTests.cs diff --git a/Octokit.Tests/Models/NewRepositoryWebHookTests.cs b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs new file mode 100644 index 0000000000..efa4d3dde5 --- /dev/null +++ b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs @@ -0,0 +1,71 @@ +using System.Collections.Generic; +using Xunit; + +namespace Octokit.Tests.Models +{ + public class NewRepositoryWebHookTests + { + public class TheCtor + { + [Fact] + public void UsesDefaultValuesForDefaultConfig() + { + var create = new NewRepositoryWebHook("windowsazure", new Dictionary(), "http://test.com/example"); + + Assert.Equal(create.Config.Count, 4); + + Assert.True(create.Config.ContainsKey("url")); + Assert.True(create.Config.ContainsKey("content_type")); + Assert.True(create.Config.ContainsKey("secret")); + Assert.True(create.Config.ContainsKey("insecure_ssl")); + + Assert.Equal(create.Config["url"], "http://test.com/example"); + Assert.Equal(create.Config["content_type"], WebHookContentType.Form.ToParameter()); + Assert.Equal(create.Config["secret"], ""); + Assert.Equal(create.Config["insecure_ssl"], "0"); + + Assert.Equal(create.Url, "http://test.com/example"); + Assert.Equal(create.ContentType, WebHookContentType.Form); + Assert.Empty(create.Secret); + Assert.False(create.InsecureSsl); + } + + [Fact] + public void CombinesUserSpecifiedContentTypeWithConfig() + { + var config = new Dictionary + { + {"hostname", "http://hostname.url"}, + {"username", "username"}, + {"password", "password"} + }; + + var create = new NewRepositoryWebHook("windowsazure", config, "http://test.com/example", WebHookContentType.Json, string.Empty, true); + + Assert.Equal(create.Config.Count, 7); + + Assert.True(create.Config.ContainsKey("url")); + Assert.True(create.Config.ContainsKey("content_type")); + Assert.True(create.Config.ContainsKey("secret")); + Assert.True(create.Config.ContainsKey("insecure_ssl")); + + Assert.Equal(create.Config["url"], "http://test.com/example"); + Assert.Equal(create.Config["content_type"], WebHookContentType.Json.ToParameter()); + Assert.Equal(create.Config["secret"], ""); + Assert.Equal(create.Config["insecure_ssl"], "1"); + + Assert.True(create.Config.ContainsKey("hostname")); + Assert.Equal(create.Config["hostname"], config["hostname"]); + Assert.True(create.Config.ContainsKey("username")); + Assert.Equal(create.Config["username"], config["username"]); + Assert.True(create.Config.ContainsKey("password")); + Assert.Equal(create.Config["password"], config["password"]); + + Assert.Equal(create.Url, "http://test.com/example"); + Assert.Equal(create.ContentType, WebHookContentType.Json); + Assert.Empty(create.Secret); + Assert.True(create.InsecureSsl); + } + } + } +} diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index 9c00502159..804ebfc417 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -157,6 +157,7 @@ + From eff0c5d973f2e56bcd2b3fccaf6032dc15a97b4a Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 29 Sep 2015 00:23:25 +1000 Subject: [PATCH 3/8] Update existing integration test to use new web hook helper class --- .../Clients/RepositoryHooksClientTests.cs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs index 33d954ae95..cde8a366b5 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs @@ -63,15 +63,16 @@ public async Task CreateAWebHookForTestRepository() var repoName = Helper.MakeNameWithTimestamp("create-hooks-test"); var repository = await github.Repository.Create(new NewRepository(repoName) { AutoInit = true }); + var url = "http://test.com/example"; + var contentType = WebHookContentType.Json; + var secret = "53cr37"; var config = new Dictionary { - { "content_type", "json" }, - { "url", "http://test.com/example" }, { "hostname", "http://hostname.url" }, { "username", "username" }, { "password", "password" } }; - var parameters = new NewRepositoryHook("windowsazure", config) + var parameters = new NewRepositoryWebHook("windowsazure", config, url, contentType, secret, false) { Events = new[] { "push" }, Active = false @@ -80,6 +81,8 @@ public async Task CreateAWebHookForTestRepository() var hook = await github.Repository.Hooks.Create(Helper.Credentials.Login, repository.Name, parameters); var baseHookUrl = CreateExpectedBaseHookUrl(repository.Url, hook.Id); + var webHookConfig = CreateExpectedConfigDictionary(config, url, contentType, secret); + Assert.Equal("windowsazure", hook.Name); Assert.Equal(new[] { "push" }.ToList(), hook.Events.ToList()); Assert.Equal(baseHookUrl, hook.Url); @@ -87,11 +90,22 @@ public async Task CreateAWebHookForTestRepository() Assert.Equal(baseHookUrl + "/pings", hook.PingUrl); Assert.NotNull(hook.CreatedAt); Assert.NotNull(hook.UpdatedAt); - Assert.Equal(config.Keys, hook.Config.Keys); - Assert.Equal(config.Values, hook.Config.Values); + Assert.Equal(webHookConfig.Keys, hook.Config.Keys); + Assert.Equal(webHookConfig.Values, hook.Config.Values); Assert.Equal(false, hook.Active); } + Dictionary CreateExpectedConfigDictionary(Dictionary config, string url, WebHookContentType contentType, string secret) + { + return config.Union(new Dictionary + { + { "url", url }, + { "content_type", contentType.ToString().ToLowerInvariant() }, + { "secret", secret }, + { "insecure_ssl", "0" } + }).ToDictionary(k => k.Key, v => v.Value); + } + string CreateExpectedBaseHookUrl(string url, int id) { return url + "/hooks/" + id; From 8c15b6de97e26459e571716e17455572a554e9ee Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sun, 4 Oct 2015 19:21:54 +1000 Subject: [PATCH 4/8] Conform NewRepositoryWebHook to new request model guidelines --- .../Clients/RepositoryHooksClientTests.cs | 10 ++- .../Models/NewRepositoryWebHookTests.cs | 73 +++++++++-------- Octokit/Clients/RepositoryHooksClient.cs | 2 +- Octokit/Models/Request/NewRepositoryHook.cs | 5 ++ .../Models/Request/NewRepositoryWebHook.cs | 80 +++++-------------- 5 files changed, 74 insertions(+), 96 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs index cde8a366b5..a206232547 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryHooksClientTests.cs @@ -72,13 +72,15 @@ public async Task CreateAWebHookForTestRepository() { "username", "username" }, { "password", "password" } }; - var parameters = new NewRepositoryWebHook("windowsazure", config, url, contentType, secret, false) + var parameters = new NewRepositoryWebHook("windowsazure", config, url) { Events = new[] { "push" }, - Active = false + Active = false, + ContentType = contentType, + Secret = secret }; - var hook = await github.Repository.Hooks.Create(Helper.Credentials.Login, repository.Name, parameters); + var hook = await github.Repository.Hooks.Create(Helper.Credentials.Login, repository.Name, parameters.ToRequest()); var baseHookUrl = CreateExpectedBaseHookUrl(repository.Url, hook.Id); var webHookConfig = CreateExpectedConfigDictionary(config, url, contentType, secret); @@ -102,7 +104,7 @@ Dictionary CreateExpectedConfigDictionary(Dictionary k.Key, v => v.Value); } diff --git a/Octokit.Tests/Models/NewRepositoryWebHookTests.cs b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs index efa4d3dde5..119b9d0c13 100644 --- a/Octokit.Tests/Models/NewRepositoryWebHookTests.cs +++ b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs @@ -11,23 +11,23 @@ public class TheCtor public void UsesDefaultValuesForDefaultConfig() { var create = new NewRepositoryWebHook("windowsazure", new Dictionary(), "http://test.com/example"); - - Assert.Equal(create.Config.Count, 4); - - Assert.True(create.Config.ContainsKey("url")); - Assert.True(create.Config.ContainsKey("content_type")); - Assert.True(create.Config.ContainsKey("secret")); - Assert.True(create.Config.ContainsKey("insecure_ssl")); - - Assert.Equal(create.Config["url"], "http://test.com/example"); - Assert.Equal(create.Config["content_type"], WebHookContentType.Form.ToParameter()); - Assert.Equal(create.Config["secret"], ""); - Assert.Equal(create.Config["insecure_ssl"], "0"); - Assert.Equal(create.Url, "http://test.com/example"); Assert.Equal(create.ContentType, WebHookContentType.Form); Assert.Empty(create.Secret); Assert.False(create.InsecureSsl); + + var request = create.ToRequest(); + Assert.Equal(request.Config.Count, 4); + + Assert.True(request.Config.ContainsKey("url")); + Assert.True(request.Config.ContainsKey("content_type")); + Assert.True(request.Config.ContainsKey("secret")); + Assert.True(request.Config.ContainsKey("insecure_ssl")); + + Assert.Equal(request.Config["url"], "http://test.com/example"); + Assert.Equal(request.Config["content_type"], WebHookContentType.Form.ToParameter()); + Assert.Equal(request.Config["secret"], ""); + Assert.Equal(request.Config["insecure_ssl"], "False"); } [Fact] @@ -40,31 +40,38 @@ public void CombinesUserSpecifiedContentTypeWithConfig() {"password", "password"} }; - var create = new NewRepositoryWebHook("windowsazure", config, "http://test.com/example", WebHookContentType.Json, string.Empty, true); - - Assert.Equal(create.Config.Count, 7); - - Assert.True(create.Config.ContainsKey("url")); - Assert.True(create.Config.ContainsKey("content_type")); - Assert.True(create.Config.ContainsKey("secret")); - Assert.True(create.Config.ContainsKey("insecure_ssl")); - - Assert.Equal(create.Config["url"], "http://test.com/example"); - Assert.Equal(create.Config["content_type"], WebHookContentType.Json.ToParameter()); - Assert.Equal(create.Config["secret"], ""); - Assert.Equal(create.Config["insecure_ssl"], "1"); - - Assert.True(create.Config.ContainsKey("hostname")); - Assert.Equal(create.Config["hostname"], config["hostname"]); - Assert.True(create.Config.ContainsKey("username")); - Assert.Equal(create.Config["username"], config["username"]); - Assert.True(create.Config.ContainsKey("password")); - Assert.Equal(create.Config["password"], config["password"]); + var create = new NewRepositoryWebHook("windowsazure", config, "http://test.com/example") + { + ContentType = WebHookContentType.Json, + Secret = string.Empty, + InsecureSsl = true + }; Assert.Equal(create.Url, "http://test.com/example"); Assert.Equal(create.ContentType, WebHookContentType.Json); Assert.Empty(create.Secret); Assert.True(create.InsecureSsl); + + var request = create.ToRequest(); + + Assert.Equal(request.Config.Count, 7); + + Assert.True(request.Config.ContainsKey("url")); + Assert.True(request.Config.ContainsKey("content_type")); + Assert.True(request.Config.ContainsKey("secret")); + Assert.True(request.Config.ContainsKey("insecure_ssl")); + + Assert.Equal(request.Config["url"], "http://test.com/example"); + Assert.Equal(request.Config["content_type"], WebHookContentType.Json.ToParameter()); + Assert.Equal(request.Config["secret"], ""); + Assert.Equal(request.Config["insecure_ssl"], true.ToString()); + + Assert.True(request.Config.ContainsKey("hostname")); + Assert.Equal(request.Config["hostname"], config["hostname"]); + Assert.True(request.Config.ContainsKey("username")); + Assert.Equal(request.Config["username"], config["username"]); + Assert.True(request.Config.ContainsKey("password")); + Assert.Equal(request.Config["password"], config["password"]); } } } diff --git a/Octokit/Clients/RepositoryHooksClient.cs b/Octokit/Clients/RepositoryHooksClient.cs index d4d30e55ca..458a1596f5 100644 --- a/Octokit/Clients/RepositoryHooksClient.cs +++ b/Octokit/Clients/RepositoryHooksClient.cs @@ -54,7 +54,7 @@ public Task Create(string owner, string repositoryName, NewRepos Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); Ensure.ArgumentNotNull(hook, "hook"); - return ApiConnection.Post(ApiUrls.RepositoryHooks(owner, repositoryName), hook); + return ApiConnection.Post(ApiUrls.RepositoryHooks(owner, repositoryName), hook.ToRequest()); } /// diff --git a/Octokit/Models/Request/NewRepositoryHook.cs b/Octokit/Models/Request/NewRepositoryHook.cs index 86ac3b70d2..7e6962387b 100644 --- a/Octokit/Models/Request/NewRepositoryHook.cs +++ b/Octokit/Models/Request/NewRepositoryHook.cs @@ -100,6 +100,11 @@ public NewRepositoryHook(string name, IReadOnlyDictionary config /// public bool Active { get; set; } + public virtual NewRepositoryHook ToRequest() + { + return this; + } + internal string DebuggerDisplay { get diff --git a/Octokit/Models/Request/NewRepositoryWebHook.cs b/Octokit/Models/Request/NewRepositoryWebHook.cs index ee680f78b6..2f3e204747 100644 --- a/Octokit/Models/Request/NewRepositoryWebHook.cs +++ b/Octokit/Models/Request/NewRepositoryWebHook.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -61,64 +62,15 @@ public class NewRepositoryWebHook : NewRepositoryHook /// /// A required string defining the URL to which the payloads will be delivered. /// - public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url) : this(name, config, url, WebHookContentType.Form, string.Empty, false) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// Use web for a webhook or use the name of a valid service. (See /hooks for the list of valid service names.) - /// Use "web" for a webhook or use the name of a valid service. (See - /// https://api.github.com/hooks for the list of valid service - /// names.) - /// - /// - /// Key/value pairs to provide settings for this hook. These settings vary between the services and are - /// defined in the github-services repository. Booleans are stored internally as “1” for true, and “0” for - /// false. Any JSON true/false values will be converted automatically. - /// - /// - /// A required string defining the URL to which the payloads will be delivered. - /// - /// - /// An optional string defining the media type used to serialize the payloads. - /// Supported values include json and form. - /// The default is form. - /// - /// - /// An optional string that’s passed with the HTTP requests as an X-Hub-Signature header. - /// The value of this header is computed as the - /// - /// HMAC hex digest of the body, using the secret as the key - /// . - /// - /// - /// An optional string that determines whether the SSL certificate of the host for url will be verified when delivering payloads. - /// Supported values include "0" (verification is performed) and "1" (verification is not performed). - /// The default is "0". - /// - public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url, WebHookContentType contentType, string secret, bool insecureSsl) - : base(name, GetWebHookConfig(url, contentType, secret, insecureSsl, config)) + public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url) + : base(name, config) { Ensure.ArgumentNotNullOrEmptyString(url, "url"); Url = url; - ContentType = contentType; - Secret = secret; - InsecureSsl = insecureSsl; - } - - static Dictionary GetWebHookConfig(string url, WebHookContentType contentType, string secret, bool insecureSsl, IReadOnlyDictionary config) - { - var webHookConfig = new Dictionary - { - { "url", url }, - { "content_type", contentType.ToParameter() }, - { "secret", secret }, - { "insecure_ssl", insecureSsl ? "1" : "0" } - }; - return config.Union(webHookConfig).ToDictionary(k => k.Key, v => v.Value); + ContentType = WebHookContentType.Form; + Secret = ""; + InsecureSsl = false; } /// @@ -128,9 +80,9 @@ static Dictionary GetWebHookConfig(string url, WebHookContentTyp /// The URL. /// public string Url { get; protected set; } - + /// - /// Gets the content type used to serialize the payload. + /// Gets the content type used to serialize the payload. The default is `form`. /// /// /// The content type. @@ -148,13 +100,25 @@ static Dictionary GetWebHookConfig(string url, WebHookContentTyp /// /// Gets wether the SSL certificate of the host will be verified when - /// delivering payloads. + /// delivering payloads. The default is `false`. /// /// /// true if SSL certificate verification is not performed; /// otherwise, false. /// public bool InsecureSsl { get; set; } + + public override NewRepositoryHook ToRequest() + { + var config = Config.Union(new Dictionary + { + { "url", Url }, + { "content_type", ContentType.ToParameter() }, + { "secret", Secret }, + { "insecure_ssl", InsecureSsl.ToString() } + }).ToDictionary(k => k.Key, v => v.Value); + return new NewRepositoryHook(Name, config); + } } /// From 110ffa705f4d8a0f1ebdd6db655b17fa577c23bb Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Mon, 5 Oct 2015 13:19:19 +1000 Subject: [PATCH 5/8] Fix up XML comments as per PR review --- Octokit/Models/Request/NewRepositoryHook.cs | 6 +++--- Octokit/Models/Request/NewRepositoryWebHook.cs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Octokit/Models/Request/NewRepositoryHook.cs b/Octokit/Models/Request/NewRepositoryHook.cs index 7e6962387b..2a651cf617 100644 --- a/Octokit/Models/Request/NewRepositoryHook.cs +++ b/Octokit/Models/Request/NewRepositoryHook.cs @@ -18,8 +18,8 @@ namespace Octokit /// /// content_type /// - /// An optional string defining the media type used to serialize the payloads.Supported values include json and - /// form.The default is form. + /// An optional string defining the media type used to serialize the payloads. Supported values include json and + /// form. The default is form. /// /// /// @@ -33,7 +33,7 @@ namespace Octokit /// insecure_ssl: /// /// An optional string that determines whether the SSL certificate of the host for url will be verified when - /// delivering payloads.Supported values include "0" (verification is performed) and "1" (verification is not + /// delivering payloads. Supported values include "0" (verification is performed) and "1" (verification is not /// performed). The default is "0". /// /// diff --git a/Octokit/Models/Request/NewRepositoryWebHook.cs b/Octokit/Models/Request/NewRepositoryWebHook.cs index 2f3e204747..2c73d06ca4 100644 --- a/Octokit/Models/Request/NewRepositoryWebHook.cs +++ b/Octokit/Models/Request/NewRepositoryWebHook.cs @@ -18,8 +18,8 @@ namespace Octokit /// /// content_type /// - /// An optional string defining the media type used to serialize the payloads.Supported values include json and - /// form.The default is form. + /// An optional string defining the media type used to serialize the payloads. Supported values include json and + /// form. The default is form. /// /// /// @@ -33,7 +33,7 @@ namespace Octokit /// insecure_ssl: /// /// An optional string that determines whether the SSL certificate of the host for url will be verified when - /// delivering payloads.Supported values include "0" (verification is performed) and "1" (verification is not + /// delivering payloads. Supported values include "0" (verification is performed) and "1" (verification is not /// performed). The default is "0". /// /// @@ -46,10 +46,10 @@ namespace Octokit public class NewRepositoryWebHook : NewRepositoryHook { /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// Using default values for ContentType, Secret and InsecureSsl. /// - /// Use web for a webhook or use the name of a valid service. (See /hooks for the list of valid service names.) + /// /// Use "web" for a webhook or use the name of a valid service. (See /// https://api.github.com/hooks for the list of valid service /// names.) @@ -57,7 +57,7 @@ public class NewRepositoryWebHook : NewRepositoryHook /// /// Key/value pairs to provide settings for this hook. These settings vary between the services and are /// defined in the github-services repository. Booleans are stored internally as “1” for true, and “0” for - /// false. Any JSON true/false values will be converted automatically. + /// false. Any true/false values will be converted automatically. /// /// /// A required string defining the URL to which the payloads will be delivered. @@ -99,7 +99,7 @@ public NewRepositoryWebHook(string name, IReadOnlyDictionary con public string Secret { get; set; } /// - /// Gets wether the SSL certificate of the host will be verified when + /// Gets whether the SSL certificate of the host will be verified when /// delivering payloads. The default is `false`. /// /// From c4b4233683c15386f92de90471e1d649f8ae3321 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Wed, 7 Oct 2015 23:31:00 +1000 Subject: [PATCH 6/8] Throw exception with helpful message if duplicate webhook config values exists. --- .../RepositoryWebHookConfigException.cs | 60 +++++++++++++++++++ Octokit/Helpers/StringExtensions.cs | 11 ++++ .../Models/Request/NewRepositoryWebHook.cs | 46 +++++++++++--- Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 3 +- Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 9 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 Octokit/Exceptions/RepositoryWebHookConfigException.cs diff --git a/Octokit/Exceptions/RepositoryWebHookConfigException.cs b/Octokit/Exceptions/RepositoryWebHookConfigException.cs new file mode 100644 index 0000000000..2368e6c73f --- /dev/null +++ b/Octokit/Exceptions/RepositoryWebHookConfigException.cs @@ -0,0 +1,60 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Linq; +using System.Runtime.Serialization; +using System.Text; +using System.Threading.Tasks; + +namespace Octokit +{ +#if !NETFX_CORE + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public class RepositoryWebHookConfigException : Exception + { + readonly string message; + + public RepositoryWebHookConfigException(IEnumerable invalidConfig) + { + var parameterList = string.Join(", ", invalidConfig.Select(ic => ic.FromRubyCase())); + message = string.Format(CultureInfo.InvariantCulture, + "Duplicate webhook config values found - these values: {0} should not be passed in as part of the config values. Use the properties on the NewRepositoryWebHook class instead.", + parameterList); + } + + public override string Message + { + get { return message; } + } + +#if !NETFX_CORE + /// + /// Constructs an instance of RepositoryWebHookConfigException + /// + /// + /// The that holds the + /// serialized object data about the exception being thrown. + /// + /// + /// The that contains + /// contextual information about the source or destination. + /// + protected RepositoryWebHookConfigException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + if (info == null) return; + message = info.GetString("Message"); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("Message", Message); + } +#endif + } +} diff --git a/Octokit/Helpers/StringExtensions.cs b/Octokit/Helpers/StringExtensions.cs index 71df432664..f195d405d4 100644 --- a/Octokit/Helpers/StringExtensions.cs +++ b/Octokit/Helpers/StringExtensions.cs @@ -84,6 +84,17 @@ public static string ToRubyCase(this string propertyName) return string.Join("_", propertyName.SplitUpperCase()).ToLowerInvariant(); } + public static string FromRubyCase(this string propertyName) + { + Ensure.ArgumentNotNullOrEmptyString(propertyName, "s"); + return string.Join("", propertyName.Split('_')).ToCapitalizedInvariant(); + } + + public static string ToCapitalizedInvariant(this string value) + { + Ensure.ArgumentNotNullOrEmptyString(value, "s"); + return string.Concat(value[0].ToString().ToUpperInvariant(), value.Substring(1)); + } static IEnumerable SplitUpperCase(this string source) { Ensure.ArgumentNotNullOrEmptyString(source, "source"); diff --git a/Octokit/Models/Request/NewRepositoryWebHook.cs b/Octokit/Models/Request/NewRepositoryWebHook.cs index 2c73d06ca4..3be98b99d5 100644 --- a/Octokit/Models/Request/NewRepositoryWebHook.cs +++ b/Octokit/Models/Request/NewRepositoryWebHook.cs @@ -62,7 +62,7 @@ public class NewRepositoryWebHook : NewRepositoryHook /// /// A required string defining the URL to which the payloads will be delivered. /// - public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url) + public NewRepositoryWebHook(string name, IReadOnlyDictionary config, string url) : base(name, config) { Ensure.ArgumentNotNullOrEmptyString(url, "url"); @@ -88,7 +88,7 @@ public NewRepositoryWebHook(string name, IReadOnlyDictionary con /// The content type. /// public WebHookContentType ContentType { get; set; } - + /// /// Gets the secret used as the key for the HMAC hex digest /// of the body passed with the HTTP requests as an X-Hub-Signature header. @@ -97,7 +97,7 @@ public NewRepositoryWebHook(string name, IReadOnlyDictionary con /// The secret. /// public string Secret { get; set; } - + /// /// Gets whether the SSL certificate of the host will be verified when /// delivering payloads. The default is `false`. @@ -110,15 +110,30 @@ public NewRepositoryWebHook(string name, IReadOnlyDictionary con public override NewRepositoryHook ToRequest() { - var config = Config.Union(new Dictionary + var webHookConfig = GetWebHookConfig(); + if (Config.Any(c => webHookConfig.ContainsKey(c.Key))) { - { "url", Url }, - { "content_type", ContentType.ToParameter() }, - { "secret", Secret }, - { "insecure_ssl", InsecureSsl.ToString() } - }).ToDictionary(k => k.Key, v => v.Value); + var invalidConfigs = Config.Where(c => webHookConfig.ContainsKey(c.Key)).Select(c => c.Key); + throw new RepositoryWebHookConfigException(invalidConfigs); + } + + var config = webHookConfig + .Union(Config, new WebHookConfigComparer()) + .ToDictionary(k => k.Key, v => v.Value); + return new NewRepositoryHook(Name, config); } + + Dictionary GetWebHookConfig() + { + return new Dictionary + { + { "url", Url }, + { "content_type", ContentType.ToParameter() }, + { "secret", Secret }, + { "insecure_ssl", InsecureSsl.ToString() } + }; + } } /// @@ -129,4 +144,17 @@ public enum WebHookContentType Form, Json } + + public class WebHookConfigComparer : IEqualityComparer> + { + public bool Equals(KeyValuePair x, KeyValuePair y) + { + return x.Key == y.Key; + } + + public int GetHashCode(KeyValuePair obj) + { + return obj.Key.GetHashCode(); + } + } } diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index ec28e2938a..4134099d3d 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -407,6 +407,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 555f68cd2f..65b473d778 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -414,6 +414,7 @@ + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 9ffaf4bafa..9f7378397f 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -410,7 +410,8 @@ + - + \ No newline at end of file diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 5d4fb93279..61de2d7b54 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -404,6 +404,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index c5f7244136..cc584a7dd1 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -411,6 +411,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index e559317dfc..c0edf9d44a 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -76,6 +76,7 @@ + From 282bcb39398fca9166e67ccb6ab45ea342958b20 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Wed, 7 Oct 2015 23:31:43 +1000 Subject: [PATCH 7/8] Add unit test to ensure correct message is returned when duplicate keys exists. --- .../Models/NewRepositoryWebHookTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Octokit.Tests/Models/NewRepositoryWebHookTests.cs b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs index 119b9d0c13..ca95ec7036 100644 --- a/Octokit.Tests/Models/NewRepositoryWebHookTests.cs +++ b/Octokit.Tests/Models/NewRepositoryWebHookTests.cs @@ -7,6 +7,9 @@ public class NewRepositoryWebHookTests { public class TheCtor { + string ExpectedRepositoryWebHookConfigExceptionMessage = + "Duplicate webhook config values found - these values: Url should not be passed in as part of the config values. Use the properties on the NewRepositoryWebHook class instead."; + [Fact] public void UsesDefaultValuesForDefaultConfig() { @@ -73,6 +76,33 @@ public void CombinesUserSpecifiedContentTypeWithConfig() Assert.True(request.Config.ContainsKey("password")); Assert.Equal(request.Config["password"], config["password"]); } + + [Fact] + public void ShouldThrowRepositoryWebHookConfigExceptionWhenDuplicateKeysExists() + { + var config = new Dictionary + { + {"url", "http://example.com/test"}, + {"hostname", "http://hostname.url"}, + {"username", "username"}, + {"password", "password"} + }; + + var create = new NewRepositoryWebHook("windowsazure", config, "http://test.com/example") + { + ContentType = WebHookContentType.Json, + Secret = string.Empty, + InsecureSsl = true + }; + + Assert.Equal(create.Url, "http://test.com/example"); + Assert.Equal(create.ContentType, WebHookContentType.Json); + Assert.Empty(create.Secret); + Assert.True(create.InsecureSsl); + + var ex = Assert.Throws(() => create.ToRequest()); + Assert.Equal(ExpectedRepositoryWebHookConfigExceptionMessage, ex.Message); + } } } } From e643534db034c87d07f61e34e8374fcb27130449 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 13 Oct 2015 17:53:00 +1000 Subject: [PATCH 8/8] A bit of code cleanup --- Octokit/Helpers/WebHookConfigComparer.cs | 17 +++++++++++++++++ Octokit/Models/Request/NewRepositoryWebHook.cs | 15 +-------------- Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 8 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 Octokit/Helpers/WebHookConfigComparer.cs diff --git a/Octokit/Helpers/WebHookConfigComparer.cs b/Octokit/Helpers/WebHookConfigComparer.cs new file mode 100644 index 0000000000..4c71681554 --- /dev/null +++ b/Octokit/Helpers/WebHookConfigComparer.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; + +namespace Octokit +{ + public class WebHookConfigComparer : IEqualityComparer> + { + public bool Equals(KeyValuePair x, KeyValuePair y) + { + return x.Key == y.Key; + } + + public int GetHashCode(KeyValuePair obj) + { + return obj.Key.GetHashCode(); + } + } +} diff --git a/Octokit/Models/Request/NewRepositoryWebHook.cs b/Octokit/Models/Request/NewRepositoryWebHook.cs index 3be98b99d5..36d0183296 100644 --- a/Octokit/Models/Request/NewRepositoryWebHook.cs +++ b/Octokit/Models/Request/NewRepositoryWebHook.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -145,16 +144,4 @@ public enum WebHookContentType Json } - public class WebHookConfigComparer : IEqualityComparer> - { - public bool Equals(KeyValuePair x, KeyValuePair y) - { - return x.Key == y.Key; - } - - public int GetHashCode(KeyValuePair obj) - { - return obj.Key.GetHashCode(); - } - } } diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 4134099d3d..fd3d9f2c6e 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -408,6 +408,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 65b473d778..7a813d2cef 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -415,6 +415,7 @@ + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 9f7378397f..5eb79ac2e6 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -411,6 +411,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 61de2d7b54..6757e1a656 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -405,6 +405,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index cc584a7dd1..d6a2fd84af 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -412,6 +412,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index c0edf9d44a..2c15e7535d 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -85,6 +85,7 @@ +