From ec938fb0233f959883e4981b467781681bcf7113 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 10:40:22 -0600 Subject: [PATCH 01/11] - Add new InvalidParameterPairError exception type --- EasyPost/Constants.cs | 1 + .../General/InvalidParameterPairError.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 EasyPost/Exceptions/General/InvalidParameterPairError.cs diff --git a/EasyPost/Constants.cs b/EasyPost/Constants.cs index 41c163b0..620b2ec0 100644 --- a/EasyPost/Constants.cs +++ b/EasyPost/Constants.cs @@ -98,6 +98,7 @@ public static class ErrorMessages #pragma warning disable CS1591 // Missing XML comment for publicly visible type or member public const string InvalidApiKeyType = "Invalid API key type."; public const string InvalidParameter = "Invalid parameter: {0}."; + public const string InvalidParameterPair = "Invalid parameter pair: '{0}' and '{1}'."; public const string InvalidWebhookSignature = "Webhook does not contain a valid HMAC signature."; public const string JsonDeserializationError = "Error deserializing JSON into object of type {0}."; public const string JsonNoDataToDeserialize = "No data to deserialize."; diff --git a/EasyPost/Exceptions/General/InvalidParameterPairError.cs b/EasyPost/Exceptions/General/InvalidParameterPairError.cs new file mode 100644 index 00000000..bdcab166 --- /dev/null +++ b/EasyPost/Exceptions/General/InvalidParameterPairError.cs @@ -0,0 +1,21 @@ +using System.Globalization; + +namespace EasyPost.Exceptions.General +{ + /// + /// Represents an error that occurs due to an invalid parameter pair. + /// + public class InvalidParameterPairError : ValidationError + { + /// + /// Initializes a new instance of the class. + /// + /// The name of the first parameter in the pair. + /// The name of the second parameter in the pair. + /// Additional message to include in error message. + internal InvalidParameterPairError(string firstParameterName, string secondParameterName, string? followUpMessage = "") + : base($"{string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.InvalidParameterPair, firstParameterName, secondParameterName)}. {followUpMessage}") + { + } + } +} From f1de34fa99346abe9b549a10513422c88ee59183 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 10:42:00 -0600 Subject: [PATCH 02/11] - Add new top-level and nested parameter attributes to denote interdependency between multiple parameters --- .../Attributes/RequestParameterAttribute.cs | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs b/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs index bf5ce4d3..90e99173 100644 --- a/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs +++ b/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs @@ -21,6 +21,38 @@ public enum Necessity Optional, } + /// + /// An enum to represent the state of an independent parameter. + /// + public enum IndependentStatus + { + /// + /// Denote the condition when an independent parameter is set. + /// + IfSet, + + /// + /// Denote the condition when an independent parameter is not set. + /// + IfNotSet, + } + + /// + /// An enum to represent the state of a dependent parameter. + /// + public enum DependentStatus + { + /// + /// Denote that a dependent parameter must be set. + /// + MustBeSet, + + /// + /// Denote that a dependent parameter must not be set. + /// + MustNotBeSet, + } + #pragma warning disable CA1019 // Define accessors for attribute arguments /// /// A to label a parameter that will be sent in an HTTP request to the EasyPost API. @@ -50,6 +82,97 @@ protected RequestParameterAttribute(Necessity necessity, params string[] jsonPat } } + /// + /// A to denote the required dependents of a request parameter, and the conditions under which they are required. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = true, Inherited = false)] + public abstract class RequestParameterDependentsAttribute : BaseCustomAttribute + { + /// + /// The conditional set status of the independent property. + /// + private IndependentStatus IndependentStatus { get; } + + /// + /// The expected set status of the dependent properties. + /// + private DependentStatus DependentStatus { get; } + + /// + /// The names of the dependent properties. + /// + private List DependentProperties { get; } + + /// + /// Initializes a new instance of the class. + /// + /// The set status of the independent property. + /// The set status of the dependent properties. + /// The names of the dependent properties. + protected RequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) + { + IndependentStatus = independentStatus; + DependentStatus = dependentStatus; + DependentProperties = dependentProperties.ToList(); + } + + /// + /// Check that the expected value state of the property is met. + /// + /// Whether the dependent property must be set or not set. + /// The value of the dependent property. + /// True if the dependent property meets the dependency condition, false otherwise. + private static bool DependencyConditionPasses(DependentStatus dependentStatus, object? dependentPropertyValue) + { + return dependentStatus switch + { + DependentStatus.MustBeSet => dependentPropertyValue != null, + DependentStatus.MustNotBeSet => dependentPropertyValue == null, + var _ => false, + }; + } + + /// + /// Check that all dependent properties are compliant with the dependency conditions. + /// + /// The object containing the dependent properties. + /// The value of the independent property. + /// A tuple containing a boolean indicating whether the dependency is met, and a string containing the name of the first dependent property that does not meet the dependency conditions. + public Tuple DependentsAreCompliant(object obj, object? propertyValue) + { + // No need to check dependent IfSet properties if the property is not set + if (propertyValue == null && IndependentStatus == IndependentStatus.IfSet) + { + return new Tuple(true, string.Empty); + } + + // No need to check dependent IfNotSet properties if the property is set + if (propertyValue != null && IndependentStatus == IndependentStatus.IfNotSet) + { + return new Tuple(true, string.Empty); + } + + foreach (string dependentPropertyName in DependentProperties) + { + PropertyInfo? dependentProperty = obj.GetType().GetProperty(dependentPropertyName); + if (dependentProperty == null) // If a listed dependent property is not found, the dependency is not met (and there is likely a bug in the parameter set source code) + { + return new Tuple(false, dependentPropertyName); + } + + object? dependentPropertyValue = dependentProperty.GetValue(obj); + // If the dependent property does not meet the dependency condition, the dependency is not met + if (!DependencyConditionPasses(DependentStatus, dependentPropertyValue)) + { + return new Tuple(false, dependentPropertyName); + } + } + + // If all dependent properties meet the dependency conditions, the dependency is met + return new Tuple(true, string.Empty); + } + } + /// /// A to label a parameter that will be included in a top-level (standalone) JSON request body. /// @@ -66,6 +189,23 @@ public TopLevelRequestParameterAttribute(Necessity necessity, params string[] js } } + /// + /// A to denote the required dependents of a top-level request parameter, and the conditions under which they are required. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = true, Inherited = false)] + public sealed class TopLevelRequestParameterDependentsAttribute : RequestParameterDependentsAttribute + { + /// + /// Initializes a new instance of the class. + /// + /// The set status of the independent property. + /// The set status of the dependent properties. + /// The names of the dependent properties. + public TopLevelRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) : base(independentStatus, dependentStatus, dependentProperties) + { + } + } + /// /// A to label a parameter that will be included in an embedded dictionary inside another JSON request body (e.g. "address" data in "shipment" parameters). /// @@ -98,5 +238,23 @@ public NestedRequestParameterAttribute(Type parentType, Necessity necessity, par return attributes.FirstOrDefault(attribute => attribute.ParentType == parentType); } } + + /// + /// A to denote the required dependents of a nested request parameter, and the conditions under which they are required. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = true, Inherited = false)] + public sealed class NestedRequestParameterDependentsAttribute : RequestParameterDependentsAttribute + { + /// + /// Initializes a new instance of the class. + /// + /// The set status of the independent property. + /// The set status of the dependent properties. + /// The names of the dependent properties. + public NestedRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) : base(independentStatus, dependentStatus, dependentProperties) + { + } + } + #pragma warning restore CA1019 // Define accessors for attribute arguments } From 110a83bdd5da49123d8ee13a32bc511d9cc47899 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 10:42:29 -0600 Subject: [PATCH 03/11] - Check interdependency of parameters during serialization --- EasyPost/Parameters/BaseParameters.cs | 62 +++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/EasyPost/Parameters/BaseParameters.cs b/EasyPost/Parameters/BaseParameters.cs index a2129169..774da0f5 100644 --- a/EasyPost/Parameters/BaseParameters.cs +++ b/EasyPost/Parameters/BaseParameters.cs @@ -78,6 +78,17 @@ public virtual Dictionary ToDictionary() object? value = property.GetValue(this); + // Check dependent parameters before we finish handling the current parameter + IEnumerable dependentParameterAttributes = property.GetCustomAttributes(); + foreach (var dependentParameterAttribute in dependentParameterAttributes) + { + Tuple dependentParameterResult = dependentParameterAttribute.DependentsAreCompliant(this, value); + if (!dependentParameterResult.Item1) + { + throw new InvalidParameterPairError(firstParameterName: property.Name, secondParameterName: dependentParameterResult.Item2, followUpMessage: "Please verify the interdependence of these parameters."); + } + } + // If the value is null, check the necessity of the parameter if (value == null) { @@ -128,6 +139,17 @@ public virtual Dictionary ToSubDictionary(Type parentParameterOb object? value = property.GetValue(this); + // Check dependent parameters before we finish handling the current parameter + IEnumerable dependentParameterAttributes = property.GetCustomAttributes(); + foreach (var dependentParameterAttribute in dependentParameterAttributes) + { + Tuple dependentParameterResult = dependentParameterAttribute.DependentsAreCompliant(this, value); + if (!dependentParameterResult.Item1) + { + throw new InvalidParameterPairError(firstParameterName: property.Name, secondParameterName: dependentParameterResult.Item2, followUpMessage: "Please verify the interdependence of these parameters."); + } + } + // If the value is null, check the necessity of the parameter if (value == null) { @@ -256,5 +278,45 @@ private void Add(RequestParameterAttribute requestParameterAttribute, object? va return dictionary; } + + /// + /// Check that all parameters in a list are set. + /// + /// List of parameter names to check. + /// True if all parameters are set, false otherwise. + private bool AllParametersSet(List parameterNames) + { + return parameterNames.All(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null); + } + + /// + /// Check that at least one parameter in a list is set. + /// + /// List of parameter names to check. + /// True if at least one parameter is set, false otherwise. + private bool AtLeastOneParameterSet(List parameterNames) + { + return parameterNames.Any(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null); + } + + /// + /// Check that only one parameter in a list is set. + /// + /// List of parameter names to check. + /// True if only one parameter is set, false otherwise. + private bool OnlyOneParameterSet(List parameterNames) + { + return parameterNames.Count(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null) == 1; + } + + /// + /// Check that no parameters in a list are set. + /// + /// List of parameter names to check. + /// True if no parameters are set, false otherwise. + private bool NoParametersSet(List parameterNames) + { + return parameterNames.All(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) == null); + } } } From 3d0237276d2c7d9d3f2c617405d6cd4207a40e57 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 10:42:55 -0600 Subject: [PATCH 04/11] - Test parameter interdependency enforcement - Test invalid parameter pair exception --- .../ExceptionsTests/ExceptionsTest.cs | 4 + .../ParametersTests/ParametersTest.cs | 163 +++++++++++++++--- 2 files changed, 144 insertions(+), 23 deletions(-) diff --git a/EasyPost.Tests/ExceptionsTests/ExceptionsTest.cs b/EasyPost.Tests/ExceptionsTests/ExceptionsTest.cs index 3225ebba..5ba92689 100644 --- a/EasyPost.Tests/ExceptionsTests/ExceptionsTest.cs +++ b/EasyPost.Tests/ExceptionsTests/ExceptionsTest.cs @@ -98,6 +98,7 @@ public void TestExceptionConstructors() { const string testMessage = "This is a test message."; const string testPropertyName = "test_property"; + const string testPropertyName2 = "test_property2"; Type testType = typeof(ExceptionsTests); // Test the base EasyPostError constructor @@ -170,6 +171,9 @@ public void TestExceptionConstructors() InvalidParameterError invalidParameterError = new(testPropertyName); Assert.Equal($"{string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.InvalidParameter, testPropertyName)}. ", invalidParameterError.Message); + InvalidParameterPairError invalidParameterPairError = new(testPropertyName, testPropertyName2); + Assert.Equal($"{string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.InvalidParameterPair, testPropertyName, testPropertyName2)}. ", invalidParameterPairError.Message); + JsonDeserializationError jsonDeserializationError = new(testType); Assert.Equal(string.Format(CultureInfo.InvariantCulture, Constants.ErrorMessages.JsonDeserializationError, testType.FullName), jsonDeserializationError.Message); diff --git a/EasyPost.Tests/ParametersTests/ParametersTest.cs b/EasyPost.Tests/ParametersTests/ParametersTest.cs index f26234c5..9e6b082a 100644 --- a/EasyPost.Tests/ParametersTests/ParametersTest.cs +++ b/EasyPost.Tests/ParametersTests/ParametersTest.cs @@ -225,37 +225,102 @@ public void TestRequiredAndOptionalParameterValidation() Assert.Throws(() => parametersWithOnlyOptionalParameterSet.ToDictionary()); } - private sealed class ParameterSetWithRequiredAndOptionalParameters : Parameters.BaseParameters + [Fact] + [Testing.Exception] + public void TestDependentTopLevelParameters() { - [TopLevelRequestParameter(Necessity.Required, "test", "required")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? RequiredParameter { get; set; } + // Either A or B must be set, but not both. - [TopLevelRequestParameter(Necessity.Optional, "test", "optional")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? OptionalParameter { get; set; } - } + // Should throw exception if both set. + var parametersWithInterdependenceBothSet = new ParameterSetWithDependentTopLevelParameters + { + AParam = "A", + BParam = "B", + }; - private sealed class ParameterSetWithCompetingParameters : Parameters.BaseParameters - { - [TopLevelRequestParameter(Necessity.Optional, "location")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? AParam { get; set; } + Assert.Throws(() => parametersWithInterdependenceBothSet.ToDictionary()); + + // Should throw exception if neither set. + var parametersWithInterdependenceNeitherSet = new ParameterSetWithDependentTopLevelParameters(); + + Assert.Throws(() => parametersWithInterdependenceNeitherSet.ToDictionary()); + + // Should not throw exception if only A is set. + var parametersWithInterdependenceOnlyASet = new ParameterSetWithDependentTopLevelParameters + { + AParam = "A", + }; + + try + { + parametersWithInterdependenceOnlyASet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if only A is set."); + } - [TopLevelRequestParameter(Necessity.Optional, "location")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? BParam { get; set; } + // Should not throw exception if only B is set. + var parametersWithInterdependenceOnlyBSet = new ParameterSetWithDependentTopLevelParameters + { + BParam = "B", + }; + + try + { + parametersWithInterdependenceOnlyBSet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if only B is set."); + } } - private sealed class ParameterSetWithCompetingParametersNonAlphabetic : Parameters.BaseParameters + [Fact] + [Testing.Exception] + public void TestDependentNestedParameters() { - [TopLevelRequestParameter(Necessity.Optional, "location")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? BParam { get; set; } + // Either A or B must be set, but not both. - [TopLevelRequestParameter(Necessity.Optional, "location")] - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public string? AParam { get; set; } + // Should throw exception if both set. + var parametersWithInterdependenceBothSet = new ParameterSetWithDependentTopLevelParameters + { + AParam = "A", + BParam = "B", + }; + + Assert.Throws(() => parametersWithInterdependenceBothSet.ToDictionary()); + + // Should throw exception if neither set. + var parametersWithInterdependenceNeitherSet = new ParameterSetWithDependentTopLevelParameters(); + + Assert.Throws(() => parametersWithInterdependenceNeitherSet.ToDictionary()); + + // Should not throw exception if only A is set. + var parametersWithInterdependenceOnlyASet = new ParameterSetWithDependentTopLevelParameters + { + AParam = "A", + }; + + try + { + parametersWithInterdependenceOnlyASet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if only A is set."); + } + + // Should not throw exception if only B is set. + var parametersWithInterdependenceOnlyBSet = new ParameterSetWithDependentTopLevelParameters + { + BParam = "B", + }; + + try + { + parametersWithInterdependenceOnlyBSet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if only B is set."); + } } /// @@ -426,6 +491,8 @@ public void TestParameterMatchOverrideFunction() #endregion } + #region Fixtures + #pragma warning disable CA1852 // Can be sealed [SuppressMessage("ReSharper", "UnusedMember.Local")] internal class ExampleDecoratorParameters : Parameters.BaseParameters @@ -460,5 +527,55 @@ internal class ExampleMatchParameters : Parameters.BaseParameters + { + [TopLevelRequestParameter(Necessity.Required, "test", "required")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? RequiredParameter { get; set; } + + [TopLevelRequestParameter(Necessity.Optional, "test", "optional")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? OptionalParameter { get; set; } + } + + internal sealed class ParameterSetWithCompetingParameters : Parameters.BaseParameters + { + [TopLevelRequestParameter(Necessity.Optional, "location")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? AParam { get; set; } + + [TopLevelRequestParameter(Necessity.Optional, "location")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? BParam { get; set; } + } + + internal sealed class ParameterSetWithCompetingParametersNonAlphabetic : Parameters.BaseParameters + { + [TopLevelRequestParameter(Necessity.Optional, "location")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? BParam { get; set; } + + [TopLevelRequestParameter(Necessity.Optional, "location")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? AParam { get; set; } + } + + internal sealed class ParameterSetWithDependentTopLevelParameters : Parameters.BaseParameters + { + [TopLevelRequestParameter(Necessity.Optional, "a_param")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustNotBeSet, "BParam")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustBeSet, "BParam")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? AParam { get; set; } + + [TopLevelRequestParameter(Necessity.Optional, "b_param")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustNotBeSet, "AParam")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustBeSet, "AParam")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? BParam { get; set; } + } + #pragma warning restore CA1852 // Can be sealed + + #endregion } From a5e199818f1e308d0e86e12c9563d27cecb16ff0 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 10:43:25 -0600 Subject: [PATCH 05/11] - Shipment and Batch parameter for a Pickup.Create parameter set are interdependent (mutually exclusive) --- EasyPost/Parameters/Pickup/Create.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/EasyPost/Parameters/Pickup/Create.cs b/EasyPost/Parameters/Pickup/Create.cs index f1b5ccd3..9bfe7d56 100644 --- a/EasyPost/Parameters/Pickup/Create.cs +++ b/EasyPost/Parameters/Pickup/Create.cs @@ -22,6 +22,8 @@ public class Create : BaseParameters, IPickupParameter /// being set for pickup (required if not provided). /// [TopLevelRequestParameter(Necessity.Optional, "pickup", "batch")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustNotBeSet, "Shipment")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustBeSet, "Shipment")] public IBatchParameter? Batch { get; set; } /// @@ -65,6 +67,8 @@ public class Create : BaseParameters, IPickupParameter /// being set for pickup (required if not provided). /// [TopLevelRequestParameter(Necessity.Optional, "pickup", "shipment")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustNotBeSet, "Batch")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustBeSet, "Batch")] public IShipmentParameter? Shipment { get; set; } #endregion From fabe25301364d24747243c9a0a59cb85bf2912cc Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 11:03:54 -0600 Subject: [PATCH 06/11] - Add unit test for both or neither dependency --- .../ParametersTests/ParametersTest.cs | 92 ++++++++++++++++--- 1 file changed, 78 insertions(+), 14 deletions(-) diff --git a/EasyPost.Tests/ParametersTests/ParametersTest.cs b/EasyPost.Tests/ParametersTests/ParametersTest.cs index 9e6b082a..2bccb0cc 100644 --- a/EasyPost.Tests/ParametersTests/ParametersTest.cs +++ b/EasyPost.Tests/ParametersTests/ParametersTest.cs @@ -227,53 +227,102 @@ public void TestRequiredAndOptionalParameterValidation() [Fact] [Testing.Exception] - public void TestDependentTopLevelParameters() + public void TestOneOrOtherDependentTopLevelParameters() { // Either A or B must be set, but not both. // Should throw exception if both set. - var parametersWithInterdependenceBothSet = new ParameterSetWithDependentTopLevelParameters + var parametersWithOneOrOtherInterdependenceBothSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { AParam = "A", BParam = "B", }; - Assert.Throws(() => parametersWithInterdependenceBothSet.ToDictionary()); + Assert.Throws(() => parametersWithOneOrOtherInterdependenceBothSet.ToDictionary()); // Should throw exception if neither set. - var parametersWithInterdependenceNeitherSet = new ParameterSetWithDependentTopLevelParameters(); + var parametersWithOneOrOtherInterdependenceNeitherSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters(); - Assert.Throws(() => parametersWithInterdependenceNeitherSet.ToDictionary()); + Assert.Throws(() => parametersWithOneOrOtherInterdependenceNeitherSet.ToDictionary()); // Should not throw exception if only A is set. - var parametersWithInterdependenceOnlyASet = new ParameterSetWithDependentTopLevelParameters + var parametersWithOneOrOtherInterdependenceOnlyASet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { AParam = "A", }; try { - parametersWithInterdependenceOnlyASet.ToDictionary(); + parametersWithOneOrOtherInterdependenceOnlyASet.ToDictionary(); } catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only A is set."); } // Should not throw exception if only B is set. - var parametersWithInterdependenceOnlyBSet = new ParameterSetWithDependentTopLevelParameters + var parametersWithOneOrOtherInterdependenceOnlyBSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { BParam = "B", }; try { - parametersWithInterdependenceOnlyBSet.ToDictionary(); + parametersWithOneOrOtherInterdependenceOnlyBSet.ToDictionary(); } catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only B is set."); } } + [Fact] + [Testing.Exception] + public void TestBothOrNeitherDependentTopLevelParameters() + { + // Either both A and B must be set, or neither. + + // Should throw exception if only A is set. + var parametersWithBothOrNeitherInterdependenceOnlyASet = new ParameterSetWithBothOrNeitherDependentTopLevelParameters + { + AParam = "A", + }; + + Assert.Throws(() => parametersWithBothOrNeitherInterdependenceOnlyASet.ToDictionary()); + + // Should throw exception if only B is set. + var parametersWithBothOrNeitherInterdependenceOnlyBSet = new ParameterSetWithBothOrNeitherDependentTopLevelParameters + { + BParam = "B", + }; + + Assert.Throws(() => parametersWithBothOrNeitherInterdependenceOnlyBSet.ToDictionary()); + + // Should not throw exception if both A and B are set. + var parametersWithBothOrNeitherInterdependenceBothSet = new ParameterSetWithBothOrNeitherDependentTopLevelParameters + { + AParam = "A", + BParam = "B", + }; + + try + { + parametersWithBothOrNeitherInterdependenceBothSet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if both A and B are set."); + } + + // Should not throw exception if neither A nor B are set. + var parametersWithBothOrNeitherInterdependenceNeitherSet = new ParameterSetWithBothOrNeitherDependentTopLevelParameters(); + + try + { + parametersWithBothOrNeitherInterdependenceNeitherSet.ToDictionary(); + } catch (Exceptions.General.InvalidParameterPairError) + { + Assert.Fail("Should not throw exception if neither A nor B are set."); + } + } + [Fact] [Testing.Exception] public void TestDependentNestedParameters() @@ -281,7 +330,7 @@ public void TestDependentNestedParameters() // Either A or B must be set, but not both. // Should throw exception if both set. - var parametersWithInterdependenceBothSet = new ParameterSetWithDependentTopLevelParameters + var parametersWithInterdependenceBothSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { AParam = "A", BParam = "B", @@ -290,12 +339,12 @@ public void TestDependentNestedParameters() Assert.Throws(() => parametersWithInterdependenceBothSet.ToDictionary()); // Should throw exception if neither set. - var parametersWithInterdependenceNeitherSet = new ParameterSetWithDependentTopLevelParameters(); + var parametersWithInterdependenceNeitherSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters(); Assert.Throws(() => parametersWithInterdependenceNeitherSet.ToDictionary()); // Should not throw exception if only A is set. - var parametersWithInterdependenceOnlyASet = new ParameterSetWithDependentTopLevelParameters + var parametersWithInterdependenceOnlyASet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { AParam = "A", }; @@ -309,7 +358,7 @@ public void TestDependentNestedParameters() } // Should not throw exception if only B is set. - var parametersWithInterdependenceOnlyBSet = new ParameterSetWithDependentTopLevelParameters + var parametersWithInterdependenceOnlyBSet = new ParameterSetWithOneOrOtherDependentTopLevelParameters { BParam = "B", }; @@ -560,7 +609,7 @@ internal sealed class ParameterSetWithCompetingParametersNonAlphabetic : Paramet public string? AParam { get; set; } } - internal sealed class ParameterSetWithDependentTopLevelParameters : Parameters.BaseParameters + internal sealed class ParameterSetWithOneOrOtherDependentTopLevelParameters : Parameters.BaseParameters { [TopLevelRequestParameter(Necessity.Optional, "a_param")] [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustNotBeSet, "BParam")] @@ -575,6 +624,21 @@ internal sealed class ParameterSetWithDependentTopLevelParameters : Parameters.B public string? BParam { get; set; } } + internal sealed class ParameterSetWithBothOrNeitherDependentTopLevelParameters : Parameters.BaseParameters + { + [TopLevelRequestParameter(Necessity.Optional, "a_param")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustBeSet, "BParam")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustNotBeSet, "BParam")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? AParam { get; set; } + + [TopLevelRequestParameter(Necessity.Optional, "b_param")] + [TopLevelRequestParameterDependents(IndependentStatus.IfSet, DependentStatus.MustBeSet, "AParam")] + [TopLevelRequestParameterDependents(IndependentStatus.IfNotSet, DependentStatus.MustNotBeSet, "AParam")] + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string? BParam { get; set; } + } + #pragma warning restore CA1852 // Can be sealed #endregion From e7a8c410da342ac158fb37977a0fb6db7f1b22eb Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 11:09:39 -0600 Subject: [PATCH 07/11] - Drop coverage requirement due to new branch logic --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9805b409..4c90d206 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ coverage: ## coverage-check - Check if the coverage is above the minimum threshold coverage-check: - bash scripts/unix/check_coverage.sh 86 + bash scripts/unix/check_coverage.sh 85 ## docs - Generates library documentation docs: From 3b5fa49e496f5f040c87d2a913e8130a6e9b16c5 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 11:10:18 -0600 Subject: [PATCH 08/11] - Linting --- .../ParametersTests/ParametersTest.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/EasyPost.Tests/ParametersTests/ParametersTest.cs b/EasyPost.Tests/ParametersTests/ParametersTest.cs index 2bccb0cc..e623546b 100644 --- a/EasyPost.Tests/ParametersTests/ParametersTest.cs +++ b/EasyPost.Tests/ParametersTests/ParametersTest.cs @@ -254,7 +254,8 @@ public void TestOneOrOtherDependentTopLevelParameters() try { parametersWithOneOrOtherInterdependenceOnlyASet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only A is set."); } @@ -268,7 +269,8 @@ public void TestOneOrOtherDependentTopLevelParameters() try { parametersWithOneOrOtherInterdependenceOnlyBSet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only B is set."); } @@ -306,7 +308,8 @@ public void TestBothOrNeitherDependentTopLevelParameters() try { parametersWithBothOrNeitherInterdependenceBothSet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if both A and B are set."); } @@ -317,7 +320,8 @@ public void TestBothOrNeitherDependentTopLevelParameters() try { parametersWithBothOrNeitherInterdependenceNeitherSet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if neither A nor B are set."); } @@ -352,7 +356,8 @@ public void TestDependentNestedParameters() try { parametersWithInterdependenceOnlyASet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only A is set."); } @@ -366,7 +371,8 @@ public void TestDependentNestedParameters() try { parametersWithInterdependenceOnlyBSet.ToDictionary(); - } catch (Exceptions.General.InvalidParameterPairError) + } + catch (Exceptions.General.InvalidParameterPairError) { Assert.Fail("Should not throw exception if only B is set."); } From 751f475265e5f1cb181c82192d7cdf596dad9692 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 11:24:11 -0600 Subject: [PATCH 09/11] - Remove dead demo code --- EasyPost/Parameters/BaseParameters.cs | 40 ------------------- .../Attributes/RequestParameterAttribute.cs | 10 +++-- 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/EasyPost/Parameters/BaseParameters.cs b/EasyPost/Parameters/BaseParameters.cs index 774da0f5..e8db7789 100644 --- a/EasyPost/Parameters/BaseParameters.cs +++ b/EasyPost/Parameters/BaseParameters.cs @@ -278,45 +278,5 @@ private void Add(RequestParameterAttribute requestParameterAttribute, object? va return dictionary; } - - /// - /// Check that all parameters in a list are set. - /// - /// List of parameter names to check. - /// True if all parameters are set, false otherwise. - private bool AllParametersSet(List parameterNames) - { - return parameterNames.All(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null); - } - - /// - /// Check that at least one parameter in a list is set. - /// - /// List of parameter names to check. - /// True if at least one parameter is set, false otherwise. - private bool AtLeastOneParameterSet(List parameterNames) - { - return parameterNames.Any(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null); - } - - /// - /// Check that only one parameter in a list is set. - /// - /// List of parameter names to check. - /// True if only one parameter is set, false otherwise. - private bool OnlyOneParameterSet(List parameterNames) - { - return parameterNames.Count(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) != null) == 1; - } - - /// - /// Check that no parameters in a list are set. - /// - /// List of parameter names to check. - /// True if no parameters are set, false otherwise. - private bool NoParametersSet(List parameterNames) - { - return parameterNames.All(parameterName => GetType().GetProperty(parameterName)?.GetValue(this) == null); - } } } diff --git a/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs b/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs index 90e99173..23ef11a1 100644 --- a/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs +++ b/EasyPost/Utilities/Internal/Attributes/RequestParameterAttribute.cs @@ -155,7 +155,9 @@ public Tuple DependentsAreCompliant(object obj, object? propertyVa foreach (string dependentPropertyName in DependentProperties) { PropertyInfo? dependentProperty = obj.GetType().GetProperty(dependentPropertyName); - if (dependentProperty == null) // If a listed dependent property is not found, the dependency is not met (and there is likely a bug in the parameter set source code) + + // If a listed dependent property is not found, the dependency is not met (and there is likely a bug in the parameter set source code) + if (dependentProperty == null) { return new Tuple(false, dependentPropertyName); } @@ -201,7 +203,8 @@ public sealed class TopLevelRequestParameterDependentsAttribute : RequestParamet /// The set status of the independent property. /// The set status of the dependent properties. /// The names of the dependent properties. - public TopLevelRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) : base(independentStatus, dependentStatus, dependentProperties) + public TopLevelRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) + : base(independentStatus, dependentStatus, dependentProperties) { } } @@ -251,7 +254,8 @@ public sealed class NestedRequestParameterDependentsAttribute : RequestParameter /// The set status of the independent property. /// The set status of the dependent properties. /// The names of the dependent properties. - public NestedRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) : base(independentStatus, dependentStatus, dependentProperties) + public NestedRequestParameterDependentsAttribute(IndependentStatus independentStatus, DependentStatus dependentStatus, params string[] dependentProperties) + : base(independentStatus, dependentStatus, dependentProperties) { } } From 31439504606b957d85e8d865be986055e61375b6 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 11:29:04 -0600 Subject: [PATCH 10/11] - Linting (inconsistency between IDE and CI) --- EasyPost/Parameters/BaseParameters.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/EasyPost/Parameters/BaseParameters.cs b/EasyPost/Parameters/BaseParameters.cs index e8db7789..b888fc4d 100644 --- a/EasyPost/Parameters/BaseParameters.cs +++ b/EasyPost/Parameters/BaseParameters.cs @@ -80,7 +80,7 @@ public virtual Dictionary ToDictionary() // Check dependent parameters before we finish handling the current parameter IEnumerable dependentParameterAttributes = property.GetCustomAttributes(); - foreach (var dependentParameterAttribute in dependentParameterAttributes) + foreach (TopLevelRequestParameterDependentsAttribute dependentParameterAttribute in dependentParameterAttributes) { Tuple dependentParameterResult = dependentParameterAttribute.DependentsAreCompliant(this, value); if (!dependentParameterResult.Item1) @@ -140,8 +140,8 @@ public virtual Dictionary ToSubDictionary(Type parentParameterOb object? value = property.GetValue(this); // Check dependent parameters before we finish handling the current parameter - IEnumerable dependentParameterAttributes = property.GetCustomAttributes(); - foreach (var dependentParameterAttribute in dependentParameterAttributes) + IEnumerable dependentParameterAttributes = property.GetCustomAttributes(); + foreach (NestedRequestParameterDependentsAttribute dependentParameterAttribute in dependentParameterAttributes) { Tuple dependentParameterResult = dependentParameterAttribute.DependentsAreCompliant(this, value); if (!dependentParameterResult.Item1) From 5013627e018f463d01578775811a6cde99d94211 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Wed, 12 Jun 2024 16:49:58 -0600 Subject: [PATCH 11/11] - Update CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4619a794..d0c35ffc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG +## Next Release + +- Enforce one-or-other for `Shipment` and `Batch` parameters in `Pickup.Create` parameter set +- Add internal parameter dependency utility + ## v6.5.2 (2024-06-12) - Fix `Shipment` parameter requirement for `Pickup.Create` parameter set