feat|fix: adding attribute to skip null properties

Adding the [SerializeIfNotNullAttribute] which can be applied to
properties serialized by the QuerySerializer to indicate they should
never be serialized when null.

As a proof of concept, the RuleParameterInput used in the creation of
rulesets for the repository was failing due to null properties being
seralized into the final output. This code now functions as expected.

I did not make a global change to no longer serialized all null values
as I anticipate that there are "side effects" we depend on where there
are expected null serialzations that will break if such a change is
made.

This is an "opt-in" feature that does not disturb the greater echosystem
that may depend on these "side effects".

#320

src/Octokit.GraphQL.Core/Core/Serializers/QuerySerializer.cs

@@ -12,7 +12,13 @@ namespace Octokit.GraphQL.Core.Serializers
 {
     public class QuerySerializer
     {
-        private static readonly ConcurrentDictionary<Type, Tuple<string, MethodInfo>[]> typeCache = new ConcurrentDictionary<Type, Tuple<string, MethodInfo>[]>();
+        /// <summary>
+        /// A cache of previously reflected types where:
+        /// Item1: The name of the type
+        /// Item2: The method info object to retrieve the value of the property.
+        /// Item3: A boolean indicating whether the value should always be serialized (true) or only serialized when not null (false)
+        /// </summary>
+        private static readonly ConcurrentDictionary<Type, Tuple<string, MethodInfo, bool>[]> typeCache = new ConcurrentDictionary<Type, Tuple<string, MethodInfo, bool>[]>();
 
         private readonly int indentation;
         private readonly string comma = ",";
@@ -232,12 +238,12 @@ private void SerializeValue(StringBuilder builder, object value)
             {
                 var objectType = value.GetType();
 
-                Tuple<string, MethodInfo>[] properties;
+                Tuple<string, MethodInfo, bool>[] properties;
                 if (!typeCache.TryGetValue(objectType, out properties))
                 {
                     properties = objectType.GetRuntimeProperties()
                         .Where(info => info.GetMethod.IsPublic)
-                        .Select(info => new Tuple<string, MethodInfo>(info.Name.LowerFirstCharacter(), info.GetMethod))
+                        .Select(info => new Tuple<string, MethodInfo, bool>(info.Name.LowerFirstCharacter(), info.GetMethod, null == info.GetCustomAttribute<SerializeIfNotNull>()))
                         .ToArray();
 
                     typeCache.TryAdd(objectType, properties);
@@ -247,12 +253,23 @@ private void SerializeValue(StringBuilder builder, object value)
                     //Cache Hit
                 }
 
+                var hasOpenBrace = false;
                 for (var index = 0; index < properties.Length; index++)
                 {
                     var property = properties[index];
+                    var valueObj = property.Item2.Invoke(value, null);
 
-                    if (index == 0)
+                    // Property 3 indicates if we should write the property when the value of the property is null.
+                    // True: always write the value
+                    // False: only write the value if it isn't null
+                    if (property.Item3 == false && null == valueObj)
                     {
+                        continue;
+                    }
+
+                    if (!hasOpenBrace)
+                    {
+                        hasOpenBrace = true;
                         OpenBrace(builder);
                     }
                     else
@@ -261,12 +278,12 @@ private void SerializeValue(StringBuilder builder, object value)
                     }
 
                     builder.Append(property.Item1.LowerFirstCharacter()).Append(colon);
-                    SerializeValue(builder, property.Item2.Invoke(value, null));
+                    SerializeValue(builder, valueObj);
+                }
 
-                    if (index + 1 == properties.Length)
-                    {
-                        CloseBrace(builder);
-                    }
+                if (hasOpenBrace)
+                {
+                    CloseBrace(builder);
                 }
             }
         }

src/Octokit.GraphQL.Core/Core/Serializers/SerializeIfNotNullAttribute.cs

@@ -0,0 +1,9 @@
+using System;
+
+namespace Octokit.GraphQL.Core.Serializers
+{
+    [AttributeUsage(AttributeTargets.Property)]
+    public class SerializeIfNotNull : Attribute
+    {
+    }
+}

src/Octokit.GraphQL.UnitTests/QueryBuilderTests.cs

@@ -394,5 +394,100 @@ ... on RepositoryOwner {
 
 
         }
+
+        /// <summary>
+        /// Tests that the <see cref="Mutation.CreateRepositoryRuleset"/> mutation skips serializing null values in the
+        /// <see cref="RuleParametersInput"/> class. When the null values in this class are sent to GitHub the call
+        /// fails.
+        /// </summary>
+        [Fact]
+        public void Repository_CreateRepositoryRuleset()
+        {
+          var expected = @"mutation {
+  createRepositoryRuleset(input:  {
+    clientMutationId: null,sourceId: ""hello world"",name: ""main"",target: BRANCH,rules: [ {
+      id: null,type: DELETION,parameters: null
+    }, {
+      id: null,type: PULL_REQUEST,parameters:  {
+        pullRequest:  {
+          dismissStaleReviewsOnPush: true,requireCodeOwnerReview: true,requireLastPushApproval: false,requiredApprovingReviewCount: 0,requiredReviewThreadResolution: false
+        }
+      }
+    }, {
+      id: null,type: REQUIRED_STATUS_CHECKS,parameters:  {
+        requiredStatusChecks:  {
+          requiredStatusChecks: [ {
+            context: ""ng test"",integrationId: null
+          }, {
+            context: ""ng lint"",integrationId: null
+          }],strictRequiredStatusChecksPolicy: true
+        }
+      }
+    }],conditions:  {
+      refName:  {
+        exclude: [],include: [""~DEFAULT_BRANCH""]
+      },repositoryName: null,repositoryId: null,repositoryProperty: null
+    },enforcement: ACTIVE,bypassActors: null
+  }) {
+    ruleset {
+      id
+    }
+  }
+}";
+
+          var expression = new Mutation()
+            .CreateRepositoryRuleset(new Arg<CreateRepositoryRulesetInput>(new CreateRepositoryRulesetInput
+            {
+              SourceId = new ID("hello world"),
+              Name = "main",
+              Enforcement = RuleEnforcement.Active,
+              Target = RepositoryRulesetTarget.Branch,
+              Rules = new[]
+              {
+                new RepositoryRuleInput {
+                  Type = RepositoryRuleType.Deletion
+                },
+                new RepositoryRuleInput {
+                  Type = RepositoryRuleType.PullRequest,
+                  Parameters = new RuleParametersInput {
+                    PullRequest = new PullRequestParametersInput {
+                      DismissStaleReviewsOnPush = true,
+                      RequireCodeOwnerReview = true
+                    }
+                  }
+                },
+                new RepositoryRuleInput {
+                  Type = RepositoryRuleType.RequiredStatusChecks,
+                  Parameters = new RuleParametersInput {
+                    RequiredStatusChecks = new RequiredStatusChecksParametersInput {
+                      RequiredStatusChecks = new[]
+                      {
+                        new StatusCheckConfigurationInput {
+                          Context = "ng test"
+                        },
+                        new StatusCheckConfigurationInput {
+                          Context = "ng lint"
+                        }
+                      },
+                      StrictRequiredStatusChecksPolicy = true
+                    }
+                  }
+                }
+              },
+              Conditions = new RepositoryRuleConditionsInput
+              {
+                RefName = new RefNameConditionTargetInput
+                {
+                  Include = new[] { "~DEFAULT_BRANCH" },
+                  Exclude = new string[] { }
+                }
+              }
+            }))
+            .Select(x => x.Ruleset.Id);
+
+          var query = expression.Compile();
+
+          Assert.Equal(expected, query.ToString(2), ignoreLineEndingDifferences: true);
+        }
     }
 }

src/Octokit.GraphQL/Model/RuleParametersInput.cs

@@ -1,7 +1,6 @@
 namespace Octokit.GraphQL.Model
 {
-    using System;
-    using System.Collections.Generic;
+    using Core.Serializers;
 
     /// <summary>
     /// Specifies the parameters for a `RepositoryRule` object. Only one of the fields should be specified.
@@ -11,51 +10,61 @@ public class RuleParametersInput
         /// <summary>
         /// Parameters used for the `update` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public UpdateParametersInput Update { get; set; }
 
         /// <summary>
         /// Parameters used for the `required_deployments` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public RequiredDeploymentsParametersInput RequiredDeployments { get; set; }
 
         /// <summary>
         /// Parameters used for the `pull_request` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public PullRequestParametersInput PullRequest { get; set; }
 
         /// <summary>
         /// Parameters used for the `required_status_checks` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public RequiredStatusChecksParametersInput RequiredStatusChecks { get; set; }
 
         /// <summary>
         /// Parameters used for the `commit_message_pattern` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public CommitMessagePatternParametersInput CommitMessagePattern { get; set; }
 
         /// <summary>
         /// Parameters used for the `commit_author_email_pattern` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public CommitAuthorEmailPatternParametersInput CommitAuthorEmailPattern { get; set; }
 
         /// <summary>
         /// Parameters used for the `committer_email_pattern` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public CommitterEmailPatternParametersInput CommitterEmailPattern { get; set; }
 
         /// <summary>
         /// Parameters used for the `branch_name_pattern` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public BranchNamePatternParametersInput BranchNamePattern { get; set; }
 
         /// <summary>
         /// Parameters used for the `tag_name_pattern` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public TagNamePatternParametersInput TagNamePattern { get; set; }
 
         /// <summary>
         /// Parameters used for the `workflows` rule type
         /// </summary>
+        [SerializeIfNotNull]
         public WorkflowsParametersInput Workflows { get; set; }
     }
 }