Adding a convention test to detect whether a model has a constructor …

…exposing all properties (#1798)

* Added a convention test to detect a model constructor exposing all properties

* add ctors to classes where they are missing

* rename ctor parameters that dont match properties

* add missing parameters to existing ctors

* add specific PunchCard ctor to allow mocking, and update test to resolve call ambiguity

* Added base class properties to the convention test

Added member exclusion attribute

* Updated newly offending classes

2 excludes and 2 ctors

* rename exclusion attribute to be a bit shorter

Octokit.Tests.Conventions/Exception/MissingPublicConstructorWithAllPropertiesException.cs

@@ -0,0 +1,22 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+namespace Octokit.Tests.Conventions
+{
+    public class MissingPublicConstructorWithAllPropertiesException : Exception
+    {
+        public MissingPublicConstructorWithAllPropertiesException(Type modelType, IEnumerable<PropertyInfo> missingProperties)
+            : base(CreateMessage(modelType, missingProperties))
+        { }
+
+        private static string CreateMessage(Type modelType, IEnumerable<PropertyInfo> missingProperties)
+        {
+            return string.Format("Model type '{0}' is missing a constructor with all properties. Closest match is missing the following properties: {1}{2}",
+                modelType.FullName,
+                Environment.NewLine,
+                string.Join(Environment.NewLine, missingProperties.Select(prop => prop.Name)));
+        }
+    }
+}

Octokit.Tests.Conventions/ModelTests.cs

@@ -52,6 +52,42 @@ public void AllResponseModelsHavePublicParameterlessCtors(Type modelType)
             }
         }
 
+        [Theory]
+        [MemberData(nameof(ResponseModelTypes))]
+        public void AllResponseModelsHavePublicCtorWithAllProperties(Type modelType)
+        {
+            var excludedProperties = modelType.GetCustomAttribute<ExcludeFromCtorWithAllPropertiesConventionTestAttribute>()?
+                                         .Properties ??
+                                     new string[] { };
+
+            var constructors = modelType.GetConstructors();
+            var properties = modelType.GetProperties()
+                .Where(prop => prop.CanWrite &&
+                               !excludedProperties.Contains(prop.Name))
+                .ToList();
+
+            var missingProperties = properties.ToList();
+            foreach (var constructor in constructors)
+            {
+                var parameters = constructor.GetParameters();
+
+                var constructorMissingProperties = properties.Where(property =>
+                        !parameters.Any(param =>
+                            string.Equals(param.Name, property.Name, StringComparison.InvariantCultureIgnoreCase)))
+                    .ToList();
+
+                if (constructorMissingProperties.Count < missingProperties.Count)
+                {
+                    missingProperties = constructorMissingProperties;
+                }
+            }
+
+            if (missingProperties.Any())
+            {
+                throw new MissingPublicConstructorWithAllPropertiesException(modelType, missingProperties);
+            }
+        }
+
         [Theory]
         [MemberData(nameof(ResponseModelTypes))]
         public void ResponseModelsHaveGetterOnlyProperties(Type modelType)

Octokit.Tests/Clients/AuthorizationsClientTests.cs

@@ -172,7 +172,7 @@ public async Task UsesCallbackToRetrieveTwoFactorCode()
                     "secret",
                     Arg.Any<NewAuthorization>(),
                     "two-factor-code")
-                    .Returns(Task.Factory.StartNew(() => new ApplicationAuthorization("xyz")));
+                    .Returns(Task.Factory.StartNew(() => new ApplicationAuthorization(0, null, null, null, null, null, null, null, DateTimeOffset.Now, DateTimeOffset.Now, null, "xyz")));
 
                 var result = await client.GetOrCreateApplicationAuthentication("clientId",
                     "secret",
@@ -204,7 +204,7 @@ public async Task RetriesWhenResendRequested()
                     "secret",
                     Arg.Any<NewAuthorization>(),
                     "two-factor-code")
-                    .Returns(Task.Factory.StartNew(() => new ApplicationAuthorization("OAUTHSECRET")));
+                    .Returns(Task.Factory.StartNew(() => new ApplicationAuthorization(0, null, null, null, null, null, null, null, DateTimeOffset.Now, DateTimeOffset.Now, null, "OAUTHSECRET")));
 
                 var result = await client.GetOrCreateApplicationAuthentication("clientId",
                     "secret",

Octokit.Tests/Models/PunchCardTests.cs

@@ -11,7 +11,7 @@ public class TheConstructor
         [Fact]
         public void ThrowsExceptionWithNullPunchCardPoints()
         {
-            Assert.Throws<ArgumentNullException>(() => new PunchCard(null));
+            Assert.Throws<ArgumentNullException>(() => new PunchCard((IEnumerable<IList<int>>)null));
         }
 
         [Fact]

Octokit.Tests/Reactive/AuthorizationExtensionsTests.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Reactive.Linq;
 using System.Threading.Tasks;
 using NSubstitute;
@@ -16,7 +17,7 @@ public async Task UsesCallbackToRetrieveTwoFactorCode()
             {
                 var firstResponse = new TwoFactorRequiredException(TwoFactorType.AuthenticatorApp);
                 var twoFactorChallengeResult = new TwoFactorChallengeResult("two-factor-code");
-                var secondResponse = new ApplicationAuthorization("OAUTHSECRET");
+                var secondResponse = new ApplicationAuthorization(0, null, null, null, null, null, null, null, DateTimeOffset.Now, DateTimeOffset.Now, null, "OAUTHSECRET");
 
                 var client = Substitute.For<IObservableAuthorizationsClient>();
                 client.GetOrCreateApplicationAuthentication(Args.String, Args.String, Args.NewAuthorization)
@@ -51,7 +52,7 @@ public async Task RetriesWhenResendRequested()
                     TwoFactorChallengeResult.RequestResendCode,
                     new TwoFactorChallengeResult("two-factor-code")
                 });
-                var secondResponse = new ApplicationAuthorization("OAUTHSECRET");
+                var secondResponse = new ApplicationAuthorization(0, null, null, null, null, null, null, null, DateTimeOffset.Now, DateTimeOffset.Now, null, "OAUTHSECRET");
 
                 var client = Substitute.For<IObservableAuthorizationsClient>();
                 client.GetOrCreateApplicationAuthentication(Args.String, Args.String, Args.NewAuthorization)

Octokit/Helpers/ExcludeFromCtorWithAllPropertiesConventionTestAttribute.cs

@@ -0,0 +1,15 @@
+using System;
+
+namespace Octokit
+{
+    [AttributeUsage(AttributeTargets.Class)]
+    public sealed class ExcludeFromCtorWithAllPropertiesConventionTestAttribute : Attribute
+    {
+        public ExcludeFromCtorWithAllPropertiesConventionTestAttribute(params string[] properties)
+        {
+            Properties = properties;
+        }
+
+        public string[] Properties { get; private set; }
+    }
+}

Octokit/Http/RateLimit.cs

@@ -32,15 +32,15 @@ public RateLimit(IDictionary<string, string> responseHeaders)
             ResetAsUtcEpochSeconds = GetHeaderValueAsInt32Safe(responseHeaders, "X-RateLimit-Reset");
         }
 
-        public RateLimit(int limit, int remaining, long reset)
+        public RateLimit(int limit, int remaining, long resetAsUtcEpochSeconds)
         {
             Ensure.ArgumentNotNull(limit, nameof(limit));
             Ensure.ArgumentNotNull(remaining, nameof(remaining));
-            Ensure.ArgumentNotNull(reset, nameof(reset));
+            Ensure.ArgumentNotNull(resetAsUtcEpochSeconds, nameof(resetAsUtcEpochSeconds));
 
             Limit = limit;
             Remaining = remaining;
-            ResetAsUtcEpochSeconds = reset;
+            ResetAsUtcEpochSeconds = resetAsUtcEpochSeconds;
         }
 
         /// <summary>

Octokit/Models/Response/Activity.cs

@@ -13,7 +13,7 @@ public class Activity
     {
         public Activity() { }
 
-        public Activity(string type, bool @public, Repository repo, User actor, Organization org, DateTimeOffset createdAt, string id)
+        public Activity(string type, bool @public, Repository repo, User actor, Organization org, DateTimeOffset createdAt, string id, ActivityPayload payload)
         {
             Type = type;
             Public = @public;
@@ -22,6 +22,7 @@ public Activity(string type, bool @public, Repository repo, User actor, Organiza
             Org = org;
             CreatedAt = createdAt;
             Id = id;
+            Payload = payload;
         }
 
         /// <summary>

Octokit/Models/Response/ActivityPayloads/ActivityPayload.cs

@@ -5,6 +5,16 @@ namespace Octokit
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class ActivityPayload
     {
+        public ActivityPayload() { }
+
+
+        public ActivityPayload(Repository repository, User sender, InstallationId installation)
+        {
+            Repository = repository;
+            Sender = sender;
+            Installation = installation;
+        }
+
         public Repository Repository { get; protected set; }
         public User Sender { get; protected set; }
         public InstallationId Installation { get; protected set; }

Octokit/Models/Response/ApplicationAuthorization.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Diagnostics;
 
 namespace Octokit
@@ -13,7 +14,8 @@ public ApplicationAuthorization()
         {
         }
 
-        public ApplicationAuthorization(string token)
+        public ApplicationAuthorization(int id, string url, Application application, string tokenLastEight, string hashedToken, string fingerprint, string note, string noteUrl, DateTimeOffset createdAt, DateTimeOffset updateAt, string[] scopes, string token)
+            : base(id, url, application, tokenLastEight, hashedToken, fingerprint, note, noteUrl, createdAt, updateAt, scopes)
         {
             Token = token;
         }

Octokit/Models/Response/Authorization.cs

@@ -14,14 +14,17 @@ public class Authorization
         // TODO: I'd love to not need this
         public Authorization() { }
 
-        public Authorization(int id, string url, Application application, string note, string noteUrl, DateTimeOffset createdAt, DateTimeOffset updateAt, string[] scopes)
+        public Authorization(int id, string url, Application application, string tokenLastEight, string hashedToken, string fingerprint, string note, string noteUrl, DateTimeOffset createdAt, DateTimeOffset updateAt, string[] scopes)
         {
             Id = id;
             Url = url;
             Application = application;
 
             // TODO: testable ctor for new values
             //Token = token;
+            TokenLastEight = tokenLastEight;
+            HashedToken = hashedToken;
+            Fingerprint = fingerprint;
 
             Note = note;
             NoteUrl = noteUrl;

Octokit/Models/Response/BranchProtection.cs

@@ -165,6 +165,13 @@ public class BranchProtectionRequiredReviews
     {
         public BranchProtectionRequiredReviews() { }
 
+        public BranchProtectionRequiredReviews(BranchProtectionRequiredReviewsDismissalRestrictions dismissalRestrictions, bool dismissStaleReviews, bool requireCodeOwnerReviews)
+        {
+            DismissalRestrictions = dismissalRestrictions;
+            DismissStaleReviews = dismissStaleReviews;
+            RequireCodeOwnerReviews = requireCodeOwnerReviews;
+        }
+
         /// <summary>
         /// Specify which users and teams can dismiss pull request reviews.
         /// </summary>

Octokit/Models/Response/CollaboratorPermission.cs

@@ -5,6 +5,14 @@ namespace Octokit
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class CollaboratorPermission
     {
+        public CollaboratorPermission() { }
+
+        public CollaboratorPermission(PermissionLevel permission, User user)
+        {
+            Permission = permission;
+            User = user;
+        }
+
         public StringEnum<PermissionLevel> Permission { get; protected set; }
         public User User { get; protected set; }
 

Octokit/Models/Response/Commit.cs

@@ -10,7 +10,7 @@ public class Commit : GitReference
     {
         public Commit() { }
 
-        public Commit(string url, string label, string @ref, string sha, User user, Repository repository, string message, Committer author, Committer committer, GitReference tree, IEnumerable<GitReference> parents, int commentCount)
+        public Commit(string url, string label, string @ref, string sha, User user, Repository repository, string message, Committer author, Committer committer, GitReference tree, IEnumerable<GitReference> parents, int commentCount, Verification verification)
             : base(url, label, @ref, sha, user, repository)
         {
             Ensure.ArgumentNotNull(parents, nameof(parents));
@@ -21,6 +21,7 @@ public Commit(string url, string label, string @ref, string sha, User user, Repo
             Tree = tree;
             Parents = new ReadOnlyCollection<GitReference>(parents.ToList());
             CommentCount = commentCount;
+            Verification = verification;
         }
 
         public string Message { get; protected set; }

Octokit/Models/Response/CommitComment.cs

@@ -9,7 +9,7 @@ public class CommitComment
     {
         public CommitComment() { }
 
-        public CommitComment(int id, string url, string htmlUrl, string body, string path, int position, int? line, string commitId, User user, DateTimeOffset createdAt, DateTimeOffset? updatedAt)
+        public CommitComment(int id, string url, string htmlUrl, string body, string path, int position, int? line, string commitId, User user, DateTimeOffset createdAt, DateTimeOffset? updatedAt, ReactionSummary reactions)
         {
             Id = id;
             Url = url;
@@ -22,6 +22,7 @@ public CommitComment(int id, string url, string htmlUrl, string body, string pat
             User = user;
             CreatedAt = createdAt;
             UpdatedAt = updatedAt;
+            Reactions = reactions;
         }
 
         /// <summary>
@@ -79,6 +80,9 @@ public CommitComment(int id, string url, string htmlUrl, string body, string pat
         /// </summary>
         public DateTimeOffset? UpdatedAt { get; protected set; }
 
+        /// <summary>
+        /// The reaction summary for this comment.
+        /// </summary>
         public ReactionSummary Reactions { get; protected set; }
 
         internal string DebuggerDisplay

Octokit/Models/Response/Deployment.cs

@@ -13,7 +13,7 @@ public class Deployment
     {
         public Deployment() { }
 
-        public Deployment(int id, string sha, string url, User creator, IReadOnlyDictionary<string, string> payload, DateTimeOffset createdAt, DateTimeOffset updatedAt, string description, string statusesUrl)
+        public Deployment(int id, string sha, string url, User creator, IReadOnlyDictionary<string, string> payload, DateTimeOffset createdAt, DateTimeOffset updatedAt, string description, string statusesUrl, bool transientEnvironment, bool productionEnvironment)
         {
             Id = id;
             Sha = sha;
@@ -24,6 +24,8 @@ public Deployment(int id, string sha, string url, User creator, IReadOnlyDiction
             UpdatedAt = updatedAt;
             Description = description;
             StatusesUrl = statusesUrl;
+            TransientEnvironment = transientEnvironment;
+            ProductionEnvironment = productionEnvironment;
         }
 
         /// <summary>

Octokit/Models/Response/DeploymentStatus.cs

@@ -11,14 +11,16 @@ public class DeploymentStatus
     {
         public DeploymentStatus() { }
 
-        public DeploymentStatus(int id, string url, DeploymentState state, User creator, IReadOnlyDictionary<string, string> payload, string targetUrl, DateTimeOffset createdAt, DateTimeOffset updatedAt, string description)
+        public DeploymentStatus(int id, string url, DeploymentState state, User creator, IReadOnlyDictionary<string, string> payload, string targetUrl, string logUrl, string environmentUrl, DateTimeOffset createdAt, DateTimeOffset updatedAt, string description)
         {
             Id = id;
             Url = url;
             State = state;
             Creator = creator;
             Payload = payload;
             TargetUrl = targetUrl;
+            LogUrl = logUrl;
+            EnvironmentUrl = environmentUrl;
             CreatedAt = createdAt;
             UpdatedAt = updatedAt;
             Description = description;

Octokit/Models/Response/GitTag.cs

@@ -7,13 +7,14 @@ public class GitTag : GitReference
     {
         public GitTag() { }
 
-        public GitTag(string url, string label, string @ref, string sha, User user, Repository repository, string tag, string message, Committer tagger, TagObject objectVar)
+        public GitTag(string url, string label, string @ref, string sha, User user, Repository repository, string tag, string message, Committer tagger, TagObject @object, Verification verification)
             : base(url, label, @ref, sha, user, repository)
         {
             Tag = tag;
             Message = message;
             Tagger = tagger;
-            Object = objectVar;
+            Object = @object;
+            Verification = verification;
         }
 
         public string Tag { get; protected set; }

Octokit/Models/Response/GpgKey.cs

@@ -10,6 +10,24 @@ namespace Octokit
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class GpgKey
     {
+        public GpgKey() { }
+
+        public GpgKey(int id, int? primaryKeyId, string keyId, string publicKey, IReadOnlyList<EmailAddress> emails, IReadOnlyList<GpgKey> subkeys, bool canSign, bool canEncryptCommunications, bool canEncryptStorage, bool canCertify, DateTimeOffset createdAt, DateTimeOffset? expiresAt)
+        {
+            Id = id;
+            PrimaryKeyId = primaryKeyId;
+            KeyId = keyId;
+            PublicKey = publicKey;
+            Emails = emails;
+            Subkeys = subkeys;
+            CanSign = canSign;
+            CanEncryptCommunications = canEncryptCommunications;
+            CanEncryptStorage = canEncryptStorage;
+            CanCertify = canCertify;
+            CreatedAt = createdAt;
+            ExpiresAt = expiresAt;
+        }
+
         public int Id { get; protected set; }
         public int? PrimaryKeyId { get; protected set; }
         public string KeyId { get; protected set; }

Octokit/Models/Response/Issue.cs

@@ -11,7 +11,7 @@ public class Issue
     {
         public Issue() { }
 
-        public Issue(string url, string htmlUrl, string commentsUrl, string eventsUrl, int number, ItemState state, string title, string body, User closedBy, User user, IReadOnlyList<Label> labels, User assignee, IReadOnlyList<User> assignees, Milestone milestone, int comments, PullRequest pullRequest, DateTimeOffset? closedAt, DateTimeOffset createdAt, DateTimeOffset? updatedAt, int id, bool locked, Repository repository)
+        public Issue(string url, string htmlUrl, string commentsUrl, string eventsUrl, int number, ItemState state, string title, string body, User closedBy, User user, IReadOnlyList<Label> labels, User assignee, IReadOnlyList<User> assignees, Milestone milestone, int comments, PullRequest pullRequest, DateTimeOffset? closedAt, DateTimeOffset createdAt, DateTimeOffset? updatedAt, int id, bool locked, Repository repository, ReactionSummary reactions)
         {
             Id = id;
             Url = url;
@@ -35,6 +35,7 @@ public Issue(string url, string htmlUrl, string commentsUrl, string eventsUrl, i
             UpdatedAt = updatedAt;
             Locked = locked;
             Repository = repository;
+            Reactions = reactions;
         }
 
         /// <summary>
@@ -144,6 +145,9 @@ public Issue(string url, string htmlUrl, string commentsUrl, string eventsUrl, i
         /// </summary>
         public Repository Repository { get; protected set; }
 
+        /// <summary>
+        /// The reaction summary for this issue.
+        /// </summary>
         public ReactionSummary Reactions { get; protected set; }
 
         internal string DebuggerDisplay