Merge pull request #1538 from stripe/ob-wholesome-test-json

Enforce that all properties have a Json attribute
stripe · Feb 22, 2019 · ead7348 · ead7348
2 parents cec3d24 + 7d89e9a
commit ead7348
Show file tree

Hide file tree

Showing 14 changed files with 95 additions and 18 deletions.
diff --git a/src/Stripe.net/Entities/EphemeralKeys/EphemeralKey.cs b/src/Stripe.net/Entities/EphemeralKeys/EphemeralKey.cs
@@ -16,15 +16,6 @@ public class EphemeralKey : StripeEntity<EphemeralKey>, IHasId, IHasObject
         [JsonProperty("associated_objects")]
         public List<EphemeralKeyAssociatedObject> AssociatedObjects { get; set; }
 
-        // RawJson is the raw JSON data of the response that was used to initialize this
-        // ephemeral key. When working with mobile clients that might only understand
-        // one version of the API you should prefer to send this value back to them so
-        // that they'll be able to decode an object that's current according to their version.
-        public string RawJson
-        {
-            get { return this.StripeResponse?.Content; }
-        }
-
         [JsonProperty("created")]
         [JsonConverter(typeof(DateTimeConverter))]
         public DateTime Created { get; set; }
@@ -41,5 +32,14 @@ public string RawJson
 
         [JsonProperty("livemode")]
         public bool Livemode { get; set; }
+
+        /// <summary>
+        /// <see cref="RawJson"/> is the raw JSON data of the response that was used to initialize
+        /// this ephemeral key. When working with mobile clients that might only understand
+        /// one version of the API you should prefer to send this value back to them so
+        /// that they'll be able to decode an object that's current according to their version.
+        /// </summary>
+        [JsonIgnore]
+        public string RawJson => this.StripeResponse?.Content;
     }
 }
diff --git a/src/Stripe.net/Entities/Orders/Order.cs b/src/Stripe.net/Entities/Orders/Order.cs
@@ -66,6 +66,7 @@ public class Order : StripeEntity<Order>, IHasId, IHasMetadata, IHasObject
         /// <summary>
         /// The customer used for the order.
         /// </summary>
+        [JsonIgnore]
         public string CustomerId { get; set; }
 
         [JsonIgnore]

diff --git a/src/Stripe.net/Entities/Orders/OrderReturn.cs b/src/Stripe.net/Entities/Orders/OrderReturn.cs
@@ -61,6 +61,7 @@ public class OrderReturn : StripeEntity<OrderReturn>, IHasId, IHasObject
         /// <para>The ID of the refund issued for this return.</para>
         /// <para>Expandable</para>
         /// </summary>
+        [JsonIgnore]
         public string RefundId { get; set; }
 
         [JsonIgnore]

diff --git a/src/Stripe.net/Entities/Topups/Topup.cs b/src/Stripe.net/Entities/Topups/Topup.cs
@@ -24,6 +24,7 @@ public class Topup : StripeEntity<Topup>, IHasId, IHasMetadata, IHasObject, IBal
         /// <summary>
         /// ID of the balance transaction that describes the impact of this Top-up on your account balance (not including refunds or disputes).
         /// </summary>
+        [JsonIgnore]
         public string BalanceTransactionId { get; set; }
 
         [JsonIgnore]

diff --git a/src/Stripe.net/Entities/_base/StripeEntity.cs b/src/Stripe.net/Entities/_base/StripeEntity.cs
@@ -5,6 +5,7 @@ namespace Stripe
     using System.Runtime.CompilerServices;
     using Newtonsoft.Json;
 
+    [JsonObject(MemberSerialization.OptIn)]
     public abstract class StripeEntity : IStripeEntity
     {
         [JsonIgnore]

diff --git a/src/Stripe.net/Services/Customers/CustomerCreateOptions.cs b/src/Stripe.net/Services/Customers/CustomerCreateOptions.cs
@@ -51,8 +51,10 @@ public class CustomerCreateOptions : BaseOptions
 
         #region Trial End
 
+        [JsonIgnore]
         public DateTime? TrialEnd { get; set; }
 
+        [JsonIgnore]
         public bool EndTrialNow { get; set; }
 
         [JsonProperty("trial_end")]

diff --git a/src/Stripe.net/Services/Invoices/UpcomingInvoiceListLineItemsOptions.cs b/src/Stripe.net/Services/Invoices/UpcomingInvoiceListLineItemsOptions.cs
@@ -38,10 +38,13 @@ public class UpcomingInvoiceListLineItemsOptions : ListOptions
         /// <c>month</c> or <c>year</c> intervals, the day of the month for subsequent invoices.For
         /// existing subscriptions, the value can only be set to <c>now</c> or <c>unchanged</c>.
         /// </summary>
+        [JsonIgnore]
         public DateTime? SubscriptionBillingCycleAnchor { get; set; }
 
+        [JsonIgnore]
         public bool SubscriptionBillingCycleAnchorNow { get; set; }
 
+        [JsonIgnore]
         public bool SubscriptionBillingCycleAnchorUnchanged { get; set; }
 
         [JsonProperty("subscription_billing_cycle_anchor")]

diff --git a/src/Stripe.net/Services/Invoices/UpcomingInvoiceOptions.cs b/src/Stripe.net/Services/Invoices/UpcomingInvoiceOptions.cs
@@ -38,10 +38,13 @@ public class UpcomingInvoiceOptions : BaseOptions
         /// <c>month</c> or <c>year</c> intervals, the day of the month for subsequent invoices.For
         /// existing subscriptions, the value can only be set to <c>now</c> or <c>unchanged</c>.
         /// </summary>
+        [JsonIgnore]
         public DateTime? SubscriptionBillingCycleAnchor { get; set; }
 
+        [JsonIgnore]
         public bool SubscriptionBillingCycleAnchorNow { get; set; }
 
+        [JsonIgnore]
         public bool SubscriptionBillingCycleAnchorUnchanged { get; set; }
 
         [JsonProperty("subscription_billing_cycle_anchor")]

diff --git a/src/Stripe.net/Services/Plans/PlanTierOptions.cs b/src/Stripe.net/Services/Plans/PlanTierOptions.cs
@@ -1,7 +1,5 @@
 namespace Stripe
 {
-    using System;
-    using System.Reflection;
     using Newtonsoft.Json;
 
     public class PlanTierOptions : INestedOptions
@@ -13,6 +11,7 @@ public class PlanTierOptions : INestedOptions
         public long? UnitAmount { get; set; }
 
         #region UpTo
+        [JsonIgnore]
         public UpToOption UpTo { get; set; }
 
         [JsonProperty("up_to")]

diff --git a/src/Stripe.net/Services/Subscriptions/SubscriptionSharedOptions.cs b/src/Stripe.net/Services/Subscriptions/SubscriptionSharedOptions.cs
@@ -69,8 +69,10 @@ public abstract class SubscriptionSharedOptions : BaseOptions
         /// <summary>
         /// Date representing the end of the trial period the customer will get before being charged for the first time. Set <see cref="EndTrialNow"/> to <c>true</c> to end the customer’s trial immediately.
         /// </summary>
+        [JsonIgnore]
         public DateTime? TrialEnd { get; set; }
 
+        [JsonIgnore]
         public bool EndTrialNow { get; set; }
 
         [JsonProperty("trial_end")]

diff --git a/src/Stripe.net/Services/Subscriptions/SubscriptionUpdateOptions.cs b/src/Stripe.net/Services/Subscriptions/SubscriptionUpdateOptions.cs
@@ -3,13 +3,14 @@ namespace Stripe
     using System;
     using System.Collections.Generic;
     using Newtonsoft.Json;
-    using Stripe.Infrastructure;
 
     public class SubscriptionUpdateOptions : SubscriptionSharedOptions
     {
         #region BillingCycleAnchor
+        [JsonIgnore]
         public bool BillingCycleAnchorNow { get; set; }
 
+        [JsonIgnore]
         public bool BillingCycleAnchorUnchanged { get; set; }
 
         [JsonProperty("billing_cycle_anchor")]

diff --git a/src/StripeTests/Wholesome/PropertiesHaveJsonAttributes.cs b/src/StripeTests/Wholesome/PropertiesHaveJsonAttributes.cs
@@ -0,0 +1,62 @@
+#if NETCOREAPP
+namespace StripeTests
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Reflection;
+    using Newtonsoft.Json;
+    using Stripe;
+    using Xunit;
+
+    /// <summary>
+    /// This wholesome test ensures that all properties in Stripe entities and options classes are
+    /// annotated with either <see cref="JsonPropertyAttribute"/> or
+    /// <see cref="JsonIgnoreAttribute"/>.
+    /// </summary>
+    public class PropertiesHaveJsonAttributes : WholesomeTest
+    {
+        private const string AssertionMessage =
+            "Found at least one property lacking a Json*Attribute. Please add either a "
+            + "[JsonProperty(\"name\")] or a [JsonIgnore] attribute.";
+
+        [Fact]
+        public void Check()
+        {
+            List<string> results = new List<string>();
+
+            // Get all classes that derive from StripeEntity or implement INestedOptions
+            var stripeClasses = GetSubclassesOf(typeof(StripeEntity));
+            stripeClasses.AddRange(GetClassesWithInterface(typeof(INestedOptions)));
+
+            foreach (var stripeClass in stripeClasses)
+            {
+                foreach (var property in stripeClass.GetProperties())
+                {
+                    var hasJsonAttribute = false;
+
+                    foreach (var attribute in property.GetCustomAttributes())
+                    {
+                        if (attribute.GetType() == typeof(JsonPropertyAttribute)
+                            || attribute.GetType() == typeof(JsonIgnoreAttribute)
+                            || attribute.GetType() == typeof(JsonExtensionDataAttribute))
+                        {
+                            hasJsonAttribute = true;
+                            break;
+                        }
+                    }
+
+                    if (hasJsonAttribute)
+                    {
+                        continue;
+                    }
+
+                    results.Add($"{stripeClass.Name}.{property.Name}");
+                }
+            }
+
+            AssertEmpty(results, AssertionMessage);
+        }
+    }
+}
+#endif
diff --git a/src/StripeTests/Wholesome/UseListsInsteadOfArrays.cs b/src/StripeTests/Wholesome/UseListsInsteadOfArrays.cs
@@ -3,7 +3,6 @@ namespace StripeTests
 {
     using System;
     using System.Collections.Generic;
-    using System.Linq;
     using System.Reflection;
     using Newtonsoft.Json;
     using Stripe;
@@ -25,7 +24,7 @@ public void Check()
 
             // Get all classes that derive from StripeEntity or implement INestedOptions
             var stripeClasses = GetSubclassesOf(typeof(StripeEntity));
-            stripeClasses.Concat(GetClassesWithInterface(typeof(INestedOptions)));
+            stripeClasses.AddRange(GetClassesWithInterface(typeof(INestedOptions)));
 
             foreach (Type stripeClass in stripeClasses)
             {

diff --git a/src/StripeTests/Wholesome/WholesomeTest.cs b/src/StripeTests/Wholesome/WholesomeTest.cs
@@ -40,23 +40,25 @@ protected static void AssertEmpty(List<string> results, string message)
         /// <summary>
         /// Returns the list of classes that are subclasses of the provided type.
         /// </summary>
-        protected static IEnumerable<Type> GetSubclassesOf(Type parentClass)
+        protected static List<Type> GetSubclassesOf(Type parentClass)
         {
             var assembly = parentClass.GetTypeInfo().Assembly;
             return assembly.DefinedTypes
                 .Where(t => t.IsClass && t.IsSubclassOf(parentClass))
-                .Select(t => t.AsType());
+                .Select(t => t.AsType())
+                .ToList();
         }
 
         /// <summary>
         /// Returns the list of classes that implement the provided interface.
         /// </summary>
-        protected static IEnumerable<Type> GetClassesWithInterface(Type implementedInterface)
+        protected static List<Type> GetClassesWithInterface(Type implementedInterface)
         {
             var assembly = implementedInterface.GetTypeInfo().Assembly;
             return assembly.DefinedTypes
                 .Where(t => t.IsClass && t.ImplementedInterfaces.Contains(implementedInterface))
-                .Select(t => t.AsType());
+                .Select(t => t.AsType())
+                .ToList();
         }
     }
 }